From c69b8e04591907d030b2cd544a606fd0e576116a Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 14 Sep 2014 19:32:51 +0300 Subject: Huge replace of old users_project and users_group references Signed-off-by: Dmitriy Zaporozhets --- app/assets/javascripts/admin.js.coffee | 4 +- app/assets/javascripts/groups.js.coffee | 2 +- app/controllers/admin/groups_controller.rb | 4 +- app/controllers/admin/projects_controller.rb | 4 +- app/controllers/groups_controller.rb | 6 +- app/controllers/profiles/groups_controller.rb | 4 +- .../profiles/notifications_controller.rb | 8 +- .../projects/team_members_controller.rb | 14 +- app/controllers/users_groups_controller.rb | 10 +- app/finders/projects_finder.rb | 6 +- app/mailers/emails/groups.rb | 4 +- app/mailers/emails/projects.rb | 4 +- app/models/ability.rb | 2 +- app/models/concerns/notifiable.rb | 2 +- app/models/group.rb | 20 +-- app/models/group_member.rb | 2 - app/models/member.rb | 14 +- app/models/members/group_member.rb | 40 ++++++ app/models/members/project_member.rb | 133 ++++++++++++++++++ app/models/project_member.rb | 2 - app/models/project_team.rb | 22 +-- app/models/user.rb | 16 ++- app/models/users_group.rb | 61 --------- app/models/users_project.rb | 152 --------------------- app/services/notification_service.rb | 16 +-- app/services/projects/create_service.rb | 5 +- app/services/projects/fork_service.rb | 2 +- app/services/system_hooks_service.rb | 6 +- app/views/admin/groups/show.html.haml | 4 +- app/views/admin/projects/show.html.haml | 4 +- app/views/admin/users/show.html.haml | 8 +- app/views/groups/_new_group_member.html.haml | 4 +- .../profiles/notifications/_settings.html.haml | 2 +- app/views/projects/team_members/_form.html.haml | 4 +- .../projects/team_members/_group_members.html.haml | 4 +- .../projects/team_members/_team_member.html.haml | 2 +- app/views/users_groups/_users_group.html.haml | 2 +- config/application.rb | 6 +- lib/api/entities.rb | 8 +- lib/api/groups.rb | 8 +- lib/api/project_members.rb | 6 +- lib/gitlab/access.rb | 4 + lib/tasks/gitlab/bulk_add_permission.rake | 12 +- lib/tasks/gitlab/check.rake | 6 +- spec/factories.rb | 2 +- spec/factories/users_groups.rb | 2 +- spec/helpers/gitlab_markdown_helper_spec.rb | 2 +- spec/models/group_member_spec.rb | 44 ++++++ spec/models/group_spec.rb | 16 +-- spec/models/members_spec.rb | 5 + spec/models/note_spec.rb | 14 +- spec/models/project_member_spec.rb | 90 ++++++++++++ spec/models/project_security_spec.rb | 16 +-- spec/models/system_hook_spec.rb | 2 +- spec/models/user_spec.rb | 2 +- spec/models/users_group_spec.rb | 67 --------- spec/models/users_project_spec.rb | 113 --------------- spec/requests/api/branches_spec.rb | 4 +- spec/requests/api/commits_spec.rb | 4 +- spec/requests/api/groups_spec.rb | 36 ++--- spec/requests/api/project_members_spec.rb | 34 ++--- spec/requests/api/projects_spec.rb | 4 +- spec/requests/api/repositories_spec.rb | 4 +- spec/services/notification_service_spec.rb | 6 +- 64 files changed, 519 insertions(+), 597 deletions(-) delete mode 100644 app/models/group_member.rb create mode 100644 app/models/members/group_member.rb create mode 100644 app/models/members/project_member.rb delete mode 100644 app/models/project_member.rb delete mode 100644 app/models/users_group.rb delete mode 100644 app/models/users_project.rb create mode 100644 spec/models/group_member_spec.rb create mode 100644 spec/models/project_member_spec.rb delete mode 100644 spec/models/users_group_spec.rb delete mode 100644 spec/models/users_project_spec.rb diff --git a/app/assets/javascripts/admin.js.coffee b/app/assets/javascripts/admin.js.coffee index 6634bb6cc34..a333eed87f2 100644 --- a/app/assets/javascripts/admin.js.coffee +++ b/app/assets/javascripts/admin.js.coffee @@ -46,10 +46,10 @@ class Admin modal.hide() $('.change-owner-link').show() - $('li.users_project').bind 'ajax:success', -> + $('li.project_member').bind 'ajax:success', -> Turbolinks.visit(location.href) - $('li.users_group').bind 'ajax:success', -> + $('li.group_member').bind 'ajax:success', -> Turbolinks.visit(location.href) @Admin = Admin diff --git a/app/assets/javascripts/groups.js.coffee b/app/assets/javascripts/groups.js.coffee index 49d6605980b..4b1000f9a6a 100644 --- a/app/assets/javascripts/groups.js.coffee +++ b/app/assets/javascripts/groups.js.coffee @@ -1,6 +1,6 @@ class GroupMembers constructor: -> - $('li.users_group').bind 'ajax:success', -> + $('li.group_member').bind 'ajax:success', -> $(this).fadeOut() @GroupMembers = GroupMembers diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index 0388997ec69..e6d0c9323c1 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -8,7 +8,7 @@ class Admin::GroupsController < Admin::ApplicationController end def show - @members = @group.members.order("group_access DESC").page(params[:members_page]).per(30) + @members = @group.members.order("access_level DESC").page(params[:members_page]).per(30) @projects = @group.projects.page(params[:projects_page]).per(30) end @@ -40,7 +40,7 @@ class Admin::GroupsController < Admin::ApplicationController end def project_teams_update - @group.add_users(params[:user_ids].split(','), params[:group_access]) + @group.add_users(params[:user_ids].split(','), params[:access_level]) redirect_to [:admin, @group], notice: 'Users were successfully added.' end diff --git a/app/controllers/admin/projects_controller.rb b/app/controllers/admin/projects_controller.rb index 1c7c09d0cd4..2f0d344802f 100644 --- a/app/controllers/admin/projects_controller.rb +++ b/app/controllers/admin/projects_controller.rb @@ -16,10 +16,10 @@ class Admin::ProjectsController < Admin::ApplicationController def show if @group - @group_members = @group.members.order("group_access DESC").page(params[:group_members_page]).per(30) + @group_members = @group.members.order("access_level DESC").page(params[:group_members_page]).per(30) end - @project_members = @project.users_projects.page(params[:project_members_page]).per(30) + @project_members = @project.project_members.page(params[:project_members_page]).per(30) end def transfer diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index ddde90d3ee0..36222758eb2 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -65,15 +65,15 @@ class GroupsController < ApplicationController def members @project = group.projects.find(params[:project_id]) if params[:project_id] - @members = group.users_groups + @members = group.group_members if params[:search].present? users = group.users.search(params[:search]).to_a @members = @members.where(user_id: users) end - @members = @members.order('group_access DESC').page(params[:page]).per(50) - @users_group = UsersGroup.new + @members = @members.order('access_level DESC').page(params[:page]).per(50) + @users_group = GroupMember.new end def edit diff --git a/app/controllers/profiles/groups_controller.rb b/app/controllers/profiles/groups_controller.rb index 9a4d088651e..ce9dd50df67 100644 --- a/app/controllers/profiles/groups_controller.rb +++ b/app/controllers/profiles/groups_controller.rb @@ -2,11 +2,11 @@ class Profiles::GroupsController < ApplicationController layout "profile" def index - @user_groups = current_user.users_groups.page(params[:page]).per(20) + @user_groups = current_user.group_members.page(params[:page]).per(20) end def leave - @users_group = group.users_groups.where(user_id: current_user.id).first + @users_group = group.group_members.where(user_id: current_user.id).first if can?(current_user, :destroy, @users_group) @users_group.destroy redirect_to(profile_groups_path, info: "You left #{group.name} group.") diff --git a/app/controllers/profiles/notifications_controller.rb b/app/controllers/profiles/notifications_controller.rb index 5c492aeb49d..936fe071347 100644 --- a/app/controllers/profiles/notifications_controller.rb +++ b/app/controllers/profiles/notifications_controller.rb @@ -3,8 +3,8 @@ class Profiles::NotificationsController < ApplicationController def show @notification = current_user.notification - @users_projects = current_user.users_projects - @users_groups = current_user.users_groups + @users_projects = current_user.project_members + @users_groups = current_user.group_members end def update @@ -14,11 +14,11 @@ class Profiles::NotificationsController < ApplicationController current_user.notification_level = params[:notification_level] current_user.save elsif type == 'group' - users_group = current_user.users_groups.find(params[:notification_id]) + users_group = current_user.group_members.find(params[:notification_id]) users_group.notification_level = params[:notification_level] users_group.save else - users_project = current_user.users_projects.find(params[:notification_id]) + users_project = current_user.project_members.find(params[:notification_id]) users_project.notification_level = params[:notification_level] users_project.save end diff --git a/app/controllers/projects/team_members_controller.rb b/app/controllers/projects/team_members_controller.rb index 1de5bac9ee8..575797d36dc 100644 --- a/app/controllers/projects/team_members_controller.rb +++ b/app/controllers/projects/team_members_controller.rb @@ -6,17 +6,17 @@ class Projects::TeamMembersController < Projects::ApplicationController def index @group = @project.group - @users_projects = @project.users_projects.order('project_access DESC') + @users_projects = @project.project_members.order('access_level DESC') end def new - @user_project_relation = project.users_projects.new + @user_project_relation = project.project_members.new end def create users = User.where(id: params[:user_ids].split(',')) - @project.team << [users, params[:project_access]] + @project.team << [users, params[:access_level]] if params[:redirect_to] redirect_to params[:redirect_to] @@ -26,7 +26,7 @@ class Projects::TeamMembersController < Projects::ApplicationController end def update - @user_project_relation = project.users_projects.find_by(user_id: member) + @user_project_relation = project.project_members.find_by(user_id: member) @user_project_relation.update_attributes(member_params) unless @user_project_relation.valid? @@ -36,7 +36,7 @@ class Projects::TeamMembersController < Projects::ApplicationController end def destroy - @user_project_relation = project.users_projects.find_by(user_id: member) + @user_project_relation = project.project_members.find_by(user_id: member) @user_project_relation.destroy respond_to do |format| @@ -46,7 +46,7 @@ class Projects::TeamMembersController < Projects::ApplicationController end def leave - project.users_projects.find_by(user_id: current_user).destroy + project.project_members.find_by(user_id: current_user).destroy respond_to do |format| format.html { redirect_to :back } @@ -69,6 +69,6 @@ class Projects::TeamMembersController < Projects::ApplicationController end def member_params - params.require(:team_member).permit(:user_id, :project_access) + params.require(:team_member).permit(:user_id, :access_level) end end diff --git a/app/controllers/users_groups_controller.rb b/app/controllers/users_groups_controller.rb index a35a12a866b..07bd41d25eb 100644 --- a/app/controllers/users_groups_controller.rb +++ b/app/controllers/users_groups_controller.rb @@ -1,4 +1,4 @@ -class UsersGroupsController < ApplicationController +class GroupMembersController < ApplicationController before_filter :group # Authorize @@ -7,18 +7,18 @@ class UsersGroupsController < ApplicationController layout 'group' def create - @group.add_users(params[:user_ids].split(','), params[:group_access]) + @group.add_users(params[:user_ids].split(','), params[:access_level]) redirect_to members_group_path(@group), notice: 'Users were successfully added.' end def update - @member = @group.users_groups.find(params[:id]) + @member = @group.group_members.find(params[:id]) @member.update_attributes(member_params) end def destroy - @users_group = @group.users_groups.find(params[:id]) + @users_group = @group.group_members.find(params[:id]) if can?(current_user, :destroy, @users_group) # May fail if last owner. @users_group.destroy respond_to do |format| @@ -43,6 +43,6 @@ class UsersGroupsController < ApplicationController end def member_params - params.require(:users_group).permit(:group_access, :user_id) + params.require(:users_group).permit(:access_level, :user_id) end end diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb index 26898bad493..222496cd317 100644 --- a/app/finders/projects_finder.rb +++ b/app/finders/projects_finder.rb @@ -19,10 +19,8 @@ class ProjectsFinder # Return ALL group projects group.projects else - projects_members = UsersProject.where( - project_id: group.projects, - user_id: current_user - ) + projects_members = ProjectMember.in_projects(group.projects). + with_user(current_user) if projects_members.any? # User is a project member diff --git a/app/mailers/emails/groups.rb b/app/mailers/emails/groups.rb index 1654fc55bca..3f7c1d27350 100644 --- a/app/mailers/emails/groups.rb +++ b/app/mailers/emails/groups.rb @@ -1,7 +1,7 @@ module Emails module Groups - def group_access_granted_email(user_group_id) - @membership = UsersGroup.find(user_group_id) + def access_level_granted_email(user_group_id) + @membership = GroupMember.find(user_group_id) @group = @membership.group @target_url = group_url(@group) mail(to: @membership.user.email, diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb index 6b13a1d746d..152d5aa9959 100644 --- a/app/mailers/emails/projects.rb +++ b/app/mailers/emails/projects.rb @@ -1,7 +1,7 @@ module Emails module Projects - def project_access_granted_email(user_project_id) - @users_project = UsersProject.find user_project_id + def access_level_granted_email(user_project_id) + @users_project = ProjectMember.find user_project_id @project = @users_project.project @target_url = project_url(@project) mail(to: @users_project.user.email, diff --git a/app/models/ability.rb b/app/models/ability.rb index f1d57de63bb..716a23a4284 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -14,7 +14,7 @@ class Ability when "MergeRequest" then merge_request_abilities(user, subject) when "Group" then group_abilities(user, subject) when "Namespace" then namespace_abilities(user, subject) - when "UsersGroup" then users_group_abilities(user, subject) + when "GroupMember" then users_group_abilities(user, subject) else [] end.concat(global_abilities(user)) end diff --git a/app/models/concerns/notifiable.rb b/app/models/concerns/notifiable.rb index 722f375e71d..d7dcd97911d 100644 --- a/app/models/concerns/notifiable.rb +++ b/app/models/concerns/notifiable.rb @@ -1,6 +1,6 @@ # == Notifiable concern # -# Contains notification functionality shared between UsersProject and UsersGroup +# Contains notification functionality # module Notifiable extend ActiveSupport::Concern diff --git a/app/models/group.rb b/app/models/group.rb index 66239f7fe6f..b8ed3b8ac73 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -17,8 +17,8 @@ require 'carrierwave/orm/activerecord' require 'file_size_validator' class Group < Namespace - has_many :users_groups, dependent: :destroy - has_many :users, through: :users_groups + has_many :group_members, dependent: :destroy, as: :source, class_name: 'GroupMember' + has_many :users, through: :group_members validate :avatar_type, if: ->(user) { user.avatar_changed? } validates :avatar, file_size: { maximum: 100.kilobytes.to_i } @@ -30,22 +30,22 @@ class Group < Namespace end def owners - @owners ||= users_groups.owners.map(&:user) + @owners ||= group_members.owners.map(&:user) end - def add_users(user_ids, group_access) + def add_users(user_ids, access_level) user_ids.compact.each do |user_id| - user = self.users_groups.find_or_initialize_by(user_id: user_id) - user.update_attributes(group_access: group_access) + user = self.group_members.find_or_initialize_by(user_id: user_id) + user.update_attributes(access_level: access_level) end end - def add_user(user, group_access) - self.users_groups.create(user_id: user.id, group_access: group_access) + def add_user(user, access_level) + self.group_members.create(user_id: user.id, access_level: access_level) end def add_owner(user) - self.add_user(user, UsersGroup::OWNER) + self.add_user(user, Gitlab::Access::OWNER) end def has_owner?(user) @@ -61,7 +61,7 @@ class Group < Namespace end def members - users_groups + group_members end def avatar_type diff --git a/app/models/group_member.rb b/app/models/group_member.rb deleted file mode 100644 index 88c207deef9..00000000000 --- a/app/models/group_member.rb +++ /dev/null @@ -1,2 +0,0 @@ -class GroupMember < Member -end diff --git a/app/models/member.rb b/app/models/member.rb index 13827db175b..7dc13c18bf3 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -8,11 +8,13 @@ class Member < ActiveRecord::Base validates :user, presence: true validates :source, presence: true validates :user_id, uniqueness: { scope: [:source_type, :source_id], message: "already exists in source" } - validates :access_level, inclusion: { in: Gitlab::Access.values }, presence: true + validates :access_level, inclusion: { in: Gitlab::Access.all_values }, presence: true - scope :guests, -> { where(group_access: GUEST) } - scope :reporters, -> { where(group_access: REPORTER) } - scope :developers, -> { where(group_access: DEVELOPER) } - scope :masters, -> { where(group_access: MASTER) } - scope :owners, -> { where(group_access: OWNER) } + scope :guests, -> { where(access_level: GUEST) } + scope :reporters, -> { where(access_level: REPORTER) } + scope :developers, -> { where(access_level: DEVELOPER) } + scope :masters, -> { where(access_level: MASTER) } + scope :owners, -> { where(access_level: OWNER) } + + delegate :name, :username, :email, to: :user, prefix: true end diff --git a/app/models/members/group_member.rb b/app/models/members/group_member.rb new file mode 100644 index 00000000000..7f68662de70 --- /dev/null +++ b/app/models/members/group_member.rb @@ -0,0 +1,40 @@ +class GroupMember < Member + SOURCE_TYPE = 'Group' + + # Make sure group member points only to group as it source + default_value_for :source_type, SOURCE_TYPE + validates_format_of :source_type, with: /\AGroup\z/ + default_scope { where(source_type: SOURCE_TYPE) } + + scope :with_group, ->(group) { where(source_id: group.id) } + scope :with_user, ->(user) { where(user_id: user.id) } + + after_create :notify_create + after_update :notify_update + + def self.access_level_roles + Gitlab::Access.options_with_owner + end + + def group + source + end + + def access_field + access_level + end + + def notify_create + notification_service.new_group_member(self) + end + + def notify_update + if access_level_changed? + notification_service.update_group_member(self) + end + end + + def notification_service + NotificationService.new + end +end diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb new file mode 100644 index 00000000000..403b87b7e36 --- /dev/null +++ b/app/models/members/project_member.rb @@ -0,0 +1,133 @@ +class ProjectMember < Member + SOURCE_TYPE = 'Project' + + include Gitlab::ShellAdapter + + # Make sure project member points only to project as it source + default_value_for :source_type, SOURCE_TYPE + validates_format_of :source_type, with: /\AProject\z/ + default_scope { where(source_type: SOURCE_TYPE) } + + after_create :post_create_hook + after_update :post_update_hook + after_destroy :post_destroy_hook + + scope :in_project, ->(project) { where(source_id: project.id) } + scope :in_projects, ->(projects) { where(source_id: projects.pluck(:id)) } + scope :with_user, ->(user) { where(user_id: user.id) } + + class << self + + # Add users to project teams with passed access option + # + # access can be an integer representing a access code + # or symbol like :master representing role + # + # Ex. + # add_users_into_projects( + # project_ids, + # user_ids, + # ProjectMember::MASTER + # ) + # + # add_users_into_projects( + # project_ids, + # user_ids, + # :master + # ) + # + def add_users_into_projects(project_ids, user_ids, access) + access_level = if roles_hash.has_key?(access) + roles_hash[access] + elsif roles_hash.values.include?(access.to_i) + access + else + raise "Non valid access" + end + + ProjectMember.transaction do + project_ids.each do |project_id| + user_ids.each do |user_id| + member = ProjectMember.new(access_level: access_level, user_id: user_id) + member.source_id = project_id + member.save + end + end + end + + true + rescue + false + end + + def truncate_teams(project_ids) + ProjectMember.transaction do + members = ProjectMember.where(source_id: project_ids) + members.each do |member| + member.destroy + end + end + + true + rescue + false + end + + def truncate_team project + truncate_teams [project.id] + end + + def roles_hash + Gitlab::Access.sym_options + end + + def access_roles + Gitlab::Access.options + end + end + + def access_field + access_level + end + + def owner? + project.owner == user + end + + def post_create_hook + Event.create( + project_id: self.project.id, + action: Event::JOINED, + author_id: self.user.id + ) + + notification_service.new_team_member(self) unless owner? + system_hook_service.execute_hooks_for(self, :create) + end + + def post_update_hook + notification_service.update_team_member(self) if self.access_level_changed? + end + + def post_destroy_hook + Event.create( + project_id: self.project.id, + action: Event::LEFT, + author_id: self.user.id + ) + + system_hook_service.execute_hooks_for(self, :destroy) + end + + def notification_service + NotificationService.new + end + + def system_hook_service + SystemHooksService.new + end + + def project + source + end +end diff --git a/app/models/project_member.rb b/app/models/project_member.rb deleted file mode 100644 index 67f65285560..00000000000 --- a/app/models/project_member.rb +++ /dev/null @@ -1,2 +0,0 @@ -class ProjectMember < Member -end diff --git a/app/models/project_team.rb b/app/models/project_team.rb index 0bbbd3d00e8..330e2aa728c 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -32,12 +32,12 @@ class ProjectTeam end def find_tm(user_id) - tm = project.users_projects.find_by(user_id: user_id) + tm = project.project_members.find_by(user_id: user_id) # If user is not in project members # we should check for group membership if group && !tm - tm = group.users_groups.find_by(user_id: user_id) + tm = group.group_members.find_by(user_id: user_id) end tm @@ -52,7 +52,7 @@ class ProjectTeam end def add_users_ids(user_ids, access) - UsersProject.add_users_into_projects( + ProjectMember.add_users_into_projects( [project.id], user_ids, access @@ -61,7 +61,7 @@ class ProjectTeam # Remove all users from project team def truncate - UsersProject.truncate_team(project) + ProjectMember.truncate_team(project) end def users @@ -91,8 +91,8 @@ class ProjectTeam def import(source_project) target_project = project - source_team = source_project.users_projects.to_a - target_user_ids = target_project.users_projects.pluck(:user_id) + source_team = source_project.project_members.to_a + target_user_ids = target_project.project_members.pluck(:user_id) source_team.reject! do |tm| # Skip if user already present in team @@ -106,7 +106,7 @@ class ProjectTeam new_tm end - UsersProject.transaction do + ProjectMember.transaction do source_team.each do |tm| tm.save end @@ -135,10 +135,10 @@ class ProjectTeam def max_tm_access(user_id) access = [] - access << project.users_projects.find_by(user_id: user_id).try(:access_field) + access << project.project_members.find_by(user_id: user_id).try(:access_field) if group - access << group.users_groups.find_by(user_id: user_id).try(:access_field) + access << group.group_members.find_by(user_id: user_id).try(:access_field) end access.compact.max @@ -147,8 +147,8 @@ class ProjectTeam private def fetch_members(level = nil) - project_members = project.users_projects - group_members = group ? group.users_groups : [] + project_members = project.project_members + group_members = group ? group.group_members : [] if level project_members = project_members.send(level) diff --git a/app/models/user.rb b/app/models/user.rb index 15e56a62a68..b235437817a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -81,10 +81,12 @@ class User < ActiveRecord::Base has_many :emails, dependent: :destroy # Groups - has_many :users_groups, dependent: :destroy - has_many :groups, through: :users_groups - has_many :owned_groups, -> { where users_groups: { group_access: UsersGroup::OWNER } }, through: :users_groups, source: :group - has_many :masters_groups, -> { where users_groups: { group_access: UsersGroup::MASTER } }, through: :users_groups, source: :group + has_many :members, dependent: :destroy + has_many :project_members, source: 'ProjectMember' + has_many :group_members, source: 'GroupMember' + has_many :groups, through: :group_members + has_many :owned_groups, -> { where group_members: { access_level: Gitlab::Access::OWNER } }, through: :group_members, source: :group + has_many :masters_groups, -> { where group_members: { access_level: Gitlab::Access::MASTER } }, through: :group_members, source: :group # Projects has_many :groups_projects, through: :groups, source: :projects @@ -140,7 +142,7 @@ class User < ActiveRecord::Base state_machine :state, initial: :active do after_transition any => :blocked do |user, transition| # Remove user from all projects and - user.users_projects.find_each do |membership| + user.project_members.find_each do |membership| # skip owned resources next if membership.project.owner == user @@ -148,7 +150,7 @@ class User < ActiveRecord::Base end # Remove user from all groups - user.users_groups.find_each do |membership| + user.group_members.find_each do |membership| # skip owned resources next if membership.group.last_owner?(user) @@ -295,7 +297,7 @@ class User < ActiveRecord::Base # Team membership in authorized projects def tm_in_authorized_projects - UsersProject.where(project_id: authorized_projects.map(&:id), user_id: self.id) + ProjectMember.where(source_id: authorized_projects.map(&:id), user_id: self.id) end def is_admin? diff --git a/app/models/users_group.rb b/app/models/users_group.rb deleted file mode 100644 index 270f968ef61..00000000000 --- a/app/models/users_group.rb +++ /dev/null @@ -1,61 +0,0 @@ -# == Schema Information -# -# Table name: users_groups -# -# id :integer not null, primary key -# group_access :integer not null -# group_id :integer not null -# user_id :integer not null -# created_at :datetime -# updated_at :datetime -# notification_level :integer default(3), not null -# - -class UsersGroup < ActiveRecord::Base - include Notifiable - include Gitlab::Access - - def self.group_access_roles - Gitlab::Access.options_with_owner - end - - belongs_to :user - belongs_to :group - - scope :guests, -> { where(group_access: GUEST) } - scope :reporters, -> { where(group_access: REPORTER) } - scope :developers, -> { where(group_access: DEVELOPER) } - scope :masters, -> { where(group_access: MASTER) } - scope :owners, -> { where(group_access: OWNER) } - - scope :with_group, ->(group) { where(group_id: group.id) } - scope :with_user, ->(user) { where(user_id: user.id) } - - after_create :notify_create - after_update :notify_update - - validates :group_access, inclusion: { in: UsersGroup.group_access_roles.values }, presence: true - validates :user_id, presence: true - validates :group_id, presence: true - validates :user_id, uniqueness: { scope: [:group_id], message: "already exists in group" } - - delegate :name, :username, :email, to: :user, prefix: true - - def access_field - group_access - end - - def notify_create - notification_service.new_group_member(self) - end - - def notify_update - if group_access_changed? - notification_service.update_group_member(self) - end - end - - def notification_service - NotificationService.new - end -end diff --git a/app/models/users_project.rb b/app/models/users_project.rb deleted file mode 100644 index 60bdf7a3cfb..00000000000 --- a/app/models/users_project.rb +++ /dev/null @@ -1,152 +0,0 @@ -# == Schema Information -# -# Table name: users_projects -# -# id :integer not null, primary key -# user_id :integer not null -# project_id :integer not null -# created_at :datetime -# updated_at :datetime -# project_access :integer default(0), not null -# notification_level :integer default(3), not null -# - -class UsersProject < ActiveRecord::Base - include Gitlab::ShellAdapter - include Notifiable - include Gitlab::Access - - belongs_to :user - belongs_to :project - - validates :user, presence: true - validates :user_id, uniqueness: { scope: [:project_id], message: "already exists in project" } - validates :project_access, inclusion: { in: Gitlab::Access.values }, presence: true - validates :project, presence: true - - delegate :name, :username, :email, to: :user, prefix: true - - scope :guests, -> { where(project_access: GUEST) } - scope :reporters, -> { where(project_access: REPORTER) } - scope :developers, -> { where(project_access: DEVELOPER) } - scope :masters, -> { where(project_access: MASTER) } - - scope :in_project, ->(project) { where(project_id: project.id) } - scope :in_projects, ->(projects) { where(project_id: projects.map { |p| p.id }) } - scope :with_user, ->(user) { where(user_id: user.id) } - - after_create :post_create_hook - after_update :post_update_hook - after_destroy :post_destroy_hook - - class << self - - # Add users to project teams with passed access option - # - # access can be an integer representing a access code - # or symbol like :master representing role - # - # Ex. - # add_users_into_projects( - # project_ids, - # user_ids, - # UsersProject::MASTER - # ) - # - # add_users_into_projects( - # project_ids, - # user_ids, - # :master - # ) - # - def add_users_into_projects(project_ids, user_ids, access) - project_access = if roles_hash.has_key?(access) - roles_hash[access] - elsif roles_hash.values.include?(access.to_i) - access - else - raise "Non valid access" - end - - UsersProject.transaction do - project_ids.each do |project_id| - user_ids.each do |user_id| - users_project = UsersProject.new(project_access: project_access, user_id: user_id) - users_project.project_id = project_id - users_project.save - end - end - end - - true - rescue - false - end - - def truncate_teams(project_ids) - UsersProject.transaction do - users_projects = UsersProject.where(project_id: project_ids) - users_projects.each do |users_project| - users_project.destroy - end - end - - true - rescue - false - end - - def truncate_team project - truncate_teams [project.id] - end - - def roles_hash - Gitlab::Access.sym_options - end - - def access_roles - Gitlab::Access.options - end - end - - def access_field - project_access - end - - def owner? - project.owner == user - end - - def post_create_hook - Event.create( - project_id: self.project.id, - action: Event::JOINED, - author_id: self.user.id - ) - - notification_service.new_team_member(self) unless owner? - system_hook_service.execute_hooks_for(self, :create) - end - - def post_update_hook - notification_service.update_team_member(self) if self.project_access_changed? - end - - def post_destroy_hook - Event.create( - project_id: self.project.id, - action: Event::LEFT, - author_id: self.user.id - ) - - system_hook_service.execute_hooks_for(self, :destroy) - end - - def notification_service - NotificationService.new - end - - def system_hook_service - SystemHooksService.new - end -end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 36d33e0d7ca..ca483e2f109 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -158,19 +158,19 @@ class NotificationService end def new_team_member(users_project) - mailer.project_access_granted_email(users_project.id) + mailer.access_level_granted_email(users_project.id) end def update_team_member(users_project) - mailer.project_access_granted_email(users_project.id) + mailer.access_level_granted_email(users_project.id) end def new_group_member(users_group) - mailer.group_access_granted_email(users_group.id) + mailer.access_level_granted_email(users_group.id) end def update_group_member(users_group) - mailer.group_access_granted_email(users_group.id) + mailer.access_level_granted_email(users_group.id) end def project_was_moved(project) @@ -199,7 +199,7 @@ class NotificationService end def users_project_notification(project, notification_level=nil) - project_members = project.users_projects + project_members = project.project_members if notification_level project_members.where(notification_level: notification_level).pluck(:user_id) @@ -210,7 +210,7 @@ class NotificationService def users_group_notification(project, notification_level) if project.group - project.group.users_groups.where(notification_level: notification_level).pluck(:user_id) + project.group.group_members.where(notification_level: notification_level).pluck(:user_id) else [] end @@ -267,10 +267,10 @@ class NotificationService users.reject do |user| next user.notification.disabled? unless project - tm = project.users_projects.find_by(user_id: user.id) + tm = project.project_members.find_by(user_id: user.id) if !tm && project.group - tm = project.group.users_groups.find_by(user_id: user.id) + tm = project.group.group_members.find_by(user_id: user.id) end # reject users who globally disabled notification and has no membership diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index 2bfb0f28d95..12386792aab 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -42,10 +42,7 @@ module Projects system_hook_service.execute_hooks_for(@project, :create) unless @project.group - @project.users_projects.create( - project_access: UsersProject::MASTER, - user: current_user - ) + @project.team << [current_user, :master] end @project.update_column(:last_activity_at, @project.created_at) diff --git a/app/services/projects/fork_service.rb b/app/services/projects/fork_service.rb index 66f0a02f0af..a59311bf942 100644 --- a/app/services/projects/fork_service.rb +++ b/app/services/projects/fork_service.rb @@ -27,7 +27,7 @@ module Projects #First save the DB entries as they can be rolled back if the repo fork fails project.build_forked_project_link(forked_to_project_id: project.id, forked_from_project_id: @from_project.id) if project.save - project.users_projects.create(project_access: UsersProject::MASTER, user: current_user) + project.team << [current_user, :master] end #Now fork the repo unless gitlab_shell.fork_repository(@from_project.path_with_namespace, project.namespace.path) diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb index bfc725e5eb5..7de85c51805 100644 --- a/app/services/system_hooks_service.rb +++ b/app/services/system_hooks_service.rb @@ -50,14 +50,14 @@ class SystemHooksService email: model.email, user_id: model.id }) - when UsersProject + when ProjectMember data.merge!({ project_name: model.project.name, project_path: model.project.path, project_id: model.project_id, user_name: model.user.name, user_email: model.user.email, - project_access: model.human_access, + access_level: model.human_access, project_visibility: Project.visibility_levels.key(model.project.visibility_level_field).downcase }) end @@ -65,7 +65,7 @@ class SystemHooksService def build_event_name(model, event) case model - when UsersProject + when ProjectMember return "user_add_to_team" if event == :create return "user_remove_from_team" if event == :destroy else diff --git a/app/views/admin/groups/show.html.haml b/app/views/admin/groups/show.html.haml index 8634f46053d..d7ccd67dac7 100644 --- a/app/views/admin/groups/show.html.haml +++ b/app/views/admin/groups/show.html.haml @@ -62,7 +62,7 @@ %div = users_select_tag(:user_ids, multiple: true) %div.prepend-top-10 - = select_tag :group_access, options_for_select(UsersGroup.group_access_roles), class: "project-access-select select2" + = select_tag :access_level, options_for_select(GroupMember.access_level_roles), class: "project-access-select select2" %hr = submit_tag 'Add users into group', class: "btn btn-create" .panel.panel-default @@ -70,7 +70,7 @@ %h3.panel-title Members %span.badge - #{@group.users_groups.count} + #{@group.group_members.count} %ul.well-list.group-users-list - @members.each do |member| - user = member.user diff --git a/app/views/admin/projects/show.html.haml b/app/views/admin/projects/show.html.haml index 66a72449f40..7a1fc06d3ac 100644 --- a/app/views/admin/projects/show.html.haml +++ b/app/views/admin/projects/show.html.haml @@ -95,7 +95,7 @@ .panel.panel-default .panel-heading %strong #{@group.name} - group members (#{@group.users_groups.count}) + group members (#{@group.group_members.count}) .pull-right = link_to admin_group_path(@group), class: 'btn btn-small' do %i.icon-edit @@ -117,7 +117,7 @@ %ul.well-list.team_members - @project_members.each do |users_project| - user = users_project.user - %li.users_project + %li.project_member .list-item-name %strong = link_to user.name, admin_user_path(user) diff --git a/app/views/admin/users/show.html.haml b/app/views/admin/users/show.html.haml index f60d40b5334..a172d12edb1 100644 --- a/app/views/admin/users/show.html.haml +++ b/app/views/admin/users/show.html.haml @@ -159,13 +159,13 @@ = render 'users/profile', user: @user #groups.tab-pane - - if @user.users_groups.present? + - if @user.group_members.present? .panel.panel-default .panel-heading Groups: %ul.well-list - - @user.users_groups.each do |user_group| + - @user.group_members.each do |user_group| - group = user_group.group - %li.users_group + %li.group_member %span{class: ("list-item-name" unless user_group.owner?)} %strong= link_to group.name, admin_group_path(group) .pull-right @@ -197,7 +197,7 @@ %ul.well-list - @joined_projects.sort_by(&:name_with_namespace).each do |project| - tm = project.team.find_tm(@user.id) - %li.users_project + %li.project_member .list-item-name = link_to admin_project_path(project), class: dom_class(project) do = project.name_with_namespace diff --git a/app/views/groups/_new_group_member.html.haml b/app/views/groups/_new_group_member.html.haml index 3ab9276c541..2938a15c608 100644 --- a/app/views/groups/_new_group_member.html.haml +++ b/app/views/groups/_new_group_member.html.haml @@ -4,8 +4,8 @@ .col-sm-10= users_select_tag(:user_ids, multiple: true, class: 'input-large') .form-group - = f.label :group_access, "Group Access", class: 'control-label' - .col-sm-10= select_tag :group_access, options_for_select(UsersGroup.group_access_roles, @users_group.group_access), class: "project-access-select select2" + = f.label :access_level, "Group Access", class: 'control-label' + .col-sm-10= select_tag :access_level, options_for_select(GroupMember.access_level_roles, @users_group.access_level), class: "project-access-select select2" .form-actions = f.submit 'Add users into group', class: "btn btn-create" diff --git a/app/views/profiles/notifications/_settings.html.haml b/app/views/profiles/notifications/_settings.html.haml index 218d51d31af..940c553d1ba 100644 --- a/app/views/profiles/notifications/_settings.html.haml +++ b/app/views/profiles/notifications/_settings.html.haml @@ -6,7 +6,7 @@ = notification_icon(notification) %span.str-truncated - - if membership.kind_of? UsersGroup + - if membership.kind_of? GroupMember = link_to membership.group.name, membership.group - else = link_to_project(membership.project) diff --git a/app/views/projects/team_members/_form.html.haml b/app/views/projects/team_members/_form.html.haml index 5998e4c6b42..7e3fe02e198 100644 --- a/app/views/projects/team_members/_form.html.haml +++ b/app/views/projects/team_members/_form.html.haml @@ -16,8 +16,8 @@ %p 2. Set access level for them .form-group - = f.label :project_access, "Project Access", class: 'control-label' - .col-sm-10= select_tag :project_access, options_for_select(Gitlab::Access.options, @user_project_relation.project_access), class: "project-access-select select2" + = f.label :access_level, "Project Access", class: 'control-label' + .col-sm-10= select_tag :access_level, options_for_select(Gitlab::Access.options, @user_project_relation.access_level), class: "project-access-select select2" .form-actions = f.submit 'Add users', class: "btn btn-create" diff --git a/app/views/projects/team_members/_group_members.html.haml b/app/views/projects/team_members/_group_members.html.haml index 83c4b6f87d5..c86648138c7 100644 --- a/app/views/projects/team_members/_group_members.html.haml +++ b/app/views/projects/team_members/_group_members.html.haml @@ -1,4 +1,4 @@ -- group_users_count = @group.users_groups.count +- group_users_count = @group.group_members.count .panel.panel-default .panel-heading %strong #{@group.name} @@ -7,7 +7,7 @@ = link_to members_group_path(@group), class: 'btn btn-small' do %i.icon-edit %ul.well-list - - @group.users_groups.order('group_access DESC').limit(20).each do |member| + - @group.group_members.order('access_level DESC').limit(20).each do |member| = render 'users_groups/users_group', member: member, show_controls: false - if group_users_count > 20 %li diff --git a/app/views/projects/team_members/_team_member.html.haml b/app/views/projects/team_members/_team_member.html.haml index d93bb44ab96..3ecbf129083 100644 --- a/app/views/projects/team_members/_team_member.html.haml +++ b/app/views/projects/team_members/_team_member.html.haml @@ -5,7 +5,7 @@ - unless @project.personal? && user == current_user .pull-left = form_for(member, as: :team_member, url: project_team_member_path(@project, member.user)) do |f| - = f.select :project_access, options_for_select(UsersProject.access_roles, member.project_access), {}, class: "medium project-access-select span2 trigger-submit" + = f.select :access_level, options_for_select(ProjectMember.access_roles, member.access_level), {}, class: "medium project-access-select span2 trigger-submit"   = link_to project_team_member_path(@project, user), data: { confirm: remove_from_project_team_message(@project, user)}, method: :delete, class: "btn-tiny btn btn-remove", title: 'Remove user from team' do %i.icon-minus.icon-white diff --git a/app/views/users_groups/_users_group.html.haml b/app/views/users_groups/_users_group.html.haml index ad363eaba23..7e246d3b7fb 100644 --- a/app/views/users_groups/_users_group.html.haml +++ b/app/views/users_groups/_users_group.html.haml @@ -27,5 +27,5 @@ .edit-member.hide.js-toggle-content = form_for [@group, member], remote: true do |f| .alert.prepend-top-20 - = f.select :group_access, options_for_select(UsersGroup.group_access_roles, member.group_access) + = f.select :access_level, options_for_select(GroupMember.access_level_roles, member.access_level) = f.submit 'Save', class: 'btn btn-save btn-small' diff --git a/config/application.rb b/config/application.rb index 68dce05fc59..a4234e8a264 100644 --- a/config/application.rb +++ b/config/application.rb @@ -12,7 +12,11 @@ module Gitlab # -- all .rb files in that directory are automatically loaded. # Custom directories with classes and modules you want to be autoloadable. - config.autoload_paths += %W(#{config.root}/lib #{config.root}/app/finders #{config.root}/app/models/concerns #{config.root}/app/models/project_services) + config.autoload_paths += %W(#{config.root}/lib + #{config.root}/app/finders + #{config.root}/app/models/concerns + #{config.root}/app/models/project_services + #{config.root}/app/models/members) # Only load the plugins named here, in the order given (default is alphabetical). # :all can be used as a placeholder for all plugins not explicitly named. diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 74fdef93543..d012a6056f5 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -54,7 +54,7 @@ module API class ProjectMember < UserBasic expose :project_access, as: :access_level do |user, options| - options[:project].users_projects.find_by(user_id: user.id).project_access + options[:project].project_members.find_by(user_id: user.id).project_access end end @@ -68,7 +68,7 @@ module API class GroupMember < UserBasic expose :group_access, as: :access_level do |user, options| - options[:group].users_groups.find_by(user_id: user.id).group_access + options[:group].group_members.find_by(user_id: user.id).group_access end end @@ -182,12 +182,12 @@ module API class ProjectWithAccess < Project expose :permissions do expose :project_access, using: Entities::ProjectAccess do |project, options| - project.users_projects.find_by(user_id: options[:user].id) + project.project_members.find_by(user_id: options[:user].id) end expose :group_access, using: Entities::GroupAccess do |project, options| if project.group - project.group.users_groups.find_by(user_id: options[:user].id) + project.group.group_members.find_by(user_id: options[:user].id) end end end diff --git a/lib/api/groups.rb b/lib/api/groups.rb index caa2ca97a3e..4841e04689d 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -104,7 +104,7 @@ module API # GET /groups/:id/members get ":id/members" do group = find_group(params[:id]) - members = group.users_groups + members = group.group_members users = (paginate members).collect(&:user) present users, with: Entities::GroupMember, group: group end @@ -123,11 +123,11 @@ module API render_api_error!("Wrong access level", 422) end group = find_group(params[:id]) - if group.users_groups.find_by(user_id: params[:user_id]) + if group.group_members.find_by(user_id: params[:user_id]) render_api_error!("Already exists", 409) end group.add_users([params[:user_id]], params[:access_level]) - member = group.users_groups.find_by(user_id: params[:user_id]) + member = group.group_members.find_by(user_id: params[:user_id]) present member.user, with: Entities::GroupMember, group: group end @@ -141,7 +141,7 @@ module API # DELETE /groups/:id/members/:user_id delete ":id/members/:user_id" do group = find_group(params[:id]) - member = group.users_groups.find_by(user_id: params[:user_id]) + member = group.group_members.find_by(user_id: params[:user_id]) if member.nil? render_api_error!("404 Not Found - user_id:#{params[:user_id]} not a member of group #{group.name}",404) else diff --git a/lib/api/project_members.rb b/lib/api/project_members.rb index 47c4ddce163..9e2b12b1311 100644 --- a/lib/api/project_members.rb +++ b/lib/api/project_members.rb @@ -56,7 +56,7 @@ module API # either the user is already a team member or a new one team_member = user_project.team_member_by_id(params[:user_id]) if team_member.nil? - team_member = user_project.users_projects.new( + team_member = user_project.project_members.new( user_id: params[:user_id], project_access: params[:access_level] ) @@ -82,7 +82,7 @@ module API authorize! :admin_project, user_project required_attributes! [:access_level] - team_member = user_project.users_projects.find_by(user_id: params[:user_id]) + team_member = user_project.project_members.find_by(user_id: params[:user_id]) not_found!("User can not be found") if team_member.nil? if team_member.update_attributes(project_access: params[:access_level]) @@ -102,7 +102,7 @@ module API # DELETE /projects/:id/members/:user_id delete ":id/members/:user_id" do authorize! :admin_project, user_project - team_member = user_project.users_projects.find_by(user_id: params[:user_id]) + team_member = user_project.project_members.find_by(user_id: params[:user_id]) unless team_member.nil? team_member.destroy else diff --git a/lib/gitlab/access.rb b/lib/gitlab/access.rb index 87f9cfab608..411b2b9a3cc 100644 --- a/lib/gitlab/access.rb +++ b/lib/gitlab/access.rb @@ -16,6 +16,10 @@ module Gitlab options.values end + def all_values + options_with_owner.values + end + def options { "Guest" => GUEST, diff --git a/lib/tasks/gitlab/bulk_add_permission.rake b/lib/tasks/gitlab/bulk_add_permission.rake index 0e1a3d071e9..3d8c171dfa3 100644 --- a/lib/tasks/gitlab/bulk_add_permission.rake +++ b/lib/tasks/gitlab/bulk_add_permission.rake @@ -7,10 +7,10 @@ namespace :gitlab do projects_ids = Project.pluck(:id) puts "Importing #{user_ids.size} users into #{projects_ids.size} projects" - UsersProject.add_users_into_projects(projects_ids, user_ids, UsersProject::DEVELOPER) + ProjectMember.add_users_into_projects(projects_ids, user_ids, ProjectMember::DEVELOPER) puts "Importing #{admin_ids.size} admins into #{projects_ids.size} projects" - UsersProject.add_users_into_projects(projects_ids, admin_ids, UsersProject::MASTER) + ProjectMember.add_users_into_projects(projects_ids, admin_ids, ProjectMember::MASTER) end desc "GITLAB | Add a specific user to all projects (as a developer)" @@ -18,7 +18,7 @@ namespace :gitlab do user = User.find_by(email: args.email) project_ids = Project.pluck(:id) puts "Importing #{user.email} users into #{project_ids.size} projects" - UsersProject.add_users_into_projects(project_ids, Array.wrap(user.id), UsersProject::DEVELOPER) + ProjectMember.add_users_into_projects(project_ids, Array.wrap(user.id), ProjectMember::DEVELOPER) end desc "GITLAB | Add all users to all groups (admin users are added as owners)" @@ -30,8 +30,8 @@ namespace :gitlab do puts "Importing #{user_ids.size} users into #{groups.size} groups" puts "Importing #{admin_ids.size} admins into #{groups.size} groups" groups.each do |group| - group.add_users(user_ids, UsersGroup::DEVELOPER) - group.add_users(admin_ids, UsersGroup::OWNER) + group.add_users(user_ids, GroupMember::DEVELOPER) + group.add_users(admin_ids, GroupMember::OWNER) end end @@ -41,7 +41,7 @@ namespace :gitlab do groups = Group.all puts "Importing #{user.email} users into #{groups.size} groups" groups.each do |group| - group.add_users(Array.wrap(user.id), UsersGroup::DEVELOPER) + group.add_users(Array.wrap(user.id), GroupMember::DEVELOPER) end end end diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake index 9ea5c55abd6..3629f1d5353 100644 --- a/lib/tasks/gitlab/check.rake +++ b/lib/tasks/gitlab/check.rake @@ -195,12 +195,12 @@ namespace :gitlab do end def check_orphaned_users_groups - print "Database contains orphaned UsersGroups? ... " - if UsersGroup.where("user_id not in (select id from users)").count > 0 + print "Database contains orphaned GroupMembers? ... " + if GroupMember.where("user_id not in (select id from users)").count > 0 puts "yes".red try_fixing_it( "You can delete the orphaned records using something along the lines of:", - sudo_gitlab("bundle exec rails runner -e production 'UsersGroup.where(\"user_id NOT IN (SELECT id FROM users)\").delete_all'") + sudo_gitlab("bundle exec rails runner -e production 'GroupMember.where(\"user_id NOT IN (SELECT id FROM users)\").delete_all'") ) else puts "no".green diff --git a/spec/factories.rb b/spec/factories.rb index f7f65bffb8b..0bf56fe84d6 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -42,7 +42,7 @@ FactoryGirl.define do factory :users_project do user project - project_access { UsersProject::MASTER } + project_access { ProjectMember::MASTER } end factory :issue do diff --git a/spec/factories/users_groups.rb b/spec/factories/users_groups.rb index 49c3a367e16..931f6a25575 100644 --- a/spec/factories/users_groups.rb +++ b/spec/factories/users_groups.rb @@ -13,7 +13,7 @@ FactoryGirl.define do factory :users_group do - group_access { UsersGroup::OWNER } + group_access { GroupMember::OWNER } group user end diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb index ba6af6f8b45..46337f8bafd 100644 --- a/spec/helpers/gitlab_markdown_helper_spec.rb +++ b/spec/helpers/gitlab_markdown_helper_spec.rb @@ -12,7 +12,7 @@ describe GitlabMarkdownHelper do let(:issue) { create(:issue, project: project) } let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } let(:snippet) { create(:project_snippet, project: project) } - let(:member) { project.users_projects.where(user_id: user).first } + let(:member) { project.project_members.where(user_id: user).first } before do # Helper expects a @project instance variable diff --git a/spec/models/group_member_spec.rb b/spec/models/group_member_spec.rb new file mode 100644 index 00000000000..9eb4859ebd5 --- /dev/null +++ b/spec/models/group_member_spec.rb @@ -0,0 +1,44 @@ +# == Schema Information +# +# Table name: users_groups +# +# id :integer not null, primary key +# group_access :integer not null +# group_id :integer not null +# user_id :integer not null +# created_at :datetime +# updated_at :datetime +# notification_level :integer default(3), not null +# + +require 'spec_helper' + +describe GroupMember do + context 'notification' do + describe "#after_create" do + it "should send email to user" do + membership = build(:users_group) + membership.stub(notification_service: double('NotificationService').as_null_object) + membership.should_receive(:notification_service) + membership.save + end + end + + describe "#after_update" do + before do + @membership = create :users_group + @membership.stub(notification_service: double('NotificationService').as_null_object) + end + + it "should send email to user" do + @membership.should_receive(:notification_service) + @membership.update_attribute(:group_access, GroupMember::MASTER) + end + + it "does not send an email when the access level has not changed" do + @membership.should_not_receive(:notification_service) + @membership.update_attribute(:group_access, GroupMember::OWNER) + end + end + end +end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 8259ed88d83..006a8402d00 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -39,26 +39,26 @@ describe Group do describe :add_users do let(:user) { create(:user) } - before { group.add_user(user, UsersGroup::MASTER) } + before { group.add_user(user, GroupMember::MASTER) } - it { group.users_groups.masters.map(&:user).should include(user) } + it { group.group_members.masters.map(&:user).should include(user) } end describe :add_users do let(:user) { create(:user) } - before { group.add_users([user.id], UsersGroup::GUEST) } + before { group.add_users([user.id], GroupMember::GUEST) } it "should update the group permission" do - group.users_groups.guests.map(&:user).should include(user) - group.add_users([user.id], UsersGroup::DEVELOPER) - group.users_groups.developers.map(&:user).should include(user) - group.users_groups.guests.map(&:user).should_not include(user) + group.group_members.guests.map(&:user).should include(user) + group.add_users([user.id], GroupMember::DEVELOPER) + group.group_members.developers.map(&:user).should include(user) + group.group_members.guests.map(&:user).should_not include(user) end end describe :avatar_type do let(:user) { create(:user) } - before { group.add_user(user, UsersGroup::MASTER) } + before { group.add_user(user, GroupMember::MASTER) } it "should be true if avatar is image" do group.update_attribute(:avatar, 'uploads/avatar.png') diff --git a/spec/models/members_spec.rb b/spec/models/members_spec.rb index 33e97686654..6866c4794c2 100644 --- a/spec/models/members_spec.rb +++ b/spec/models/members_spec.rb @@ -12,4 +12,9 @@ describe Member do it { should validate_presence_of(:source) } it { should ensure_inclusion_of(:access_level).in_array(Gitlab::Access.values) } end + + describe "Delegate methods" do + it { should respond_to(:user_name) } + it { should respond_to(:user_email) } + end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index d06dee6ce92..a6796d4ab15 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -321,8 +321,8 @@ describe Note do describe :read do before do - @p1.users_projects.create(user: @u2, project_access: UsersProject::GUEST) - @p2.users_projects.create(user: @u3, project_access: UsersProject::GUEST) + @p1.project_members.create(user: @u2, project_access: ProjectMember::GUEST) + @p2.project_members.create(user: @u3, project_access: ProjectMember::GUEST) end it { @abilities.allowed?(@u1, :read_note, @p1).should be_false } @@ -332,8 +332,8 @@ describe Note do describe :write do before do - @p1.users_projects.create(user: @u2, project_access: UsersProject::DEVELOPER) - @p2.users_projects.create(user: @u3, project_access: UsersProject::DEVELOPER) + @p1.project_members.create(user: @u2, project_access: ProjectMember::DEVELOPER) + @p2.project_members.create(user: @u3, project_access: ProjectMember::DEVELOPER) end it { @abilities.allowed?(@u1, :write_note, @p1).should be_false } @@ -343,9 +343,9 @@ describe Note do describe :admin do before do - @p1.users_projects.create(user: @u1, project_access: UsersProject::REPORTER) - @p1.users_projects.create(user: @u2, project_access: UsersProject::MASTER) - @p2.users_projects.create(user: @u3, project_access: UsersProject::MASTER) + @p1.project_members.create(user: @u1, project_access: ProjectMember::REPORTER) + @p1.project_members.create(user: @u2, project_access: ProjectMember::MASTER) + @p2.project_members.create(user: @u3, project_access: ProjectMember::MASTER) end it { @abilities.allowed?(@u1, :admin_note, @p1).should be_false } diff --git a/spec/models/project_member_spec.rb b/spec/models/project_member_spec.rb new file mode 100644 index 00000000000..76c5437a555 --- /dev/null +++ b/spec/models/project_member_spec.rb @@ -0,0 +1,90 @@ +# == Schema Information +# +# Table name: users_projects +# +# id :integer not null, primary key +# user_id :integer not null +# project_id :integer not null +# created_at :datetime +# updated_at :datetime +# project_access :integer default(0), not null +# notification_level :integer default(3), not null +# + +require 'spec_helper' + +describe ProjectMember do + describe :import_team do + before do + @abilities = Six.new + @abilities << Ability + + @project_1 = create :project + @project_2 = create :project + + @user_1 = create :user + @user_2 = create :user + + @project_1.team << [ @user_1, :developer ] + @project_2.team << [ @user_2, :reporter ] + + @status = @project_2.team.import(@project_1) + end + + it { @status.should be_true } + + describe 'project 2 should get user 1 as developer. user_2 should not be changed' do + it { @project_2.users.should include(@user_1) } + it { @project_2.users.should include(@user_2) } + + it { @abilities.allowed?(@user_1, :write_project, @project_2).should be_true } + it { @abilities.allowed?(@user_2, :read_project, @project_2).should be_true } + end + + describe 'project 1 should not be changed' do + it { @project_1.users.should include(@user_1) } + it { @project_1.users.should_not include(@user_2) } + end + end + + describe :add_users_into_projects do + before do + @project_1 = create :project + @project_2 = create :project + + @user_1 = create :user + @user_2 = create :user + + ProjectMember.add_users_into_projects( + [@project_1.id, @project_2.id], + [@user_1.id, @user_2.id], + ProjectMember::MASTER + ) + end + + it { @project_1.users.should include(@user_1) } + it { @project_1.users.should include(@user_2) } + + + it { @project_2.users.should include(@user_1) } + it { @project_2.users.should include(@user_2) } + end + + describe :truncate_teams do + before do + @project_1 = create :project + @project_2 = create :project + + @user_1 = create :user + @user_2 = create :user + + @project_1.team << [ @user_1, :developer] + @project_2.team << [ @user_2, :reporter] + + ProjectMember.truncate_teams([@project_1.id, @project_2.id]) + end + + it { @project_1.users.should be_empty } + it { @project_2.users.should be_empty } + end +end diff --git a/spec/models/project_security_spec.rb b/spec/models/project_security_spec.rb index 1f2bd7a56ff..564edaf8c1b 100644 --- a/spec/models/project_security_spec.rb +++ b/spec/models/project_security_spec.rb @@ -30,7 +30,7 @@ describe Project do describe "Guest Rules" do before do - @p1.users_projects.create(project: @p1, user: @u2, project_access: UsersProject::GUEST) + @p1.project_members.create(project: @p1, user: @u2, project_access: ProjectMember::GUEST) end it "should allow for project user any guest actions" do @@ -42,7 +42,7 @@ describe Project do describe "Report Rules" do before do - @p1.users_projects.create(project: @p1, user: @u2, project_access: UsersProject::REPORTER) + @p1.project_members.create(project: @p1, user: @u2, project_access: ProjectMember::REPORTER) end it "should allow for project user any report actions" do @@ -54,8 +54,8 @@ describe Project do describe "Developer Rules" do before do - @p1.users_projects.create(project: @p1, user: @u2, project_access: UsersProject::REPORTER) - @p1.users_projects.create(project: @p1, user: @u3, project_access: UsersProject::DEVELOPER) + @p1.project_members.create(project: @p1, user: @u2, project_access: ProjectMember::REPORTER) + @p1.project_members.create(project: @p1, user: @u3, project_access: ProjectMember::DEVELOPER) end it "should deny for developer master-specific actions" do @@ -73,8 +73,8 @@ describe Project do describe "Master Rules" do before do - @p1.users_projects.create(project: @p1, user: @u2, project_access: UsersProject::DEVELOPER) - @p1.users_projects.create(project: @p1, user: @u3, project_access: UsersProject::MASTER) + @p1.project_members.create(project: @p1, user: @u2, project_access: ProjectMember::DEVELOPER) + @p1.project_members.create(project: @p1, user: @u3, project_access: ProjectMember::MASTER) end it "should deny for developer master-specific actions" do @@ -92,8 +92,8 @@ describe Project do describe "Admin Rules" do before do - @p1.users_projects.create(project: @p1, user: @u2, project_access: UsersProject::DEVELOPER) - @p1.users_projects.create(project: @p1, user: @u3, project_access: UsersProject::MASTER) + @p1.project_members.create(project: @p1, user: @u2, project_access: ProjectMember::DEVELOPER) + @p1.project_members.create(project: @p1, user: @u3, project_access: ProjectMember::MASTER) end it "should deny for masters admin-specific actions" do diff --git a/spec/models/system_hook_spec.rb b/spec/models/system_hook_spec.rb index 2b98acdeb6c..4ab5261dc9d 100644 --- a/spec/models/system_hook_spec.rb +++ b/spec/models/system_hook_spec.rb @@ -58,7 +58,7 @@ describe SystemHook do user = create(:user) project = create(:project) project.team << [user, :master] - project.users_projects.destroy_all + project.project_members.destroy_all WebMock.should have_requested(:post, @system_hook.url).with(body: /user_remove_from_team/).once end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 7221328a45f..f5c42f7cb2d 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -182,7 +182,7 @@ describe User do @group = create :group @group.add_owner(@user) - @group.add_user(@user2, UsersGroup::OWNER) + @group.add_user(@user2, GroupMember::OWNER) end it { @user2.several_namespaces?.should be_true } diff --git a/spec/models/users_group_spec.rb b/spec/models/users_group_spec.rb deleted file mode 100644 index 0b6f7a08198..00000000000 --- a/spec/models/users_group_spec.rb +++ /dev/null @@ -1,67 +0,0 @@ -# == Schema Information -# -# Table name: users_groups -# -# id :integer not null, primary key -# group_access :integer not null -# group_id :integer not null -# user_id :integer not null -# created_at :datetime -# updated_at :datetime -# notification_level :integer default(3), not null -# - -require 'spec_helper' - -describe UsersGroup do - describe "Associations" do - it { should belong_to(:group) } - it { should belong_to(:user) } - end - - describe "Mass assignment" do - end - - describe "Validation" do - let!(:users_group) { create(:users_group) } - - it { should validate_presence_of(:user_id) } - it { should validate_uniqueness_of(:user_id).scoped_to(:group_id).with_message(/already exists/) } - - it { should validate_presence_of(:group_id) } - it { should ensure_inclusion_of(:group_access).in_array(UsersGroup.group_access_roles.values) } - end - - describe "Delegate methods" do - it { should respond_to(:user_name) } - it { should respond_to(:user_email) } - end - - context 'notification' do - describe "#after_create" do - it "should send email to user" do - membership = build(:users_group) - membership.stub(notification_service: double('NotificationService').as_null_object) - membership.should_receive(:notification_service) - membership.save - end - end - - describe "#after_update" do - before do - @membership = create :users_group - @membership.stub(notification_service: double('NotificationService').as_null_object) - end - - it "should send email to user" do - @membership.should_receive(:notification_service) - @membership.update_attribute(:group_access, UsersGroup::MASTER) - end - - it "does not send an email when the access level has not changed" do - @membership.should_not_receive(:notification_service) - @membership.update_attribute(:group_access, UsersGroup::OWNER) - end - end - end -end diff --git a/spec/models/users_project_spec.rb b/spec/models/users_project_spec.rb deleted file mode 100644 index 3f38164e964..00000000000 --- a/spec/models/users_project_spec.rb +++ /dev/null @@ -1,113 +0,0 @@ -# == Schema Information -# -# Table name: users_projects -# -# id :integer not null, primary key -# user_id :integer not null -# project_id :integer not null -# created_at :datetime -# updated_at :datetime -# project_access :integer default(0), not null -# notification_level :integer default(3), not null -# - -require 'spec_helper' - -describe UsersProject do - describe "Associations" do - it { should belong_to(:project) } - it { should belong_to(:user) } - end - - describe "Mass assignment" do - end - - describe "Validation" do - let!(:users_project) { create(:users_project) } - - it { should validate_presence_of(:user) } - it { should validate_uniqueness_of(:user_id).scoped_to(:project_id).with_message(/already exists/) } - - it { should validate_presence_of(:project) } - it { should ensure_inclusion_of(:project_access).in_array(UsersProject.access_roles.values) } - end - - describe "Delegate methods" do - it { should respond_to(:user_name) } - it { should respond_to(:user_email) } - end - - describe :import_team do - before do - @abilities = Six.new - @abilities << Ability - - @project_1 = create :project - @project_2 = create :project - - @user_1 = create :user - @user_2 = create :user - - @project_1.team << [ @user_1, :developer ] - @project_2.team << [ @user_2, :reporter ] - - @status = @project_2.team.import(@project_1) - end - - it { @status.should be_true } - - describe 'project 2 should get user 1 as developer. user_2 should not be changed' do - it { @project_2.users.should include(@user_1) } - it { @project_2.users.should include(@user_2) } - - it { @abilities.allowed?(@user_1, :write_project, @project_2).should be_true } - it { @abilities.allowed?(@user_2, :read_project, @project_2).should be_true } - end - - describe 'project 1 should not be changed' do - it { @project_1.users.should include(@user_1) } - it { @project_1.users.should_not include(@user_2) } - end - end - - describe :add_users_into_projects do - before do - @project_1 = create :project - @project_2 = create :project - - @user_1 = create :user - @user_2 = create :user - - UsersProject.add_users_into_projects( - [@project_1.id, @project_2.id], - [@user_1.id, @user_2.id], - UsersProject::MASTER - ) - end - - it { @project_1.users.should include(@user_1) } - it { @project_1.users.should include(@user_2) } - - - it { @project_2.users.should include(@user_1) } - it { @project_2.users.should include(@user_2) } - end - - describe :truncate_teams do - before do - @project_1 = create :project - @project_2 = create :project - - @user_1 = create :user - @user_2 = create :user - - @project_1.team << [ @user_1, :developer] - @project_2.team << [ @user_2, :reporter] - - UsersProject.truncate_teams([@project_1.id, @project_2.id]) - end - - it { @project_1.users.should be_empty } - it { @project_2.users.should be_empty } - end -end diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb index e7f91c5e46e..1690146aea7 100644 --- a/spec/requests/api/branches_spec.rb +++ b/spec/requests/api/branches_spec.rb @@ -7,8 +7,8 @@ describe API::API, api: true do let(:user) { create(:user) } let(:user2) { create(:user) } let!(:project) { create(:project, creator_id: user.id) } - let!(:master) { create(:users_project, user: user, project: project, project_access: UsersProject::MASTER) } - let!(:guest) { create(:users_project, user: user2, project: project, project_access: UsersProject::GUEST) } + let!(:master) { create(:users_project, user: user, project: project, project_access: ProjectMember::MASTER) } + let!(:guest) { create(:users_project, user: user2, project: project, project_access: ProjectMember::GUEST) } let!(:branch_name) { 'feature' } let!(:branch_sha) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' } diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index b56269d275d..21c0329e60f 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -6,8 +6,8 @@ describe API::API, api: true do let(:user) { create(:user) } let(:user2) { create(:user) } let!(:project) { create(:project, creator_id: user.id) } - let!(:master) { create(:users_project, user: user, project: project, project_access: UsersProject::MASTER) } - let!(:guest) { create(:users_project, user: user2, project: project, project_access: UsersProject::GUEST) } + let!(:master) { create(:users_project, user: user, project: project, project_access: ProjectMember::MASTER) } + let!(:guest) { create(:users_project, user: user2, project: project, project_access: ProjectMember::GUEST) } before { project.team << [user, :reporter] } diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index f27a60e4bc0..42ccad71aaf 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -174,10 +174,10 @@ describe API::API, api: true do let(:guest) { create(:user) } let!(:group_with_members) do group = create(:group) - group.add_users([reporter.id], UsersGroup::REPORTER) - group.add_users([developer.id], UsersGroup::DEVELOPER) - group.add_users([master.id], UsersGroup::MASTER) - group.add_users([guest.id], UsersGroup::GUEST) + group.add_users([reporter.id], GroupMember::REPORTER) + group.add_users([developer.id], GroupMember::DEVELOPER) + group.add_users([master.id], GroupMember::MASTER) + group.add_users([guest.id], GroupMember::GUEST) group end let!(:group_no_members) { create(:group) } @@ -195,11 +195,11 @@ describe API::API, api: true do response.status.should == 200 json_response.should be_an Array json_response.size.should == 5 - json_response.find { |e| e['id']==owner.id }['access_level'].should == UsersGroup::OWNER - json_response.find { |e| e['id']==reporter.id }['access_level'].should == UsersGroup::REPORTER - json_response.find { |e| e['id']==developer.id }['access_level'].should == UsersGroup::DEVELOPER - json_response.find { |e| e['id']==master.id }['access_level'].should == UsersGroup::MASTER - json_response.find { |e| e['id']==guest.id }['access_level'].should == UsersGroup::GUEST + json_response.find { |e| e['id']==owner.id }['access_level'].should == GroupMember::OWNER + json_response.find { |e| e['id']==reporter.id }['access_level'].should == GroupMember::REPORTER + json_response.find { |e| e['id']==developer.id }['access_level'].should == GroupMember::DEVELOPER + json_response.find { |e| e['id']==master.id }['access_level'].should == GroupMember::MASTER + json_response.find { |e| e['id']==guest.id }['access_level'].should == GroupMember::GUEST end end @@ -213,29 +213,29 @@ describe API::API, api: true do describe "POST /groups/:id/members" do context "when not a member of the group" do it "should not add guest as member of group_no_members when adding being done by person outside the group" do - post api("/groups/#{group_no_members.id}/members", reporter), user_id: guest.id, access_level: UsersGroup::MASTER + post api("/groups/#{group_no_members.id}/members", reporter), user_id: guest.id, access_level: GroupMember::MASTER response.status.should == 403 end end context "when a member of the group" do it "should return ok and add new member" do - count_before=group_no_members.users_groups.count + count_before=group_no_members.group_members.count new_user = create(:user) - post api("/groups/#{group_no_members.id}/members", owner), user_id: new_user.id, access_level: UsersGroup::MASTER + post api("/groups/#{group_no_members.id}/members", owner), user_id: new_user.id, access_level: GroupMember::MASTER response.status.should == 201 json_response['name'].should == new_user.name - json_response['access_level'].should == UsersGroup::MASTER - group_no_members.users_groups.count.should == count_before + 1 + json_response['access_level'].should == GroupMember::MASTER + group_no_members.group_members.count.should == count_before + 1 end it "should return error if member already exists" do - post api("/groups/#{group_with_members.id}/members", owner), user_id: master.id, access_level: UsersGroup::MASTER + post api("/groups/#{group_with_members.id}/members", owner), user_id: master.id, access_level: GroupMember::MASTER response.status.should == 409 end it "should return a 400 error when user id is not given" do - post api("/groups/#{group_no_members.id}/members", owner), access_level: UsersGroup::MASTER + post api("/groups/#{group_no_members.id}/members", owner), access_level: GroupMember::MASTER response.status.should == 400 end @@ -262,10 +262,10 @@ describe API::API, api: true do context "when a member of the group" do it "should delete guest's membership of group" do - count_before=group_with_members.users_groups.count + count_before=group_with_members.group_members.count delete api("/groups/#{group_with_members.id}/members/#{guest.id}", owner) response.status.should == 200 - group_with_members.users_groups.count.should == count_before - 1 + group_with_members.group_members.count.should == count_before - 1 end it "should return a 404 error when user id is not known" do diff --git a/spec/requests/api/project_members_spec.rb b/spec/requests/api/project_members_spec.rb index 3c480c2ac4b..e101f871ff0 100644 --- a/spec/requests/api/project_members_spec.rb +++ b/spec/requests/api/project_members_spec.rb @@ -6,8 +6,8 @@ describe API::API, api: true do let(:user2) { create(:user) } let(:user3) { create(:user) } let(:project) { create(:project, creator_id: user.id, namespace: user.namespace) } - let(:users_project) { create(:users_project, user: user, project: project, project_access: UsersProject::MASTER) } - let(:users_project2) { create(:users_project, user: user3, project: project, project_access: UsersProject::DEVELOPER) } + let(:users_project) { create(:users_project, user: user, project: project, project_access: ProjectMember::MASTER) } + let(:users_project2) { create(:users_project, user: user3, project: project, project_access: ProjectMember::DEVELOPER) } describe "GET /projects/:id/members" do before { users_project } @@ -42,7 +42,7 @@ describe API::API, api: true do get api("/projects/#{project.id}/members/#{user.id}", user) response.status.should == 200 json_response['username'].should == user.username - json_response['access_level'].should == UsersProject::MASTER + json_response['access_level'].should == ProjectMember::MASTER end it "should return a 404 error if user id not found" do @@ -55,29 +55,29 @@ describe API::API, api: true do it "should add user to project team" do expect { post api("/projects/#{project.id}/members", user), user_id: user2.id, - access_level: UsersProject::DEVELOPER - }.to change { UsersProject.count }.by(1) + access_level: ProjectMember::DEVELOPER + }.to change { ProjectMember.count }.by(1) response.status.should == 201 json_response['username'].should == user2.username - json_response['access_level'].should == UsersProject::DEVELOPER + json_response['access_level'].should == ProjectMember::DEVELOPER end it "should return a 201 status if user is already project member" do post api("/projects/#{project.id}/members", user), user_id: user2.id, - access_level: UsersProject::DEVELOPER + access_level: ProjectMember::DEVELOPER expect { post api("/projects/#{project.id}/members", user), user_id: user2.id, - access_level: UsersProject::DEVELOPER - }.not_to change { UsersProject.count }.by(1) + access_level: ProjectMember::DEVELOPER + }.not_to change { ProjectMember.count }.by(1) response.status.should == 201 json_response['username'].should == user2.username - json_response['access_level'].should == UsersProject::DEVELOPER + json_response['access_level'].should == ProjectMember::DEVELOPER end it "should return a 400 error when user id is not given" do - post api("/projects/#{project.id}/members", user), access_level: UsersProject::MASTER + post api("/projects/#{project.id}/members", user), access_level: ProjectMember::MASTER response.status.should == 400 end @@ -96,14 +96,14 @@ describe API::API, api: true do before { users_project2 } it "should update project team member" do - put api("/projects/#{project.id}/members/#{user3.id}", user), access_level: UsersProject::MASTER + put api("/projects/#{project.id}/members/#{user3.id}", user), access_level: ProjectMember::MASTER response.status.should == 200 json_response['username'].should == user3.username - json_response['access_level'].should == UsersProject::MASTER + json_response['access_level'].should == ProjectMember::MASTER end it "should return a 404 error if user_id is not found" do - put api("/projects/#{project.id}/members/1234", user), access_level: UsersProject::MASTER + put api("/projects/#{project.id}/members/1234", user), access_level: ProjectMember::MASTER response.status.should == 404 end @@ -125,14 +125,14 @@ describe API::API, api: true do it "should remove user from project team" do expect { delete api("/projects/#{project.id}/members/#{user3.id}", user) - }.to change { UsersProject.count }.by(-1) + }.to change { ProjectMember.count }.by(-1) end it "should return 200 if team member is not part of a project" do delete api("/projects/#{project.id}/members/#{user3.id}", user) expect { delete api("/projects/#{project.id}/members/#{user3.id}", user) - }.to_not change { UsersProject.count }.by(1) + }.to_not change { ProjectMember.count }.by(1) end it "should return 200 if team member already removed" do @@ -144,7 +144,7 @@ describe API::API, api: true do it "should return 200 OK when the user was not member" do expect { delete api("/projects/#{project.id}/members/1000000", user) - }.to change { UsersProject.count }.by(0) + }.to change { ProjectMember.count }.by(0) response.status.should == 200 json_response['message'].should == "Access revoked" json_response['id'].should == 1000000 diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 058b831e783..7d36043e3d8 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -8,8 +8,8 @@ describe API::API, api: true do let(:admin) { create(:admin) } let(:project) { create(:project, creator_id: user.id, namespace: user.namespace) } let(:snippet) { create(:project_snippet, author: user, project: project, title: 'example') } - let(:users_project) { create(:users_project, user: user, project: project, project_access: UsersProject::MASTER) } - let(:users_project2) { create(:users_project, user: user3, project: project, project_access: UsersProject::DEVELOPER) } + let(:users_project) { create(:users_project, user: user, project: project, project_access: ProjectMember::MASTER) } + let(:users_project2) { create(:users_project, user: user3, project: project, project_access: ProjectMember::DEVELOPER) } describe "GET /projects" do before { project } diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index 17173aaeeac..ba404901a49 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -8,8 +8,8 @@ describe API::API, api: true do let(:user) { create(:user) } let(:user2) { create(:user) } let!(:project) { create(:project, creator_id: user.id) } - let!(:master) { create(:users_project, user: user, project: project, project_access: UsersProject::MASTER) } - let!(:guest) { create(:users_project, user: user2, project: project, project_access: UsersProject::GUEST) } + let!(:master) { create(:users_project, user: user, project: project, project_access: ProjectMember::MASTER) } + let!(:guest) { create(:users_project, user: user2, project: project, project_access: ProjectMember::GUEST) } before { project.team << [user, :reporter] } diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index df355f6f07a..f8377650e0a 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -64,12 +64,12 @@ describe NotificationService do before do note.project.namespace_id = group.id - note.project.group.add_user(@u_watcher, UsersGroup::MASTER) + note.project.group.add_user(@u_watcher, GroupMember::MASTER) note.project.save - user_project = note.project.users_projects.find_by_user_id(@u_watcher.id) + user_project = note.project.project_members.find_by_user_id(@u_watcher.id) user_project.notification_level = Notification::N_PARTICIPATING user_project.save - user_group = note.project.group.users_groups.find_by_user_id(@u_watcher.id) + user_group = note.project.group.group_members.find_by_user_id(@u_watcher.id) user_group.notification_level = Notification::N_GLOBAL user_group.save end -- cgit v1.2.1 From 8210d813812571b479d630ff61410347f8ebeb5b Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 14 Sep 2014 19:51:54 +0300 Subject: Replace old references of users_project & users_group Signed-off-by: Dmitriy Zaporozhets --- app/controllers/profiles/notifications_controller.rb | 4 ++-- app/controllers/projects/team_members_controller.rb | 2 +- app/models/project.rb | 10 +++++----- app/models/user.rb | 6 +++--- app/views/admin/projects/show.html.haml | 2 +- app/views/groups/_new_group_member.html.haml | 2 +- app/views/groups/members.html.haml | 2 +- app/views/profiles/notifications/show.html.haml | 4 ++-- app/views/projects/team_members/_group_members.html.haml | 2 +- app/views/projects/team_members/index.html.haml | 2 +- lib/tasks/gitlab/check.rake | 4 ++-- spec/factories/users_groups.rb | 2 +- spec/models/group_member_spec.rb | 2 +- spec/models/group_spec.rb | 2 +- spec/models/project_member_spec.rb | 2 +- spec/models/project_spec.rb | 2 +- spec/models/user_spec.rb | 2 +- 17 files changed, 26 insertions(+), 26 deletions(-) diff --git a/app/controllers/profiles/notifications_controller.rb b/app/controllers/profiles/notifications_controller.rb index 936fe071347..efed4a19ee3 100644 --- a/app/controllers/profiles/notifications_controller.rb +++ b/app/controllers/profiles/notifications_controller.rb @@ -3,8 +3,8 @@ class Profiles::NotificationsController < ApplicationController def show @notification = current_user.notification - @users_projects = current_user.project_members - @users_groups = current_user.group_members + @project_members = current_user.project_members + @group_members = current_user.group_members end def update diff --git a/app/controllers/projects/team_members_controller.rb b/app/controllers/projects/team_members_controller.rb index 575797d36dc..158661f66fc 100644 --- a/app/controllers/projects/team_members_controller.rb +++ b/app/controllers/projects/team_members_controller.rb @@ -6,7 +6,7 @@ class Projects::TeamMembersController < Projects::ApplicationController def index @group = @project.group - @users_projects = @project.project_members.order('access_level DESC') + @project_members = @project.project_members.order('access_level DESC') end def new diff --git a/app/models/project.rb b/app/models/project.rb index 114e40983f8..0adedaa8dcd 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -79,8 +79,8 @@ class Project < ActiveRecord::Base has_many :snippets, dependent: :destroy, class_name: "ProjectSnippet" has_many :hooks, dependent: :destroy, class_name: "ProjectHook" has_many :protected_branches, dependent: :destroy - has_many :users_projects, dependent: :destroy - has_many :users, through: :users_projects + has_many :project_members, dependent: :destroy, as: :source, class_name: 'ProjectMember' + has_many :users, through: :project_members has_many :deploy_keys_projects, dependent: :destroy has_many :deploy_keys, through: :deploy_keys_projects has_many :users_star_projects, dependent: :destroy @@ -353,12 +353,12 @@ class Project < ActiveRecord::Base def team_member_by_name_or_email(name = nil, email = nil) user = users.where("name like ? or email like ?", name, email).first - users_projects.where(user: user) if user + project_members.where(user: user) if user end # Get Team Member record by user id def team_member_by_id(user_id) - users_projects.find_by(user_id: user_id) + project_members.find_by(user_id: user_id) end def name_with_namespace @@ -555,7 +555,7 @@ class Project < ActiveRecord::Base end def project_member(user) - users_projects.where(user_id: user).first + project_members.where(user_id: user).first end def default_branch diff --git a/app/models/user.rb b/app/models/user.rb index b235437817a..ef006e07b64 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -91,13 +91,13 @@ class User < ActiveRecord::Base # Projects has_many :groups_projects, through: :groups, source: :projects has_many :personal_projects, through: :namespace, source: :projects - has_many :projects, through: :users_projects + has_many :projects, through: :project_members has_many :created_projects, foreign_key: :creator_id, class_name: 'Project' has_many :users_star_projects, dependent: :destroy has_many :starred_projects, through: :users_star_projects, source: :project has_many :snippets, dependent: :destroy, foreign_key: :author_id, class_name: "Snippet" - has_many :users_projects, dependent: :destroy + has_many :project_members, dependent: :destroy has_many :issues, dependent: :destroy, foreign_key: :author_id has_many :notes, dependent: :destroy, foreign_key: :author_id has_many :merge_requests, dependent: :destroy, foreign_key: :author_id @@ -177,7 +177,7 @@ class User < ActiveRecord::Base scope :in_team, ->(team){ where(id: team.member_ids) } scope :not_in_team, ->(team){ where('users.id NOT IN (:ids)', ids: team.member_ids) } scope :not_in_project, ->(project) { project.users.present? ? where("id not in (:ids)", ids: project.users.map(&:id) ) : all } - scope :without_projects, -> { where('id NOT IN (SELECT DISTINCT(user_id) FROM users_projects)') } + scope :without_projects, -> { where('id NOT IN (SELECT DISTINCT(user_id) FROM project_members)') } scope :ldap, -> { where(provider: 'ldap') } scope :potential_team_members, ->(team) { team.members.any? ? active.not_in_team(team) : active } diff --git a/app/views/admin/projects/show.html.haml b/app/views/admin/projects/show.html.haml index 7a1fc06d3ac..52fc5cea887 100644 --- a/app/views/admin/projects/show.html.haml +++ b/app/views/admin/projects/show.html.haml @@ -101,7 +101,7 @@ %i.icon-edit %ul.well-list - @group_members.each do |member| - = render 'users_groups/users_group', member: member, show_controls: false + = render 'group_members/users_group', member: member, show_controls: false .panel-footer = paginate @group_members, param_name: 'group_members_page', theme: 'gitlab' diff --git a/app/views/groups/_new_group_member.html.haml b/app/views/groups/_new_group_member.html.haml index 2938a15c608..e590ddbf931 100644 --- a/app/views/groups/_new_group_member.html.haml +++ b/app/views/groups/_new_group_member.html.haml @@ -1,4 +1,4 @@ -= form_for @users_group, url: group_users_groups_path(@group), html: { class: 'form-horizontal users-group-form' } do |f| += form_for @users_group, url: group_group_members_path(@group), html: { class: 'form-horizontal users-group-form' } do |f| .form-group = f.label :user_ids, "People", class: 'control-label' .col-sm-10= users_select_tag(:user_ids, multiple: true, class: 'input-large') diff --git a/app/views/groups/members.html.haml b/app/views/groups/members.html.haml index 19819c96124..7035cb95092 100644 --- a/app/views/groups/members.html.haml +++ b/app/views/groups/members.html.haml @@ -32,7 +32,7 @@ (#{@members.total_count}) %ul.well-list - @members.each do |member| - = render 'users_groups/users_group', member: member, show_roles: show_roles, show_controls: true + = render 'group_members/users_group', member: member, show_roles: show_roles, show_controls: true = paginate @members, theme: 'gitlab' :coffeescript diff --git a/app/views/profiles/notifications/show.html.haml b/app/views/profiles/notifications/show.html.haml index efe9c032190..3e28f2f5f90 100644 --- a/app/views/profiles/notifications/show.html.haml +++ b/app/views/profiles/notifications/show.html.haml @@ -39,13 +39,13 @@ .col-md-6 %h4 Groups: %ul.bordered-list - - @users_groups.each do |users_group| + - @group_members.each do |users_group| - notification = Notification.new(users_group) = render 'settings', type: 'group', membership: users_group, notification: notification .col-md-6 %h4 Projects: %ul.bordered-list - - @users_projects.each do |users_project| + - @project_members.each do |users_project| - notification = Notification.new(users_project) = render 'settings', type: 'project', membership: users_project, notification: notification diff --git a/app/views/projects/team_members/_group_members.html.haml b/app/views/projects/team_members/_group_members.html.haml index c86648138c7..837f2f5c932 100644 --- a/app/views/projects/team_members/_group_members.html.haml +++ b/app/views/projects/team_members/_group_members.html.haml @@ -8,7 +8,7 @@ %i.icon-edit %ul.well-list - @group.group_members.order('access_level DESC').limit(20).each do |member| - = render 'users_groups/users_group', member: member, show_controls: false + = render 'group_members/users_group', member: member, show_controls: false - if group_users_count > 20 %li and #{group_users_count - 20} more. For full list visit #{link_to 'group members page', members_group_path(@group)} diff --git a/app/views/projects/team_members/index.html.haml b/app/views/projects/team_members/index.html.haml index ddb3b9d4a9d..ecb7c689e8a 100644 --- a/app/views/projects/team_members/index.html.haml +++ b/app/views/projects/team_members/index.html.haml @@ -11,6 +11,6 @@ %p.light Read more about project permissions %strong= link_to "here", help_page_path("permissions", "permissions"), class: "vlink" -= render "team", members: @users_projects += render "team", members: @project_members - if @group = render "group_members" diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake index 3629f1d5353..e00844227a8 100644 --- a/lib/tasks/gitlab/check.rake +++ b/lib/tasks/gitlab/check.rake @@ -17,7 +17,7 @@ namespace :gitlab do check_database_config_exists check_database_is_not_sqlite check_migrations_are_up - check_orphaned_users_groups + check_orphaned_group_members check_gitlab_config_exists check_gitlab_config_not_outdated check_log_writable @@ -194,7 +194,7 @@ namespace :gitlab do end end - def check_orphaned_users_groups + def check_orphaned_group_members print "Database contains orphaned GroupMembers? ... " if GroupMember.where("user_id not in (select id from users)").count > 0 puts "yes".red diff --git a/spec/factories/users_groups.rb b/spec/factories/users_groups.rb index 931f6a25575..450a0d88b76 100644 --- a/spec/factories/users_groups.rb +++ b/spec/factories/users_groups.rb @@ -1,6 +1,6 @@ # == Schema Information # -# Table name: users_groups +# Table name: group_members # # id :integer not null, primary key # group_access :integer not null diff --git a/spec/models/group_member_spec.rb b/spec/models/group_member_spec.rb index 9eb4859ebd5..58205e599fe 100644 --- a/spec/models/group_member_spec.rb +++ b/spec/models/group_member_spec.rb @@ -1,6 +1,6 @@ # == Schema Information # -# Table name: users_groups +# Table name: group_members # # id :integer not null, primary key # group_access :integer not null diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 006a8402d00..1d4ba8a2b85 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -20,7 +20,7 @@ describe Group do describe "Associations" do it { should have_many :projects } - it { should have_many :users_groups } + it { should have_many :group_members } end it { should validate_presence_of :name } diff --git a/spec/models/project_member_spec.rb b/spec/models/project_member_spec.rb index 76c5437a555..0178d065e57 100644 --- a/spec/models/project_member_spec.rb +++ b/spec/models/project_member_spec.rb @@ -1,6 +1,6 @@ # == Schema Information # -# Table name: users_projects +# Table name: project_members # # id :integer not null, primary key # user_id :integer not null diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 1c11ac39567..21800ab98ff 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -38,7 +38,7 @@ describe Project do it { should have_many(:merge_requests).dependent(:destroy) } it { should have_many(:issues).dependent(:destroy) } it { should have_many(:milestones).dependent(:destroy) } - it { should have_many(:users_projects).dependent(:destroy) } + it { should have_many(:project_members).dependent(:destroy) } it { should have_many(:notes).dependent(:destroy) } it { should have_many(:snippets).class_name('ProjectSnippet').dependent(:destroy) } it { should have_many(:deploy_keys_projects).dependent(:destroy) } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index f5c42f7cb2d..0250014bc21 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -52,7 +52,7 @@ describe User do describe "Associations" do it { should have_one(:namespace) } it { should have_many(:snippets).class_name('Snippet').dependent(:destroy) } - it { should have_many(:users_projects).dependent(:destroy) } + it { should have_many(:project_members).dependent(:destroy) } it { should have_many(:groups) } it { should have_many(:keys).dependent(:destroy) } it { should have_many(:events).class_name('Event').dependent(:destroy) } -- cgit v1.2.1 From 13af7de9411ef20fcab00f7c5d7cdc6b90dbd4b7 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 14 Sep 2014 20:38:57 +0300 Subject: Fix migration and association for new members table Signed-off-by: Dmitriy Zaporozhets --- app/models/members/group_member.rb | 2 ++ app/models/members/project_member.rb | 2 ++ app/models/user.rb | 6 ++--- app/services/system_hooks_service.rb | 2 +- db/migrate/20140914113604_add_members_table.rb | 5 ++++ .../20140914145549_migrate_to_new_members_model.rb | 11 ++++++++ .../20140914145549_migrate_to_newmembers_model.rb | 31 ---------------------- .../20140914173417_remove_old_member_tables.rb | 26 ++++++++++++++++++ db/schema.rb | 31 +++++----------------- 9 files changed, 56 insertions(+), 60 deletions(-) create mode 100644 db/migrate/20140914145549_migrate_to_new_members_model.rb delete mode 100644 db/migrate/20140914145549_migrate_to_newmembers_model.rb create mode 100644 db/migrate/20140914173417_remove_old_member_tables.rb diff --git a/app/models/members/group_member.rb b/app/models/members/group_member.rb index 7f68662de70..e7eb4a18e34 100644 --- a/app/models/members/group_member.rb +++ b/app/models/members/group_member.rb @@ -1,6 +1,8 @@ class GroupMember < Member SOURCE_TYPE = 'Group' + belongs_to :group, class_name: 'Group', foreign_key: 'source_id' + # Make sure group member points only to group as it source default_value_for :source_type, SOURCE_TYPE validates_format_of :source_type, with: /\AGroup\z/ diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb index 403b87b7e36..3f8137ed06e 100644 --- a/app/models/members/project_member.rb +++ b/app/models/members/project_member.rb @@ -3,6 +3,8 @@ class ProjectMember < Member include Gitlab::ShellAdapter + belongs_to :project, class_name: 'Project', foreign_key: 'source_id' + # Make sure project member points only to project as it source default_value_for :source_type, SOURCE_TYPE validates_format_of :source_type, with: /\AProject\z/ diff --git a/app/models/user.rb b/app/models/user.rb index ef006e07b64..d5a6b468fcd 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -85,8 +85,8 @@ class User < ActiveRecord::Base has_many :project_members, source: 'ProjectMember' has_many :group_members, source: 'GroupMember' has_many :groups, through: :group_members - has_many :owned_groups, -> { where group_members: { access_level: Gitlab::Access::OWNER } }, through: :group_members, source: :group - has_many :masters_groups, -> { where group_members: { access_level: Gitlab::Access::MASTER } }, through: :group_members, source: :group + has_many :owned_groups, -> { where members: { access_level: Gitlab::Access::OWNER } }, through: :group_members, source: :group + has_many :masters_groups, -> { where members: { access_level: Gitlab::Access::MASTER } }, through: :group_members, source: :group # Projects has_many :groups_projects, through: :groups, source: :projects @@ -97,7 +97,7 @@ class User < ActiveRecord::Base has_many :starred_projects, through: :users_star_projects, source: :project has_many :snippets, dependent: :destroy, foreign_key: :author_id, class_name: "Snippet" - has_many :project_members, dependent: :destroy + has_many :project_members, dependent: :destroy, class_name: 'ProjectMember' has_many :issues, dependent: :destroy, foreign_key: :author_id has_many :notes, dependent: :destroy, foreign_key: :author_id has_many :merge_requests, dependent: :destroy, foreign_key: :author_id diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb index 7de85c51805..a6b68749a71 100644 --- a/app/services/system_hooks_service.rb +++ b/app/services/system_hooks_service.rb @@ -54,7 +54,7 @@ class SystemHooksService data.merge!({ project_name: model.project.name, project_path: model.project.path, - project_id: model.project_id, + project_id: model.project.id, user_name: model.user.name, user_email: model.user.email, access_level: model.human_access, diff --git a/db/migrate/20140914113604_add_members_table.rb b/db/migrate/20140914113604_add_members_table.rb index 1738c8dc655..d311f3033ee 100644 --- a/db/migrate/20140914113604_add_members_table.rb +++ b/db/migrate/20140914113604_add_members_table.rb @@ -10,5 +10,10 @@ class AddMembersTable < ActiveRecord::Migration t.timestamps end + + add_index :members, :type + add_index :members, :user_id + add_index :members, :access_level + add_index :members, [:source_id, :source_type] end end diff --git a/db/migrate/20140914145549_migrate_to_new_members_model.rb b/db/migrate/20140914145549_migrate_to_new_members_model.rb new file mode 100644 index 00000000000..b4f7718f26d --- /dev/null +++ b/db/migrate/20140914145549_migrate_to_new_members_model.rb @@ -0,0 +1,11 @@ +class MigrateToNewMembersModel < ActiveRecord::Migration + def up + execute "INSERT INTO members ( user_id, source_id, source_type, access_level, notification_level, type ) SELECT user_id, group_id, 'Group', group_access, notification_level, 'GroupMember' FROM users_groups" + execute "INSERT INTO members ( user_id, source_id, source_type, access_level, notification_level, type ) SELECT user_id, project_id, 'Project', project_access, notification_level, 'ProjectMember' FROM users_projects" + end + + def down + Member.delete_all + end +end + diff --git a/db/migrate/20140914145549_migrate_to_newmembers_model.rb b/db/migrate/20140914145549_migrate_to_newmembers_model.rb deleted file mode 100644 index 9c4517f95e5..00000000000 --- a/db/migrate/20140914145549_migrate_to_newmembers_model.rb +++ /dev/null @@ -1,31 +0,0 @@ -class MigrateToNewmembersModel < ActiveRecord::Migration - def up - UsersGroup.find_each(batch_size: 500) do |user_group| - GroupMember.create( - user_id: user_group.user_id, - source_type: 'Group', - source_id: user_group.group_id, - access_level: user_group.group_access, - notification_level: user_group.notification_level, - ) - - print '.' - end - - UsersProject.find_each(batch_size: 500) do |user_project| - ProjectMember.create( - user_id: user_project.user_id, - source_type: 'Project', - source_id: user_project.project_id, - access_level: user_project.project_access, - notification_level: user_project.notification_level, - ) - - print '.' - end - end - - def down - Member.destroy_all - end -end diff --git a/db/migrate/20140914173417_remove_old_member_tables.rb b/db/migrate/20140914173417_remove_old_member_tables.rb new file mode 100644 index 00000000000..408b9551dbb --- /dev/null +++ b/db/migrate/20140914173417_remove_old_member_tables.rb @@ -0,0 +1,26 @@ +class RemoveOldMemberTables < ActiveRecord::Migration + def up + drop_table :users_groups + drop_table :users_projects + end + + def down + create_table :users_groups do |t| + t.integer :group_access, null: false + t.integer :group_id, null: false + t.integer :user_id, null: false + t.integer :notification_level, null: false, default: 3 + + t.timestamps + end + + create_table :users_projects do |t| + t.integer :project_access, null: false + t.integer :project_id, null: false + t.integer :user_id, null: false + t.integer :notification_level, null: false, default: 3 + + t.timestamps + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 41ae717c670..4e249caa022 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20140914145549) do +ActiveRecord::Schema.define(version: 20140914173417) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -141,6 +141,11 @@ ActiveRecord::Schema.define(version: 20140914145549) do t.datetime "updated_at" end + add_index "members", ["access_level"], name: "index_members_on_access_level", using: :btree + add_index "members", ["source_id", "source_type"], name: "index_members_on_source_id_and_source_type", using: :btree + add_index "members", ["type"], name: "index_members_on_type", using: :btree + add_index "members", ["user_id"], name: "index_members_on_user_id", using: :btree + create_table "merge_request_diffs", force: true do |t| t.string "state" t.text "st_commits" @@ -374,30 +379,6 @@ ActiveRecord::Schema.define(version: 20140914145549) do add_index "users", ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true, using: :btree add_index "users", ["username"], name: "index_users_on_username", using: :btree - create_table "users_groups", force: true do |t| - t.integer "group_access", null: false - t.integer "group_id", null: false - t.integer "user_id", null: false - t.datetime "created_at" - t.datetime "updated_at" - t.integer "notification_level", default: 3, null: false - end - - add_index "users_groups", ["user_id"], name: "index_users_groups_on_user_id", using: :btree - - create_table "users_projects", force: true do |t| - t.integer "user_id", null: false - t.integer "project_id", null: false - t.datetime "created_at" - t.datetime "updated_at" - t.integer "project_access", default: 0, null: false - t.integer "notification_level", default: 3, null: false - end - - add_index "users_projects", ["project_access"], name: "index_users_projects_on_project_access", using: :btree - add_index "users_projects", ["project_id"], name: "index_users_projects_on_project_id", using: :btree - add_index "users_projects", ["user_id"], name: "index_users_projects_on_user_id", using: :btree - create_table "users_star_projects", force: true do |t| t.integer "project_id", null: false t.integer "user_id", null: false -- cgit v1.2.1 From 1aa48174db63871bb10b53e49c86222a4d9b7c6d Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 14 Sep 2014 21:52:54 +0300 Subject: Fix STI+polymorphic for Group <-> GroupMember Signed-off-by: Dmitriy Zaporozhets --- app/controllers/groups/group_members_controller.rb | 48 ++++++++++++++++++++++ .../projects/team_members_controller.rb | 2 +- app/controllers/users_groups_controller.rb | 48 ---------------------- app/models/members/group_member.rb | 5 ++- app/models/members/project_member.rb | 2 + .../groups/group_members/_group_member.html.haml | 31 ++++++++++++++ app/views/groups/group_members/update.js.haml | 2 + app/views/groups/members.html.haml | 2 +- app/views/users_groups/_users_group.html.haml | 31 -------------- app/views/users_groups/update.js.haml | 2 - config/routes.rb | 3 +- .../20140914145549_migrate_to_new_members_model.rb | 2 +- 12 files changed, 90 insertions(+), 88 deletions(-) create mode 100644 app/controllers/groups/group_members_controller.rb delete mode 100644 app/controllers/users_groups_controller.rb create mode 100644 app/views/groups/group_members/_group_member.html.haml create mode 100644 app/views/groups/group_members/update.js.haml delete mode 100644 app/views/users_groups/_users_group.html.haml delete mode 100644 app/views/users_groups/update.js.haml diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb new file mode 100644 index 00000000000..63c05d4f33b --- /dev/null +++ b/app/controllers/groups/group_members_controller.rb @@ -0,0 +1,48 @@ +class Groups::GroupMembersController < ApplicationController + before_filter :group + + # Authorize + before_filter :authorize_admin_group! + + layout 'group' + + def create + @group.add_users(params[:user_ids].split(','), params[:access_level]) + + redirect_to members_group_path(@group), notice: 'Users were successfully added.' + end + + def update + @member = @group.group_members.find(params[:id]) + @member.update_attributes(member_params) + end + + def destroy + @users_group = @group.group_members.find(params[:id]) + if can?(current_user, :destroy, @users_group) # May fail if last owner. + @users_group.destroy + respond_to do |format| + format.html { redirect_to members_group_path(@group), notice: 'User was successfully removed from group.' } + format.js { render nothing: true } + end + else + return render_403 + end + end + + protected + + def group + @group ||= Group.find_by(path: params[:group_id]) + end + + def authorize_admin_group! + unless can?(current_user, :manage_group, group) + return render_404 + end + end + + def member_params + params.require(:group_member).permit(:access_level, :user_id) + end +end diff --git a/app/controllers/projects/team_members_controller.rb b/app/controllers/projects/team_members_controller.rb index 158661f66fc..7bb799eba64 100644 --- a/app/controllers/projects/team_members_controller.rb +++ b/app/controllers/projects/team_members_controller.rb @@ -69,6 +69,6 @@ class Projects::TeamMembersController < Projects::ApplicationController end def member_params - params.require(:team_member).permit(:user_id, :access_level) + params.require(:project_member).permit(:user_id, :access_level) end end diff --git a/app/controllers/users_groups_controller.rb b/app/controllers/users_groups_controller.rb deleted file mode 100644 index 07bd41d25eb..00000000000 --- a/app/controllers/users_groups_controller.rb +++ /dev/null @@ -1,48 +0,0 @@ -class GroupMembersController < ApplicationController - before_filter :group - - # Authorize - before_filter :authorize_admin_group! - - layout 'group' - - def create - @group.add_users(params[:user_ids].split(','), params[:access_level]) - - redirect_to members_group_path(@group), notice: 'Users were successfully added.' - end - - def update - @member = @group.group_members.find(params[:id]) - @member.update_attributes(member_params) - end - - def destroy - @users_group = @group.group_members.find(params[:id]) - if can?(current_user, :destroy, @users_group) # May fail if last owner. - @users_group.destroy - respond_to do |format| - format.html { redirect_to members_group_path(@group), notice: 'User was successfully removed from group.' } - format.js { render nothing: true } - end - else - return render_403 - end - end - - protected - - def group - @group ||= Group.find_by(path: params[:group_id]) - end - - def authorize_admin_group! - unless can?(current_user, :manage_group, group) - return render_404 - end - end - - def member_params - params.require(:users_group).permit(:access_level, :user_id) - end -end diff --git a/app/models/members/group_member.rb b/app/models/members/group_member.rb index e7eb4a18e34..e72393c4278 100644 --- a/app/models/members/group_member.rb +++ b/app/models/members/group_member.rb @@ -1,11 +1,12 @@ class GroupMember < Member - SOURCE_TYPE = 'Group' + SOURCE_TYPE = 'Namespace' belongs_to :group, class_name: 'Group', foreign_key: 'source_id' # Make sure group member points only to group as it source default_value_for :source_type, SOURCE_TYPE - validates_format_of :source_type, with: /\AGroup\z/ + default_value_for :notification_level, Notification::N_GLOBAL + validates_format_of :source_type, with: /\ANamespace\z/ default_scope { where(source_type: SOURCE_TYPE) } scope :with_group, ->(group) { where(source_id: group.id) } diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb index 3f8137ed06e..f14900ad3e6 100644 --- a/app/models/members/project_member.rb +++ b/app/models/members/project_member.rb @@ -5,8 +5,10 @@ class ProjectMember < Member belongs_to :project, class_name: 'Project', foreign_key: 'source_id' + # Make sure project member points only to project as it source default_value_for :source_type, SOURCE_TYPE + default_value_for :notification_level, Notification::N_GLOBAL validates_format_of :source_type, with: /\AProject\z/ default_scope { where(source_type: SOURCE_TYPE) } diff --git a/app/views/groups/group_members/_group_member.html.haml b/app/views/groups/group_members/_group_member.html.haml new file mode 100644 index 00000000000..099e50a384e --- /dev/null +++ b/app/views/groups/group_members/_group_member.html.haml @@ -0,0 +1,31 @@ +- user = member.user +- return unless user +- show_roles = true if show_roles.nil? +%li{class: "#{dom_class(member)} js-toggle-container", id: dom_id(member)} + %span{class: ("list-item-name" if show_controls)} + = image_tag avatar_icon(user.email, 16), class: "avatar s16" + %strong= user.name + %span.cgray= user.username + - if user == current_user + %span.label.label-success It's you + + - if show_roles + %span.pull-right + %strong= member.human_access + - if show_controls + - if can?(current_user, :modify, member) + = link_to '#', class: "btn-tiny btn js-toggle-button", title: 'Edit access level' do + %i.icon-edit + - if can?(current_user, :destroy, member) + - if current_user == member.user + = link_to leave_profile_group_path(@group), data: { confirm: leave_group_message(@group.name)}, method: :delete, class: "btn-tiny btn btn-remove", title: 'Remove user from group' do + %i.icon-minus.icon-white + - else + = link_to group_group_member_path(@group, member), data: { confirm: remove_user_from_group_message(@group, user) }, method: :delete, remote: true, class: "btn-tiny btn btn-remove", title: 'Remove user from group' do + %i.icon-minus.icon-white + + .edit-member.hide.js-toggle-content + = form_for [@group, member], remote: true do |f| + .alert.prepend-top-20 + = f.select :access_level, options_for_select(GroupMember.access_level_roles, member.access_level) + = f.submit 'Save', class: 'btn btn-save btn-small' diff --git a/app/views/groups/group_members/update.js.haml b/app/views/groups/group_members/update.js.haml new file mode 100644 index 00000000000..5bad48abafd --- /dev/null +++ b/app/views/groups/group_members/update.js.haml @@ -0,0 +1,2 @@ +:plain + $("##{dom_id(@member)}").replaceWith('#{escape_javascript(render(@member, member: @member, show_controls: true))}'); diff --git a/app/views/groups/members.html.haml b/app/views/groups/members.html.haml index 7035cb95092..ebf407d4ef1 100644 --- a/app/views/groups/members.html.haml +++ b/app/views/groups/members.html.haml @@ -32,7 +32,7 @@ (#{@members.total_count}) %ul.well-list - @members.each do |member| - = render 'group_members/users_group', member: member, show_roles: show_roles, show_controls: true + = render 'groups/group_members/group_member', member: member, show_roles: show_roles, show_controls: true = paginate @members, theme: 'gitlab' :coffeescript diff --git a/app/views/users_groups/_users_group.html.haml b/app/views/users_groups/_users_group.html.haml deleted file mode 100644 index 7e246d3b7fb..00000000000 --- a/app/views/users_groups/_users_group.html.haml +++ /dev/null @@ -1,31 +0,0 @@ -- user = member.user -- return unless user -- show_roles = true if show_roles.nil? -%li{class: "#{dom_class(member)} js-toggle-container", id: dom_id(member)} - %span{class: ("list-item-name" if show_controls)} - = image_tag avatar_icon(user.email, 16), class: "avatar s16" - %strong= user.name - %span.cgray= user.username - - if user == current_user - %span.label.label-success It's you - - - if show_roles - %span.pull-right - %strong= member.human_access - - if show_controls - - if can?(current_user, :modify, member) - = link_to '#', class: "btn-tiny btn js-toggle-button", title: 'Edit access level' do - %i.icon-edit - - if can?(current_user, :destroy, member) - - if current_user == member.user - = link_to leave_profile_group_path(@group), data: { confirm: leave_group_message(@group.name)}, method: :delete, class: "btn-tiny btn btn-remove", title: 'Remove user from group' do - %i.icon-minus.icon-white - - else - = link_to group_users_group_path(@group, member), data: { confirm: remove_user_from_group_message(@group, user) }, method: :delete, remote: true, class: "btn-tiny btn btn-remove", title: 'Remove user from group' do - %i.icon-minus.icon-white - - .edit-member.hide.js-toggle-content - = form_for [@group, member], remote: true do |f| - .alert.prepend-top-20 - = f.select :access_level, options_for_select(GroupMember.access_level_roles, member.access_level) - = f.submit 'Save', class: 'btn btn-save btn-small' diff --git a/app/views/users_groups/update.js.haml b/app/views/users_groups/update.js.haml deleted file mode 100644 index 5bad48abafd..00000000000 --- a/app/views/users_groups/update.js.haml +++ /dev/null @@ -1,2 +0,0 @@ -:plain - $("##{dom_id(@member)}").replaceWith('#{escape_javascript(render(@member, member: @member, show_controls: true))}'); diff --git a/config/routes.rb b/config/routes.rb index ce66ea99951..39ab9f4265a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -161,9 +161,8 @@ Gitlab::Application.routes.draw do get :projects end - resources :users_groups, only: [:create, :update, :destroy] - scope module: :groups do + resources :group_members, only: [:create, :update, :destroy] resource :avatar, only: [:destroy] resources :milestones end diff --git a/db/migrate/20140914145549_migrate_to_new_members_model.rb b/db/migrate/20140914145549_migrate_to_new_members_model.rb index b4f7718f26d..2a5a49c724a 100644 --- a/db/migrate/20140914145549_migrate_to_new_members_model.rb +++ b/db/migrate/20140914145549_migrate_to_new_members_model.rb @@ -1,6 +1,6 @@ class MigrateToNewMembersModel < ActiveRecord::Migration def up - execute "INSERT INTO members ( user_id, source_id, source_type, access_level, notification_level, type ) SELECT user_id, group_id, 'Group', group_access, notification_level, 'GroupMember' FROM users_groups" + execute "INSERT INTO members ( user_id, source_id, source_type, access_level, notification_level, type ) SELECT user_id, group_id, 'Namespace', group_access, notification_level, 'GroupMember' FROM users_groups" execute "INSERT INTO members ( user_id, source_id, source_type, access_level, notification_level, type ) SELECT user_id, project_id, 'Project', project_access, notification_level, 'ProjectMember' FROM users_projects" end -- cgit v1.2.1 From bdbadebe3e6e25180d8c1465dde4573fa0ecc389 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 15 Sep 2014 10:57:02 +0300 Subject: Fix adminarea and emails for new membership logic Signed-off-by: Dmitriy Zaporozhets --- .../profiles/notifications_controller.rb | 6 +++--- app/mailers/emails/groups.rb | 2 +- app/mailers/emails/projects.rb | 8 ++++---- app/models/user.rb | 2 +- app/services/notification_service.rb | 24 +++++++++++----------- app/views/admin/groups/show.html.haml | 2 +- app/views/admin/projects/show.html.haml | 10 ++++----- .../notify/project_access_granted_email.html.haml | 2 +- .../notify/project_access_granted_email.text.erb | 2 +- app/views/profiles/notifications/show.html.haml | 6 +++--- .../projects/team_members/_group_members.html.haml | 2 +- spec/factories.rb | 4 ++-- spec/factories/users_groups.rb | 4 ++-- spec/mailers/notify_spec.rb | 6 +++--- spec/requests/api/branches_spec.rb | 4 ++-- spec/requests/api/commits_spec.rb | 4 ++-- spec/requests/api/project_members_spec.rb | 16 +++++++-------- spec/requests/api/projects_spec.rb | 8 ++++---- spec/requests/api/repositories_spec.rb | 4 ++-- spec/services/system_hooks_service_spec.rb | 10 ++++----- 20 files changed, 63 insertions(+), 63 deletions(-) diff --git a/app/controllers/profiles/notifications_controller.rb b/app/controllers/profiles/notifications_controller.rb index efed4a19ee3..638d1f9789b 100644 --- a/app/controllers/profiles/notifications_controller.rb +++ b/app/controllers/profiles/notifications_controller.rb @@ -18,9 +18,9 @@ class Profiles::NotificationsController < ApplicationController users_group.notification_level = params[:notification_level] users_group.save else - users_project = current_user.project_members.find(params[:notification_id]) - users_project.notification_level = params[:notification_level] - users_project.save + project_member = current_user.project_members.find(params[:notification_id]) + project_member.notification_level = params[:notification_level] + project_member.save end end end diff --git a/app/mailers/emails/groups.rb b/app/mailers/emails/groups.rb index 3f7c1d27350..8c09389985e 100644 --- a/app/mailers/emails/groups.rb +++ b/app/mailers/emails/groups.rb @@ -1,6 +1,6 @@ module Emails module Groups - def access_level_granted_email(user_group_id) + def group_access_granted_email(user_group_id) @membership = GroupMember.find(user_group_id) @group = @membership.group @target_url = group_url(@group) diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb index 152d5aa9959..d6edfd7059f 100644 --- a/app/mailers/emails/projects.rb +++ b/app/mailers/emails/projects.rb @@ -1,10 +1,10 @@ module Emails module Projects - def access_level_granted_email(user_project_id) - @users_project = ProjectMember.find user_project_id - @project = @users_project.project + def project_access_granted_email(user_project_id) + @project_member = ProjectMember.find user_project_id + @project = @project_member.project @target_url = project_url(@project) - mail(to: @users_project.user.email, + mail(to: @project_member.user.email, subject: subject("Access to project was granted")) end diff --git a/app/models/user.rb b/app/models/user.rb index d5a6b468fcd..ed3eba4cdf0 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -177,7 +177,7 @@ class User < ActiveRecord::Base scope :in_team, ->(team){ where(id: team.member_ids) } scope :not_in_team, ->(team){ where('users.id NOT IN (:ids)', ids: team.member_ids) } scope :not_in_project, ->(project) { project.users.present? ? where("id not in (:ids)", ids: project.users.map(&:id) ) : all } - scope :without_projects, -> { where('id NOT IN (SELECT DISTINCT(user_id) FROM project_members)') } + scope :without_projects, -> { where('id NOT IN (SELECT DISTINCT(user_id) FROM members)') } scope :ldap, -> { where(provider: 'ldap') } scope :potential_team_members, ->(team) { team.members.any? ? active.not_in_team(team) : active } diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index ca483e2f109..fe39f83b400 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -157,20 +157,20 @@ class NotificationService end end - def new_team_member(users_project) - mailer.access_level_granted_email(users_project.id) + def new_team_member(project_member) + mailer.project_access_granted_email(project_member.id) end - def update_team_member(users_project) - mailer.access_level_granted_email(users_project.id) + def update_team_member(project_member) + mailer.project_access_granted_email(project_member.id) end def new_group_member(users_group) - mailer.access_level_granted_email(users_group.id) + mailer.group_access_granted_email(users_group.id) end def update_group_member(users_group) - mailer.access_level_granted_email(users_group.id) + mailer.group_access_granted_email(users_group.id) end def project_was_moved(project) @@ -186,19 +186,19 @@ class NotificationService # Get project users with WATCH notification level def project_watchers(project) - project_members = users_project_notification(project) + project_members = project_member_notification(project) - users_with_project_level_global = users_project_notification(project, Notification::N_GLOBAL) + users_with_project_level_global = project_member_notification(project, Notification::N_GLOBAL) users_with_group_level_global = users_group_notification(project, Notification::N_GLOBAL) users = users_with_global_level_watch([users_with_project_level_global, users_with_group_level_global].flatten.uniq) - users_with_project_setting = select_users_project_setting(project, users_with_project_level_global, users) + users_with_project_setting = select_project_member_setting(project, users_with_project_level_global, users) users_with_group_setting = select_users_group_setting(project, project_members, users_with_group_level_global, users) User.where(id: users_with_project_setting.concat(users_with_group_setting).uniq).to_a end - def users_project_notification(project, notification_level=nil) + def project_member_notification(project, notification_level=nil) project_members = project.project_members if notification_level @@ -224,8 +224,8 @@ class NotificationService end # Build a list of users based on project notifcation settings - def select_users_project_setting(project, global_setting, users_global_level_watch) - users = users_project_notification(project, Notification::N_WATCH) + def select_project_member_setting(project, global_setting, users_global_level_watch) + users = project_member_notification(project, Notification::N_WATCH) # If project setting is global, add to watch list if global setting is watch global_setting.each do |user_id| diff --git a/app/views/admin/groups/show.html.haml b/app/views/admin/groups/show.html.haml index d7ccd67dac7..d59d2a23179 100644 --- a/app/views/admin/groups/show.html.haml +++ b/app/views/admin/groups/show.html.haml @@ -80,7 +80,7 @@ = link_to user.name, admin_user_path(user) %span.pull-right.light = member.human_access - = link_to group_users_group_path(@group, member), data: { confirm: remove_user_from_group_message(@group, user) }, method: :delete, remote: true, class: "btn-tiny btn btn-remove", title: 'Remove user from group' do + = link_to group_group_members_path(@group, member), data: { confirm: remove_user_from_group_message(@group, user) }, method: :delete, remote: true, class: "btn-tiny btn btn-remove", title: 'Remove user from group' do %i.icon-minus.icon-white .panel-footer = paginate @members, param_name: 'members_page', theme: 'gitlab' diff --git a/app/views/admin/projects/show.html.haml b/app/views/admin/projects/show.html.haml index 52fc5cea887..8413f0cb7f9 100644 --- a/app/views/admin/projects/show.html.haml +++ b/app/views/admin/projects/show.html.haml @@ -101,7 +101,7 @@ %i.icon-edit %ul.well-list - @group_members.each do |member| - = render 'group_members/users_group', member: member, show_controls: false + = render 'groups/group_members/group_member', member: member, show_controls: false .panel-footer = paginate @group_members, param_name: 'group_members_page', theme: 'gitlab' @@ -115,17 +115,17 @@ %i.icon-edit Manage Access %ul.well-list.team_members - - @project_members.each do |users_project| - - user = users_project.user + - @project_members.each do |project_member| + - user = project_member.user %li.project_member .list-item-name %strong = link_to user.name, admin_user_path(user) .pull-right - - if users_project.owner? + - if project_member.owner? %span.light Owner - else - %span.light= users_project.human_access + %span.light= project_member.human_access = link_to project_team_member_path(@project, user), data: { confirm: remove_from_project_team_message(@project, user)}, method: :delete, remote: true, class: "btn btn-small btn-remove" do %i.icon-remove .panel-footer diff --git a/app/views/notify/project_access_granted_email.html.haml b/app/views/notify/project_access_granted_email.html.haml index ce34f825358..4596205f39b 100644 --- a/app/views/notify/project_access_granted_email.html.haml +++ b/app/views/notify/project_access_granted_email.html.haml @@ -1,5 +1,5 @@ %p - = "You have been granted #{@users_project.human_access} access to project" + = "You have been granted #{@project_member.human_access} access to project" %p = link_to project_url(@project) do = @project.name_with_namespace diff --git a/app/views/notify/project_access_granted_email.text.erb b/app/views/notify/project_access_granted_email.text.erb index 66c57def375..de24feb802f 100644 --- a/app/views/notify/project_access_granted_email.text.erb +++ b/app/views/notify/project_access_granted_email.text.erb @@ -1,4 +1,4 @@ -You have been granted <%= @users_project.human_access %> access to project <%= @project.name_with_namespace %> +You have been granted <%= @project_member.human_access %> access to project <%= @project.name_with_namespace %> <%= url_for(project_url(@project)) %> diff --git a/app/views/profiles/notifications/show.html.haml b/app/views/profiles/notifications/show.html.haml index 3e28f2f5f90..f84de4430cc 100644 --- a/app/views/profiles/notifications/show.html.haml +++ b/app/views/profiles/notifications/show.html.haml @@ -46,6 +46,6 @@ .col-md-6 %h4 Projects: %ul.bordered-list - - @project_members.each do |users_project| - - notification = Notification.new(users_project) - = render 'settings', type: 'project', membership: users_project, notification: notification + - @project_members.each do |project_member| + - notification = Notification.new(project_member) + = render 'settings', type: 'project', membership: project_member, notification: notification diff --git a/app/views/projects/team_members/_group_members.html.haml b/app/views/projects/team_members/_group_members.html.haml index 837f2f5c932..77ffab89f37 100644 --- a/app/views/projects/team_members/_group_members.html.haml +++ b/app/views/projects/team_members/_group_members.html.haml @@ -8,7 +8,7 @@ %i.icon-edit %ul.well-list - @group.group_members.order('access_level DESC').limit(20).each do |member| - = render 'group_members/users_group', member: member, show_controls: false + = render 'groups/group_members/group_member', member: member, show_controls: false - if group_users_count > 20 %li and #{group_users_count - 20} more. For full list visit #{link_to 'group members page', members_group_path(@group)} diff --git a/spec/factories.rb b/spec/factories.rb index 0bf56fe84d6..a960571206c 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -39,10 +39,10 @@ FactoryGirl.define do owner end - factory :users_project do + factory :project_member do user project - project_access { ProjectMember::MASTER } + access_level { ProjectMember::MASTER } end factory :issue do diff --git a/spec/factories/users_groups.rb b/spec/factories/users_groups.rb index 450a0d88b76..debb86d997f 100644 --- a/spec/factories/users_groups.rb +++ b/spec/factories/users_groups.rb @@ -12,8 +12,8 @@ # FactoryGirl.define do - factory :users_group do - group_access { GroupMember::OWNER } + factory :group_member do + access_level { GroupMember::OWNER } group user end diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 702431e0712..4b64cefa852 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -402,10 +402,10 @@ describe Notify do describe 'project access changed' do let(:project) { create(:project) } let(:user) { create(:user) } - let(:users_project) { create(:users_project, + let(:project_member) { create(:project_member, project: project, user: user) } - subject { Notify.project_access_granted_email(users_project.id) } + subject { Notify.project_access_granted_email(project_member.id) } it_behaves_like 'an email sent from GitLab' @@ -416,7 +416,7 @@ describe Notify do should have_body_text /#{project.name}/ end it 'contains new user role' do - should have_body_text /#{users_project.human_access}/ + should have_body_text /#{project_member.human_access}/ end end diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb index 1690146aea7..5f6258488a4 100644 --- a/spec/requests/api/branches_spec.rb +++ b/spec/requests/api/branches_spec.rb @@ -7,8 +7,8 @@ describe API::API, api: true do let(:user) { create(:user) } let(:user2) { create(:user) } let!(:project) { create(:project, creator_id: user.id) } - let!(:master) { create(:users_project, user: user, project: project, project_access: ProjectMember::MASTER) } - let!(:guest) { create(:users_project, user: user2, project: project, project_access: ProjectMember::GUEST) } + let!(:master) { create(:project_member, user: user, project: project, project_access: ProjectMember::MASTER) } + let!(:guest) { create(:project_member, user: user2, project: project, project_access: ProjectMember::GUEST) } let!(:branch_name) { 'feature' } let!(:branch_sha) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' } diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index 21c0329e60f..3a5a2efdb0f 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -6,8 +6,8 @@ describe API::API, api: true do let(:user) { create(:user) } let(:user2) { create(:user) } let!(:project) { create(:project, creator_id: user.id) } - let!(:master) { create(:users_project, user: user, project: project, project_access: ProjectMember::MASTER) } - let!(:guest) { create(:users_project, user: user2, project: project, project_access: ProjectMember::GUEST) } + let!(:master) { create(:project_member, user: user, project: project, project_access: ProjectMember::MASTER) } + let!(:guest) { create(:project_member, user: user2, project: project, project_access: ProjectMember::GUEST) } before { project.team << [user, :reporter] } diff --git a/spec/requests/api/project_members_spec.rb b/spec/requests/api/project_members_spec.rb index e101f871ff0..626da6196d5 100644 --- a/spec/requests/api/project_members_spec.rb +++ b/spec/requests/api/project_members_spec.rb @@ -6,12 +6,12 @@ describe API::API, api: true do let(:user2) { create(:user) } let(:user3) { create(:user) } let(:project) { create(:project, creator_id: user.id, namespace: user.namespace) } - let(:users_project) { create(:users_project, user: user, project: project, project_access: ProjectMember::MASTER) } - let(:users_project2) { create(:users_project, user: user3, project: project, project_access: ProjectMember::DEVELOPER) } + let(:project_member) { create(:project_member, user: user, project: project, project_access: ProjectMember::MASTER) } + let(:project_member2) { create(:project_member, user: user3, project: project, project_access: ProjectMember::DEVELOPER) } describe "GET /projects/:id/members" do - before { users_project } - before { users_project2 } + before { project_member } + before { project_member2 } it "should return project team members" do get api("/projects/#{project.id}/members", user) @@ -36,7 +36,7 @@ describe API::API, api: true do end describe "GET /projects/:id/members/:user_id" do - before { users_project } + before { project_member } it "should return project team member" do get api("/projects/#{project.id}/members/#{user.id}", user) @@ -93,7 +93,7 @@ describe API::API, api: true do end describe "PUT /projects/:id/members/:user_id" do - before { users_project2 } + before { project_member2 } it "should update project team member" do put api("/projects/#{project.id}/members/#{user3.id}", user), access_level: ProjectMember::MASTER @@ -119,8 +119,8 @@ describe API::API, api: true do end describe "DELETE /projects/:id/members/:user_id" do - before { users_project } - before { users_project2 } + before { project_member } + before { project_member2 } it "should remove user from project team" do expect { diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 7d36043e3d8..59a20c37489 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -8,8 +8,8 @@ describe API::API, api: true do let(:admin) { create(:admin) } let(:project) { create(:project, creator_id: user.id, namespace: user.namespace) } let(:snippet) { create(:project_snippet, author: user, project: project, title: 'example') } - let(:users_project) { create(:users_project, user: user, project: project, project_access: ProjectMember::MASTER) } - let(:users_project2) { create(:users_project, user: user3, project: project, project_access: ProjectMember::DEVELOPER) } + let(:project_member) { create(:project_member, user: user, project: project, project_access: ProjectMember::MASTER) } + let(:project_member2) { create(:project_member, user: user3, project: project, project_access: ProjectMember::DEVELOPER) } describe "GET /projects" do before { project } @@ -254,7 +254,7 @@ describe API::API, api: true do describe "GET /projects/:id" do before { project } - before { users_project } + before { project_member } it "should return a project by id" do get api("/projects/#{project.id}", user) @@ -305,7 +305,7 @@ describe API::API, api: true do end describe "GET /projects/:id/events" do - before { users_project } + before { project_member } it "should return a project events" do get api("/projects/#{project.id}/events", user) diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index ba404901a49..6618ce0e378 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -8,8 +8,8 @@ describe API::API, api: true do let(:user) { create(:user) } let(:user2) { create(:user) } let!(:project) { create(:project, creator_id: user.id) } - let!(:master) { create(:users_project, user: user, project: project, project_access: ProjectMember::MASTER) } - let!(:guest) { create(:users_project, user: user2, project: project, project_access: ProjectMember::GUEST) } + let!(:master) { create(:project_member, user: user, project: project, project_access: ProjectMember::MASTER) } + let!(:guest) { create(:project_member, user: user2, project: project, project_access: ProjectMember::GUEST) } before { project.team << [user, :reporter] } diff --git a/spec/services/system_hooks_service_spec.rb b/spec/services/system_hooks_service_spec.rb index 7497bdb0b3c..573446d3a19 100644 --- a/spec/services/system_hooks_service_spec.rb +++ b/spec/services/system_hooks_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe SystemHooksService do let (:user) { create :user } let (:project) { create :project } - let (:users_project) { create :users_project } + let (:project_member) { create :project_member } let (:key) { create(:key, user: user) } context 'event data' do @@ -11,8 +11,8 @@ describe SystemHooksService do it { event_data(user, :destroy).should include(:event_name, :name, :created_at, :email, :user_id) } it { event_data(project, :create).should include(:event_name, :name, :created_at, :path, :project_id, :owner_name, :owner_email, :project_visibility) } it { event_data(project, :destroy).should include(:event_name, :name, :created_at, :path, :project_id, :owner_name, :owner_email, :project_visibility) } - it { event_data(users_project, :create).should include(:event_name, :created_at, :project_name, :project_path, :project_id, :user_name, :user_email, :project_access, :project_visibility) } - it { event_data(users_project, :destroy).should include(:event_name, :created_at, :project_name, :project_path, :project_id, :user_name, :user_email, :project_access, :project_visibility) } + it { event_data(project_member, :create).should include(:event_name, :created_at, :project_name, :project_path, :project_id, :user_name, :user_email, :access_level, :project_visibility) } + it { event_data(project_member, :destroy).should include(:event_name, :created_at, :project_name, :project_path, :project_id, :user_name, :user_email, :access_level, :project_visibility) } it { event_data(key, :create).should include(:username, :key, :id) } it { event_data(key, :destroy).should include(:username, :key, :id) } end @@ -22,8 +22,8 @@ describe SystemHooksService do it { event_name(user, :destroy).should eq "user_destroy" } it { event_name(project, :create).should eq "project_create" } it { event_name(project, :destroy).should eq "project_destroy" } - it { event_name(users_project, :create).should eq "user_add_to_team" } - it { event_name(users_project, :destroy).should eq "user_remove_from_team" } + it { event_name(project_member, :create).should eq "user_add_to_team" } + it { event_name(project_member, :destroy).should eq "user_remove_from_team" } it { event_name(key, :create).should eq 'key_create' } it { event_name(key, :destroy).should eq 'key_destroy' } end -- cgit v1.2.1 From 39ab7527d41c2351b82398947307f3d668f06944 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 15 Sep 2014 11:04:25 +0300 Subject: Fix project member management Signed-off-by: Dmitriy Zaporozhets --- app/models/project_team.rb | 2 +- app/views/projects/team_members/_form.html.haml | 2 +- app/views/projects/team_members/_team_member.html.haml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/project_team.rb b/app/models/project_team.rb index 330e2aa728c..e2af10c6899 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -102,7 +102,7 @@ class ProjectTeam source_team.map! do |tm| new_tm = tm.dup new_tm.id = nil - new_tm.project_id = target_project.id + new_tm.source = target_project new_tm end diff --git a/app/views/projects/team_members/_form.html.haml b/app/views/projects/team_members/_form.html.haml index 7e3fe02e198..2bf61fa12bb 100644 --- a/app/views/projects/team_members/_form.html.haml +++ b/app/views/projects/team_members/_form.html.haml @@ -1,7 +1,7 @@ %h3.page-title New project member(s) -= form_for @user_project_relation, as: :team_member, url: project_team_members_path(@project), html: { class: "form-horizontal users-project-form" } do |f| += form_for @user_project_relation, as: :project_member, url: project_team_members_path(@project), html: { class: "form-horizontal users-project-form" } do |f| -if @user_project_relation.errors.any? .alert.alert-danger %ul diff --git a/app/views/projects/team_members/_team_member.html.haml b/app/views/projects/team_members/_team_member.html.haml index 3ecbf129083..79b78665417 100644 --- a/app/views/projects/team_members/_team_member.html.haml +++ b/app/views/projects/team_members/_team_member.html.haml @@ -4,7 +4,7 @@ - if current_user_can_admin_project - unless @project.personal? && user == current_user .pull-left - = form_for(member, as: :team_member, url: project_team_member_path(@project, member.user)) do |f| + = form_for(member, as: :project_member, url: project_team_member_path(@project, member.user)) do |f| = f.select :access_level, options_for_select(ProjectMember.access_roles, member.access_level), {}, class: "medium project-access-select span2 trigger-submit"   = link_to project_team_member_path(@project, user), data: { confirm: remove_from_project_team_message(@project, user)}, method: :delete, class: "btn-tiny btn btn-remove", title: 'Remove user from team' do -- cgit v1.2.1 From 38ed0deaac78f74cf983caf2f607956c1b2dcf83 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 15 Sep 2014 11:31:30 +0300 Subject: Move hook models in separate dir Signed-off-by: Dmitriy Zaporozhets --- app/models/hooks/project_hook.rb | 25 +++++++++++++++++++ app/models/hooks/service_hook.rb | 20 ++++++++++++++++ app/models/hooks/system_hook.rb | 19 +++++++++++++++ app/models/hooks/web_hook.rb | 52 ++++++++++++++++++++++++++++++++++++++++ app/models/project_hook.rb | 25 ------------------- app/models/service_hook.rb | 20 ---------------- app/models/system_hook.rb | 19 --------------- app/models/web_hook.rb | 52 ---------------------------------------- config/application.rb | 1 + 9 files changed, 117 insertions(+), 116 deletions(-) create mode 100644 app/models/hooks/project_hook.rb create mode 100644 app/models/hooks/service_hook.rb create mode 100644 app/models/hooks/system_hook.rb create mode 100644 app/models/hooks/web_hook.rb delete mode 100644 app/models/project_hook.rb delete mode 100644 app/models/service_hook.rb delete mode 100644 app/models/system_hook.rb delete mode 100644 app/models/web_hook.rb diff --git a/app/models/hooks/project_hook.rb b/app/models/hooks/project_hook.rb new file mode 100644 index 00000000000..21867a9316c --- /dev/null +++ b/app/models/hooks/project_hook.rb @@ -0,0 +1,25 @@ +# == Schema Information +# +# Table name: web_hooks +# +# id :integer not null, primary key +# url :string(255) +# project_id :integer +# created_at :datetime +# updated_at :datetime +# type :string(255) default("ProjectHook") +# service_id :integer +# push_events :boolean default(TRUE), not null +# issues_events :boolean default(FALSE), not null +# merge_requests_events :boolean default(FALSE), not null +# tag_push_events :boolean default(FALSE) +# + +class ProjectHook < WebHook + belongs_to :project + + scope :push_hooks, -> { where(push_events: true) } + scope :tag_push_hooks, -> { where(tag_push_events: true) } + scope :issue_hooks, -> { where(issues_events: true) } + scope :merge_request_hooks, -> { where(merge_requests_events: true) } +end diff --git a/app/models/hooks/service_hook.rb b/app/models/hooks/service_hook.rb new file mode 100644 index 00000000000..2e11239c40b --- /dev/null +++ b/app/models/hooks/service_hook.rb @@ -0,0 +1,20 @@ +# == Schema Information +# +# Table name: web_hooks +# +# id :integer not null, primary key +# url :string(255) +# project_id :integer +# created_at :datetime +# updated_at :datetime +# type :string(255) default("ProjectHook") +# service_id :integer +# push_events :boolean default(TRUE), not null +# issues_events :boolean default(FALSE), not null +# merge_requests_events :boolean default(FALSE), not null +# tag_push_events :boolean default(FALSE) +# + +class ServiceHook < WebHook + belongs_to :service +end diff --git a/app/models/hooks/system_hook.rb b/app/models/hooks/system_hook.rb new file mode 100644 index 00000000000..ee32b49bc66 --- /dev/null +++ b/app/models/hooks/system_hook.rb @@ -0,0 +1,19 @@ +# == Schema Information +# +# Table name: web_hooks +# +# id :integer not null, primary key +# url :string(255) +# project_id :integer +# created_at :datetime +# updated_at :datetime +# type :string(255) default("ProjectHook") +# service_id :integer +# push_events :boolean default(TRUE), not null +# issues_events :boolean default(FALSE), not null +# merge_requests_events :boolean default(FALSE), not null +# tag_push_events :boolean default(FALSE) +# + +class SystemHook < WebHook +end diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb new file mode 100644 index 00000000000..752eb8074ac --- /dev/null +++ b/app/models/hooks/web_hook.rb @@ -0,0 +1,52 @@ +# == Schema Information +# +# Table name: web_hooks +# +# id :integer not null, primary key +# url :string(255) +# project_id :integer +# created_at :datetime +# updated_at :datetime +# type :string(255) default("ProjectHook") +# service_id :integer +# push_events :boolean default(TRUE), not null +# issues_events :boolean default(FALSE), not null +# merge_requests_events :boolean default(FALSE), not null +# tag_push_events :boolean default(FALSE) +# + +class WebHook < ActiveRecord::Base + include HTTParty + + default_value_for :push_events, true + default_value_for :issues_events, false + default_value_for :merge_requests_events, false + + # HTTParty timeout + default_timeout Gitlab.config.gitlab.webhook_timeout + + validates :url, presence: true, + format: { with: URI::regexp(%w(http https)), message: "should be a valid url" } + + def execute(data) + parsed_url = URI.parse(url) + if parsed_url.userinfo.blank? + WebHook.post(url, body: data.to_json, headers: { "Content-Type" => "application/json" }, verify: false) + else + post_url = url.gsub("#{parsed_url.userinfo}@", "") + auth = { + username: URI.decode(parsed_url.user), + password: URI.decode(parsed_url.password), + } + WebHook.post(post_url, + body: data.to_json, + headers: {"Content-Type" => "application/json"}, + verify: false, + basic_auth: auth) + end + end + + def async_execute(data) + Sidekiq::Client.enqueue(ProjectWebHookWorker, id, data) + end +end diff --git a/app/models/project_hook.rb b/app/models/project_hook.rb deleted file mode 100644 index 21867a9316c..00000000000 --- a/app/models/project_hook.rb +++ /dev/null @@ -1,25 +0,0 @@ -# == Schema Information -# -# Table name: web_hooks -# -# id :integer not null, primary key -# url :string(255) -# project_id :integer -# created_at :datetime -# updated_at :datetime -# type :string(255) default("ProjectHook") -# service_id :integer -# push_events :boolean default(TRUE), not null -# issues_events :boolean default(FALSE), not null -# merge_requests_events :boolean default(FALSE), not null -# tag_push_events :boolean default(FALSE) -# - -class ProjectHook < WebHook - belongs_to :project - - scope :push_hooks, -> { where(push_events: true) } - scope :tag_push_hooks, -> { where(tag_push_events: true) } - scope :issue_hooks, -> { where(issues_events: true) } - scope :merge_request_hooks, -> { where(merge_requests_events: true) } -end diff --git a/app/models/service_hook.rb b/app/models/service_hook.rb deleted file mode 100644 index 2e11239c40b..00000000000 --- a/app/models/service_hook.rb +++ /dev/null @@ -1,20 +0,0 @@ -# == Schema Information -# -# Table name: web_hooks -# -# id :integer not null, primary key -# url :string(255) -# project_id :integer -# created_at :datetime -# updated_at :datetime -# type :string(255) default("ProjectHook") -# service_id :integer -# push_events :boolean default(TRUE), not null -# issues_events :boolean default(FALSE), not null -# merge_requests_events :boolean default(FALSE), not null -# tag_push_events :boolean default(FALSE) -# - -class ServiceHook < WebHook - belongs_to :service -end diff --git a/app/models/system_hook.rb b/app/models/system_hook.rb deleted file mode 100644 index ee32b49bc66..00000000000 --- a/app/models/system_hook.rb +++ /dev/null @@ -1,19 +0,0 @@ -# == Schema Information -# -# Table name: web_hooks -# -# id :integer not null, primary key -# url :string(255) -# project_id :integer -# created_at :datetime -# updated_at :datetime -# type :string(255) default("ProjectHook") -# service_id :integer -# push_events :boolean default(TRUE), not null -# issues_events :boolean default(FALSE), not null -# merge_requests_events :boolean default(FALSE), not null -# tag_push_events :boolean default(FALSE) -# - -class SystemHook < WebHook -end diff --git a/app/models/web_hook.rb b/app/models/web_hook.rb deleted file mode 100644 index 752eb8074ac..00000000000 --- a/app/models/web_hook.rb +++ /dev/null @@ -1,52 +0,0 @@ -# == Schema Information -# -# Table name: web_hooks -# -# id :integer not null, primary key -# url :string(255) -# project_id :integer -# created_at :datetime -# updated_at :datetime -# type :string(255) default("ProjectHook") -# service_id :integer -# push_events :boolean default(TRUE), not null -# issues_events :boolean default(FALSE), not null -# merge_requests_events :boolean default(FALSE), not null -# tag_push_events :boolean default(FALSE) -# - -class WebHook < ActiveRecord::Base - include HTTParty - - default_value_for :push_events, true - default_value_for :issues_events, false - default_value_for :merge_requests_events, false - - # HTTParty timeout - default_timeout Gitlab.config.gitlab.webhook_timeout - - validates :url, presence: true, - format: { with: URI::regexp(%w(http https)), message: "should be a valid url" } - - def execute(data) - parsed_url = URI.parse(url) - if parsed_url.userinfo.blank? - WebHook.post(url, body: data.to_json, headers: { "Content-Type" => "application/json" }, verify: false) - else - post_url = url.gsub("#{parsed_url.userinfo}@", "") - auth = { - username: URI.decode(parsed_url.user), - password: URI.decode(parsed_url.password), - } - WebHook.post(post_url, - body: data.to_json, - headers: {"Content-Type" => "application/json"}, - verify: false, - basic_auth: auth) - end - end - - def async_execute(data) - Sidekiq::Client.enqueue(ProjectWebHookWorker, id, data) - end -end diff --git a/config/application.rb b/config/application.rb index a4234e8a264..99dfafdb786 100644 --- a/config/application.rb +++ b/config/application.rb @@ -14,6 +14,7 @@ module Gitlab # Custom directories with classes and modules you want to be autoloadable. config.autoload_paths += %W(#{config.root}/lib #{config.root}/app/finders + #{config.root}/app/models/hooks #{config.root}/app/models/concerns #{config.root}/app/models/project_services #{config.root}/app/models/members) -- cgit v1.2.1 From 77c64a9b361a35cfdb7d93c92c5ecc94a8d1aa08 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 15 Sep 2014 11:36:50 +0300 Subject: Replace project_access attribute with access_level in specs and API Signed-off-by: Dmitriy Zaporozhets --- lib/api/project_members.rb | 4 ++-- spec/factories/group_members.rb | 20 ++++++++++++++++++++ spec/factories/users_groups.rb | 20 -------------------- spec/models/group_member_spec.rb | 10 +++++----- spec/models/note_spec.rb | 14 +++++++------- spec/models/project_security_spec.rb | 16 ++++++++-------- spec/requests/api/branches_spec.rb | 4 ++-- spec/requests/api/commits_spec.rb | 4 ++-- spec/requests/api/project_members_spec.rb | 4 ++-- spec/requests/api/projects_spec.rb | 4 ++-- spec/requests/api/repositories_spec.rb | 4 ++-- 11 files changed, 52 insertions(+), 52 deletions(-) create mode 100644 spec/factories/group_members.rb delete mode 100644 spec/factories/users_groups.rb diff --git a/lib/api/project_members.rb b/lib/api/project_members.rb index 9e2b12b1311..c0e532f4137 100644 --- a/lib/api/project_members.rb +++ b/lib/api/project_members.rb @@ -58,7 +58,7 @@ module API if team_member.nil? team_member = user_project.project_members.new( user_id: params[:user_id], - project_access: params[:access_level] + access_level: params[:access_level] ) end @@ -85,7 +85,7 @@ module API team_member = user_project.project_members.find_by(user_id: params[:user_id]) not_found!("User can not be found") if team_member.nil? - if team_member.update_attributes(project_access: params[:access_level]) + if team_member.update_attributes(access_level: params[:access_level]) @member = team_member.user present @member, with: Entities::ProjectMember, project: user_project else diff --git a/spec/factories/group_members.rb b/spec/factories/group_members.rb new file mode 100644 index 00000000000..debb86d997f --- /dev/null +++ b/spec/factories/group_members.rb @@ -0,0 +1,20 @@ +# == Schema Information +# +# Table name: group_members +# +# id :integer not null, primary key +# group_access :integer not null +# group_id :integer not null +# user_id :integer not null +# created_at :datetime +# updated_at :datetime +# notification_level :integer default(3), not null +# + +FactoryGirl.define do + factory :group_member do + access_level { GroupMember::OWNER } + group + user + end +end diff --git a/spec/factories/users_groups.rb b/spec/factories/users_groups.rb deleted file mode 100644 index debb86d997f..00000000000 --- a/spec/factories/users_groups.rb +++ /dev/null @@ -1,20 +0,0 @@ -# == Schema Information -# -# Table name: group_members -# -# id :integer not null, primary key -# group_access :integer not null -# group_id :integer not null -# user_id :integer not null -# created_at :datetime -# updated_at :datetime -# notification_level :integer default(3), not null -# - -FactoryGirl.define do - factory :group_member do - access_level { GroupMember::OWNER } - group - user - end -end diff --git a/spec/models/group_member_spec.rb b/spec/models/group_member_spec.rb index 58205e599fe..6acbc9bb4ae 100644 --- a/spec/models/group_member_spec.rb +++ b/spec/models/group_member_spec.rb @@ -3,7 +3,7 @@ # Table name: group_members # # id :integer not null, primary key -# group_access :integer not null +# access_level :integer not null # group_id :integer not null # user_id :integer not null # created_at :datetime @@ -17,7 +17,7 @@ describe GroupMember do context 'notification' do describe "#after_create" do it "should send email to user" do - membership = build(:users_group) + membership = build(:group_member) membership.stub(notification_service: double('NotificationService').as_null_object) membership.should_receive(:notification_service) membership.save @@ -26,18 +26,18 @@ describe GroupMember do describe "#after_update" do before do - @membership = create :users_group + @membership = create :group_member @membership.stub(notification_service: double('NotificationService').as_null_object) end it "should send email to user" do @membership.should_receive(:notification_service) - @membership.update_attribute(:group_access, GroupMember::MASTER) + @membership.update_attribute(:access_level, GroupMember::MASTER) end it "does not send an email when the access level has not changed" do @membership.should_not_receive(:notification_service) - @membership.update_attribute(:group_access, GroupMember::OWNER) + @membership.update_attribute(:access_level, GroupMember::OWNER) end end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index a6796d4ab15..da51100e0d7 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -321,8 +321,8 @@ describe Note do describe :read do before do - @p1.project_members.create(user: @u2, project_access: ProjectMember::GUEST) - @p2.project_members.create(user: @u3, project_access: ProjectMember::GUEST) + @p1.project_members.create(user: @u2, access_level: ProjectMember::GUEST) + @p2.project_members.create(user: @u3, access_level: ProjectMember::GUEST) end it { @abilities.allowed?(@u1, :read_note, @p1).should be_false } @@ -332,8 +332,8 @@ describe Note do describe :write do before do - @p1.project_members.create(user: @u2, project_access: ProjectMember::DEVELOPER) - @p2.project_members.create(user: @u3, project_access: ProjectMember::DEVELOPER) + @p1.project_members.create(user: @u2, access_level: ProjectMember::DEVELOPER) + @p2.project_members.create(user: @u3, access_level: ProjectMember::DEVELOPER) end it { @abilities.allowed?(@u1, :write_note, @p1).should be_false } @@ -343,9 +343,9 @@ describe Note do describe :admin do before do - @p1.project_members.create(user: @u1, project_access: ProjectMember::REPORTER) - @p1.project_members.create(user: @u2, project_access: ProjectMember::MASTER) - @p2.project_members.create(user: @u3, project_access: ProjectMember::MASTER) + @p1.project_members.create(user: @u1, access_level: ProjectMember::REPORTER) + @p1.project_members.create(user: @u2, access_level: ProjectMember::MASTER) + @p2.project_members.create(user: @u3, access_level: ProjectMember::MASTER) end it { @abilities.allowed?(@u1, :admin_note, @p1).should be_false } diff --git a/spec/models/project_security_spec.rb b/spec/models/project_security_spec.rb index 564edaf8c1b..5c8d1e7438b 100644 --- a/spec/models/project_security_spec.rb +++ b/spec/models/project_security_spec.rb @@ -30,7 +30,7 @@ describe Project do describe "Guest Rules" do before do - @p1.project_members.create(project: @p1, user: @u2, project_access: ProjectMember::GUEST) + @p1.project_members.create(project: @p1, user: @u2, access_level: ProjectMember::GUEST) end it "should allow for project user any guest actions" do @@ -42,7 +42,7 @@ describe Project do describe "Report Rules" do before do - @p1.project_members.create(project: @p1, user: @u2, project_access: ProjectMember::REPORTER) + @p1.project_members.create(project: @p1, user: @u2, access_level: ProjectMember::REPORTER) end it "should allow for project user any report actions" do @@ -54,8 +54,8 @@ describe Project do describe "Developer Rules" do before do - @p1.project_members.create(project: @p1, user: @u2, project_access: ProjectMember::REPORTER) - @p1.project_members.create(project: @p1, user: @u3, project_access: ProjectMember::DEVELOPER) + @p1.project_members.create(project: @p1, user: @u2, access_level: ProjectMember::REPORTER) + @p1.project_members.create(project: @p1, user: @u3, access_level: ProjectMember::DEVELOPER) end it "should deny for developer master-specific actions" do @@ -73,8 +73,8 @@ describe Project do describe "Master Rules" do before do - @p1.project_members.create(project: @p1, user: @u2, project_access: ProjectMember::DEVELOPER) - @p1.project_members.create(project: @p1, user: @u3, project_access: ProjectMember::MASTER) + @p1.project_members.create(project: @p1, user: @u2, access_level: ProjectMember::DEVELOPER) + @p1.project_members.create(project: @p1, user: @u3, access_level: ProjectMember::MASTER) end it "should deny for developer master-specific actions" do @@ -92,8 +92,8 @@ describe Project do describe "Admin Rules" do before do - @p1.project_members.create(project: @p1, user: @u2, project_access: ProjectMember::DEVELOPER) - @p1.project_members.create(project: @p1, user: @u3, project_access: ProjectMember::MASTER) + @p1.project_members.create(project: @p1, user: @u2, access_level: ProjectMember::DEVELOPER) + @p1.project_members.create(project: @p1, user: @u3, access_level: ProjectMember::MASTER) end it "should deny for masters admin-specific actions" do diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb index 5f6258488a4..caded2c9289 100644 --- a/spec/requests/api/branches_spec.rb +++ b/spec/requests/api/branches_spec.rb @@ -7,8 +7,8 @@ describe API::API, api: true do let(:user) { create(:user) } let(:user2) { create(:user) } let!(:project) { create(:project, creator_id: user.id) } - let!(:master) { create(:project_member, user: user, project: project, project_access: ProjectMember::MASTER) } - let!(:guest) { create(:project_member, user: user2, project: project, project_access: ProjectMember::GUEST) } + let!(:master) { create(:project_member, user: user, project: project, access_level: ProjectMember::MASTER) } + let!(:guest) { create(:project_member, user: user2, project: project, access_level: ProjectMember::GUEST) } let!(:branch_name) { 'feature' } let!(:branch_sha) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' } diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index 3a5a2efdb0f..38e0a284c36 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -6,8 +6,8 @@ describe API::API, api: true do let(:user) { create(:user) } let(:user2) { create(:user) } let!(:project) { create(:project, creator_id: user.id) } - let!(:master) { create(:project_member, user: user, project: project, project_access: ProjectMember::MASTER) } - let!(:guest) { create(:project_member, user: user2, project: project, project_access: ProjectMember::GUEST) } + let!(:master) { create(:project_member, user: user, project: project, access_level: ProjectMember::MASTER) } + let!(:guest) { create(:project_member, user: user2, project: project, access_level: ProjectMember::GUEST) } before { project.team << [user, :reporter] } diff --git a/spec/requests/api/project_members_spec.rb b/spec/requests/api/project_members_spec.rb index 626da6196d5..836f21f3e0b 100644 --- a/spec/requests/api/project_members_spec.rb +++ b/spec/requests/api/project_members_spec.rb @@ -6,8 +6,8 @@ describe API::API, api: true do let(:user2) { create(:user) } let(:user3) { create(:user) } let(:project) { create(:project, creator_id: user.id, namespace: user.namespace) } - let(:project_member) { create(:project_member, user: user, project: project, project_access: ProjectMember::MASTER) } - let(:project_member2) { create(:project_member, user: user3, project: project, project_access: ProjectMember::DEVELOPER) } + let(:project_member) { create(:project_member, user: user, project: project, access_level: ProjectMember::MASTER) } + let(:project_member2) { create(:project_member, user: user3, project: project, access_level: ProjectMember::DEVELOPER) } describe "GET /projects/:id/members" do before { project_member } diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 59a20c37489..a52ba30187e 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -8,8 +8,8 @@ describe API::API, api: true do let(:admin) { create(:admin) } let(:project) { create(:project, creator_id: user.id, namespace: user.namespace) } let(:snippet) { create(:project_snippet, author: user, project: project, title: 'example') } - let(:project_member) { create(:project_member, user: user, project: project, project_access: ProjectMember::MASTER) } - let(:project_member2) { create(:project_member, user: user3, project: project, project_access: ProjectMember::DEVELOPER) } + let(:project_member) { create(:project_member, user: user, project: project, access_level: ProjectMember::MASTER) } + let(:project_member2) { create(:project_member, user: user3, project: project, access_level: ProjectMember::DEVELOPER) } describe "GET /projects" do before { project } diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index 6618ce0e378..a339dbfe9db 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -8,8 +8,8 @@ describe API::API, api: true do let(:user) { create(:user) } let(:user2) { create(:user) } let!(:project) { create(:project, creator_id: user.id) } - let!(:master) { create(:project_member, user: user, project: project, project_access: ProjectMember::MASTER) } - let!(:guest) { create(:project_member, user: user2, project: project, project_access: ProjectMember::GUEST) } + let!(:master) { create(:project_member, user: user, project: project, access_level: ProjectMember::MASTER) } + let!(:guest) { create(:project_member, user: user2, project: project, access_level: ProjectMember::GUEST) } before { project.team << [user, :reporter] } -- cgit v1.2.1 From ce49f035cfccc9cb2e86b986b8a69b18fcc9cadc Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 15 Sep 2014 11:55:36 +0300 Subject: Fix access_level api Signed-off-by: Dmitriy Zaporozhets --- features/steps/project/team_management.rb | 4 ++-- lib/api/entities.rb | 10 +++++----- spec/requests/api/projects_spec.rb | 5 ++++- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/features/steps/project/team_management.rb b/features/steps/project/team_management.rb index ffc5016529f..db04e234c84 100644 --- a/features/steps/project/team_management.rb +++ b/features/steps/project/team_management.rb @@ -24,7 +24,7 @@ class ProjectTeamManagement < Spinach::FeatureSteps select2(user.id, from: "#user_ids", multiple: true) within "#new_team_member" do - select "Reporter", from: "project_access" + select "Reporter", from: "access_level" end click_button "Add users" end @@ -44,7 +44,7 @@ class ProjectTeamManagement < Spinach::FeatureSteps And 'I change "Sam" role to "Reporter"' do user = User.find_by(name: "Sam") within "#user_#{user.id}" do - select "Reporter", from: "team_member_project_access" + select "Reporter", from: "project_member_access_level" end end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index d012a6056f5..4b14472fd5c 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -53,8 +53,8 @@ module API end class ProjectMember < UserBasic - expose :project_access, as: :access_level do |user, options| - options[:project].project_members.find_by(user_id: user.id).project_access + expose :access_level do |user, options| + options[:project].project_members.find_by(user_id: user.id).access_level end end @@ -67,7 +67,7 @@ module API end class GroupMember < UserBasic - expose :group_access, as: :access_level do |user, options| + expose :access_level do |user, options| options[:group].group_members.find_by(user_id: user.id).group_access end end @@ -170,12 +170,12 @@ module API end class ProjectAccess < Grape::Entity - expose :project_access, as: :access_level + expose :access_level expose :notification_level end class GroupAccess < Grape::Entity - expose :group_access, as: :access_level + expose :access_level expose :notification_level end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index a52ba30187e..5575da86c2e 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -283,7 +283,10 @@ describe API::API, api: true do describe 'permissions' do context 'personal project' do - before { get api("/projects/#{project.id}", user) } + before do + project.team << [user, :master] + get api("/projects/#{project.id}", user) + end it { response.status.should == 200 } it { json_response['permissions']["project_access"]["access_level"].should == Gitlab::Access::MASTER } -- cgit v1.2.1 From 6558e52d904daa9f710192e838fdb7872b1682e2 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 15 Sep 2014 12:01:45 +0300 Subject: More fixes to project members api Signed-off-by: Dmitriy Zaporozhets --- lib/api/project_members.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/api/project_members.rb b/lib/api/project_members.rb index c0e532f4137..1595ed0bc36 100644 --- a/lib/api/project_members.rb +++ b/lib/api/project_members.rb @@ -6,8 +6,8 @@ module API resource :projects do helpers do def handle_project_member_errors(errors) - if errors[:project_access].any? - error!(errors[:project_access], 422) + if errors[:access_level].any? + error!(errors[:access_level], 422) end not_found! end -- cgit v1.2.1 From 4f1bb91a75111188ce52e8fde1c3ac0acfff452e Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 15 Sep 2014 16:45:28 +0300 Subject: Fix finder and tests for new membership models Signed-off-by: Dmitriy Zaporozhets --- app/finders/projects_finder.rb | 2 +- features/steps/admin/groups.rb | 2 +- features/steps/group/group.rb | 2 +- features/steps/project/team_management.rb | 2 +- lib/api/entities.rb | 2 +- spec/mailers/notify_spec.rb | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb index 222496cd317..c81bb51583a 100644 --- a/app/finders/projects_finder.rb +++ b/app/finders/projects_finder.rb @@ -32,7 +32,7 @@ class ProjectsFinder # group.projects.where( "projects.id IN (?) OR projects.visibility_level IN (?)", - projects_members.pluck(:project_id), + projects_members.pluck(:source_id), Project.public_and_internal_levels ) else diff --git a/features/steps/admin/groups.rb b/features/steps/admin/groups.rb index 9c1bcfefb9c..d761584ad93 100644 --- a/features/steps/admin/groups.rb +++ b/features/steps/admin/groups.rb @@ -40,7 +40,7 @@ class AdminGroups < Spinach::FeatureSteps user = User.find_by(name: "John Doe") select2(user.id, from: "#user_ids", multiple: true) within "#new_team_member" do - select "Reporter", from: "group_access" + select "Reporter", from: "access_level" end click_button "Add users into group" end diff --git a/features/steps/group/group.rb b/features/steps/group/group.rb index c3ee42f1127..d0ec503f47b 100644 --- a/features/steps/group/group.rb +++ b/features/steps/group/group.rb @@ -32,7 +32,7 @@ class Groups < Spinach::FeatureSteps click_link 'Add members' within ".users-group-form" do select2(user.id, from: "#user_ids", multiple: true) - select "Reporter", from: "group_access" + select "Reporter", from: "access_level" end click_button "Add users into group" end diff --git a/features/steps/project/team_management.rb b/features/steps/project/team_management.rb index db04e234c84..76a0671721e 100644 --- a/features/steps/project/team_management.rb +++ b/features/steps/project/team_management.rb @@ -23,7 +23,7 @@ class ProjectTeamManagement < Spinach::FeatureSteps user = User.find_by(name: "Mike") select2(user.id, from: "#user_ids", multiple: true) - within "#new_team_member" do + within "#new_project_member" do select "Reporter", from: "access_level" end click_button "Add users" diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 4b14472fd5c..ffa3e8a149e 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -68,7 +68,7 @@ module API class GroupMember < UserBasic expose :access_level do |user, options| - options[:group].group_members.find_by(user_id: user.id).group_access + options[:group].group_members.find_by(user_id: user.id).access_level end end diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 4b64cefa852..799dce442cc 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -506,7 +506,7 @@ describe Notify do describe 'group access changed' do let(:group) { create(:group) } let(:user) { create(:user) } - let(:membership) { create(:users_group, group: group, user: user) } + let(:membership) { create(:group_member, group: group, user: user) } subject { Notify.group_access_granted_email(membership.id) } -- cgit v1.2.1