From 31fc73f0a9b9225ba3737b9525fcf7a1695a45f2 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 13 Mar 2015 16:22:03 +0100 Subject: [PATCH] Use `project_member` instead of `team_member`. --- app/assets/javascripts/dispatcher.js.coffee | 2 +- app/assets/stylesheets/generic/common.scss | 2 +- app/assets/stylesheets/pages/projects.scss | 2 +- app/controllers/admin/groups_controller.rb | 4 +- .../projects/project_members_controller.rb | 88 +++++++++++++++++++ .../projects/team_members_controller.rb | 73 --------------- app/helpers/search_helper.rb | 2 +- app/helpers/tab_helper.rb | 2 +- app/models/ability.rb | 6 +- app/models/members/project_member.rb | 4 +- app/models/project.rb | 4 +- app/models/user.rb | 5 +- app/services/notification_service.rb | 12 +-- app/services/projects/participants_service.rb | 4 +- app/views/admin/groups/show.html.haml | 2 +- app/views/admin/projects/show.html.haml | 4 +- app/views/groups/projects.html.haml | 2 +- app/views/projects/_dropdown.html.haml | 2 +- app/views/projects/_settings_nav.html.haml | 2 +- config/routes.rb | 4 +- features/steps/admin/groups.rb | 2 +- lib/api/project_members.rb | 28 +++--- lib/gitlab/markdown.rb | 2 +- spec/helpers/gitlab_markdown_helper_spec.rb | 2 +- spec/routing/project_routing_spec.rb | 17 ++-- 25 files changed, 143 insertions(+), 134 deletions(-) create mode 100644 app/controllers/projects/project_members_controller.rb delete mode 100644 app/controllers/projects/team_members_controller.rb diff --git a/app/assets/javascripts/dispatcher.js.coffee b/app/assets/javascripts/dispatcher.js.coffee index e1015a63d52..5da774cfb23 100644 --- a/app/assets/javascripts/dispatcher.js.coffee +++ b/app/assets/javascripts/dispatcher.js.coffee @@ -127,7 +127,7 @@ class Dispatcher new DropzoneInput($('.wiki-form')) when 'snippets', 'labels', 'graphs' shortcut_handler = new ShortcutsNavigation() - when 'team_members', 'deploy_keys', 'hooks', 'services', 'protected_branches' + when 'project_members', 'deploy_keys', 'hooks', 'services', 'protected_branches' shortcut_handler = new ShortcutsNavigation() new UsersSelect() diff --git a/app/assets/stylesheets/generic/common.scss b/app/assets/stylesheets/generic/common.scss index af8e90eb1a9..876eea72e8a 100644 --- a/app/assets/stylesheets/generic/common.scss +++ b/app/assets/stylesheets/generic/common.scss @@ -167,7 +167,7 @@ li.note { background-color: inherit; } -.team_member_show { +.project_member_show { td:first-child { color: #aaa; } diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index e359aa45025..74755951670 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -156,7 +156,7 @@ ul.nav.nav-projects-tabs { } } -.team_member_row form { +.project_member_row form { margin: 0px; } diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index e338abeac4c..9d9adaa467f 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -1,5 +1,5 @@ class Admin::GroupsController < Admin::ApplicationController - before_filter :group, only: [:edit, :show, :update, :destroy, :project_update, :project_teams_update] + before_filter :group, only: [:edit, :show, :update, :destroy, :project_update, :members_update] def index @groups = Group.all @@ -40,7 +40,7 @@ class Admin::GroupsController < Admin::ApplicationController end end - def project_teams_update + def members_update @group.add_users(params[:user_ids].split(','), params[:access_level]) redirect_to [:admin, @group], notice: 'Users were successfully added.' diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb new file mode 100644 index 00000000000..4ab15db01f7 --- /dev/null +++ b/app/controllers/projects/project_members_controller.rb @@ -0,0 +1,88 @@ +class Projects::ProjectMembersController < Projects::ApplicationController + # Authorize + before_filter :authorize_admin_project!, except: :leave + + layout "project_settings" + + def index + @project_members = @project.project_members + + if params[:search].present? + users = @project.users.search(params[:search]).to_a + @project_members = @project_members.where(user_id: users) + end + + @project_members = @project_members.order('access_level DESC') + + @group = @project.group + if @group + @group_members = @group.group_members + + if params[:search].present? + users = @group.users.search(params[:search]).to_a + @group_members = @group_members.where(user_id: users) + end + + @group_members = @group_members.order('access_level DESC').limit(20) + end + + @project_member = @project.project_members.new + end + + def new + @project_member = @project.project_members.new + end + + def create + users = User.where(id: params[:user_ids].split(',')) + @project.team << [users, params[:access_level]] + + redirect_to namespace_project_project_members_path(@project.namespace, @project) + end + + def update + @project_member = @project.project_members.find_by(user_id: member) + @project_member.update_attributes(member_params) + end + + def destroy + @project_member = @project.project_members.find_by(user_id: member) + @project_member.destroy + + respond_to do |format| + format.html do + redirect_to namespace_project_project_members_path(@project.namespace, + @project) + end + format.js { render nothing: true } + end + end + + def leave + @project.project_members.find_by(user_id: current_user).destroy + + respond_to do |format| + format.html { redirect_to :back } + format.js { render nothing: true } + end + end + + def apply_import + giver = Project.find(params[:source_project_id]) + status = @project.team.import(giver) + notice = status ? "Successfully imported" : "Import failed" + + redirect_to(namespace_project_project_members_path(project.namespace, project), + notice: notice) + end + + protected + + def member + @member ||= User.find_by(username: params[:id]) + end + + def member_params + params.require(:project_member).permit(:user_id, :access_level) + end +end diff --git a/app/controllers/projects/team_members_controller.rb b/app/controllers/projects/team_members_controller.rb deleted file mode 100644 index f8a248ed729..00000000000 --- a/app/controllers/projects/team_members_controller.rb +++ /dev/null @@ -1,73 +0,0 @@ -class Projects::TeamMembersController < Projects::ApplicationController - # Authorize - before_filter :authorize_admin_project!, except: :leave - - layout "project_settings" - - def index - @group = @project.group - @project_members = @project.project_members.order('access_level DESC') - end - - def new - @user_project_relation = @project.project_members.new - end - - def create - users = User.where(id: params[:user_ids].split(',')) - @project.team << [users, params[:access_level]] - - redirect_to namespace_project_team_index_path(@project.namespace, @project) - end - - def update - @user_project_relation = @project.project_members.find_by(user_id: member) - @user_project_relation.update_attributes(member_params) - - unless @user_project_relation.valid? - flash[:alert] = "User should have at least one role" - end - redirect_to namespace_project_team_index_path(@project.namespace, @project) - end - - def destroy - @user_project_relation = @project.project_members.find_by(user_id: member) - @user_project_relation.destroy - - respond_to do |format| - format.html do - redirect_to namespace_project_team_index_path(@project.namespace, - @project) - end - format.js { render nothing: true } - end - end - - def leave - @project.project_members.find_by(user_id: current_user).destroy - - respond_to do |format| - format.html { redirect_to :back } - format.js { render nothing: true } - end - end - - def apply_import - giver = Project.find(params[:source_project_id]) - status = @project.team.import(giver) - notice = status ? "Successfully imported" : "Import failed" - - redirect_to(namespace_project_team_index_path(project.namespace, project), - notice: notice) - end - - protected - - def member - @member ||= User.find_by(username: params[:id]) - end - - def member_params - params.require(:project_member).permit(:user_id, :access_level) - end -end diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index cb829037697..7d3fcfa7037 100644 --- a/app/helpers/search_helper.rb +++ b/app/helpers/search_helper.rb @@ -60,7 +60,7 @@ module SearchHelper { label: "#{prefix} - Merge Requests", url: namespace_project_merge_requests_path(@project.namespace, @project) }, { label: "#{prefix} - Milestones", url: namespace_project_milestones_path(@project.namespace, @project) }, { label: "#{prefix} - Snippets", url: namespace_project_snippets_path(@project.namespace, @project) }, - { label: "#{prefix} - Team", url: namespace_project_team_index_path(@project.namespace, @project) }, + { label: "#{prefix} - Members", url: namespace_project_project_members_path(@project.namespace, @project) }, { label: "#{prefix} - Wiki", url: namespace_project_wikis_path(@project.namespace, @project) }, ] else diff --git a/app/helpers/tab_helper.rb b/app/helpers/tab_helper.rb index 7a401a274d3..a1d263d9d3a 100644 --- a/app/helpers/tab_helper.rb +++ b/app/helpers/tab_helper.rb @@ -89,7 +89,7 @@ module TabHelper def project_tab_class return "active" if current_page?(controller: "/projects", action: :edit, id: @project) - if ['services', 'hooks', 'deploy_keys', 'team_members', 'protected_branches'].include? controller.controller_name + if ['services', 'hooks', 'deploy_keys', 'project_members', 'protected_branches'].include? controller.controller_name "active" end end diff --git a/app/models/ability.rb b/app/models/ability.rb index 773b51a7bcd..855134dd39b 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -37,7 +37,7 @@ class Ability :read_issue, :read_milestone, :read_project_snippet, - :read_team_member, + :read_project_member, :read_merge_request, :read_note, :download_code @@ -119,7 +119,7 @@ class Ability :read_issue, :read_milestone, :read_project_snippet, - :read_team_member, + :read_project_member, :read_merge_request, :read_note, :write_project, @@ -166,7 +166,7 @@ class Ability :admin_issue, :admin_milestone, :admin_project_snippet, - :admin_team_member, + :admin_project_member, :admin_merge_request, :admin_note, :admin_wiki, diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb index e4791d0f0aa..6b13e0ff30b 100644 --- a/app/models/members/project_member.rb +++ b/app/models/members/project_member.rb @@ -116,14 +116,14 @@ class ProjectMember < Member def post_create_hook unless owner? event_service.join_project(self.project, self.user) - notification_service.new_team_member(self) + notification_service.new_project_member(self) end system_hook_service.execute_hooks_for(self, :create) end def post_update_hook - notification_service.update_team_member(self) if self.access_level_changed? + notification_service.update_project_member(self) if self.access_level_changed? end def post_destroy_hook diff --git a/app/models/project.rb b/app/models/project.rb index c45338bf4eb..f0b416c8f4a 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -445,13 +445,13 @@ class Project < ActiveRecord::Base end end - def team_member_by_name_or_email(name = nil, email = nil) + def project_member_by_name_or_email(name = nil, email = nil) user = users.where('name like ? or email like ?', name, email).first project_members.where(user: user) if user end # Get Team Member record by user id - def team_member_by_id(user_id) + def project_member_by_id(user_id) project_members.find_by(user_id: user_id) end diff --git a/app/models/user.rb b/app/models/user.rb index 0d40ac8309e..ba325132df8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -169,11 +169,8 @@ class User < ActiveRecord::Base scope :admins, -> { where(admin: true) } scope :blocked, -> { with_state(:blocked) } scope :active, -> { with_state(:active) } - 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 members)') } - scope :potential_team_members, ->(team) { team.members.any? ? active.not_in_team(team) : active } # # Class methods @@ -407,7 +404,7 @@ class User < ActiveRecord::Base end def tm_of(project) - project.team_member_by_id(self.id) + project.project_member_by_id(self.id) end def already_forked?(project) diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 843cb0c5ee0..fb5baaf74b3 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -162,20 +162,20 @@ class NotificationService end end - def new_team_member(project_member) + def new_project_member(project_member) mailer.project_access_granted_email(project_member.id) end - def update_team_member(project_member) + def update_project_member(project_member) mailer.project_access_granted_email(project_member.id) end - def new_group_member(users_group) - mailer.group_access_granted_email(users_group.id) + def new_group_member(group_member) + mailer.group_access_granted_email(group_member.id) end - def update_group_member(users_group) - mailer.group_access_granted_email(users_group.id) + def update_group_member(group_member) + mailer.group_access_granted_email(group_member.id) end def project_was_moved(project) diff --git a/app/services/projects/participants_service.rb b/app/services/projects/participants_service.rb index f6f9aceef95..bcbacbff562 100644 --- a/app/services/projects/participants_service.rb +++ b/app/services/projects/participants_service.rb @@ -12,8 +12,8 @@ module Projects else [] end - team_members = sorted(@project.team.members) - participants = all_members + groups + team_members + participating + project_members = sorted(@project.team.members) + participants = all_members + groups + project_members + participating participants.uniq end diff --git a/app/views/admin/groups/show.html.haml b/app/views/admin/groups/show.html.haml index a28eae38925..7d292118075 100644 --- a/app/views/admin/groups/show.html.haml +++ b/app/views/admin/groups/show.html.haml @@ -58,7 +58,7 @@ Read more about project permissions %strong= link_to "here", help_page_path("permissions", "permissions"), class: "vlink" - = form_tag project_teams_update_admin_group_path(@group), id: "new_team_member", class: "bulk_import", method: :put do + = form_tag members_update_admin_group_path(@group), id: "new_project_member", class: "bulk_import", method: :put do %div = users_select_tag(:user_ids, multiple: true) %div.prepend-top-10 diff --git a/app/views/admin/projects/show.html.haml b/app/views/admin/projects/show.html.haml index ebb3b3a636e..70ebc9561d4 100644 --- a/app/views/admin/projects/show.html.haml +++ b/app/views/admin/projects/show.html.haml @@ -114,7 +114,7 @@ = link_to namespace_project_team_index_path(@project.namespace, @project), class: "btn btn-xs" do %i.fa.fa-pencil-square-o Manage Access - %ul.well-list.team_members + %ul.well-list.project_members - @project_members.each do |project_member| - user = project_member.user %li.project_member @@ -126,7 +126,7 @@ %span.light Owner - else %span.light= project_member.human_access - = link_to namespace_project_team_member_path(@project.namespace, @project, user), data: { confirm: remove_from_project_team_message(@project, user)}, method: :delete, remote: true, class: "btn btn-sm btn-remove" do + = link_to namespace_project_project_member_path(@project.namespace, @project, user), data: { confirm: remove_from_project_team_message(@project, user)}, method: :delete, remote: true, class: "btn btn-sm btn-remove" do %i.fa.fa-times .panel-footer = paginate @project_members, param_name: 'project_members_page', theme: 'gitlab' diff --git a/app/views/groups/projects.html.haml b/app/views/groups/projects.html.haml index 3b8c26ed391..dd1fa3840d5 100644 --- a/app/views/groups/projects.html.haml +++ b/app/views/groups/projects.html.haml @@ -16,7 +16,7 @@ %span.label.label-gray = repository_size(project) .pull-right - = link_to 'Members', namespace_project_team_index_path(project.namespace, project), id: "edit_#{dom_id(project)}", class: "btn btn-sm" + = link_to 'Members', namespace_project_project_members_path(project.namespace, project), id: "edit_#{dom_id(project)}", class: "btn btn-sm" = link_to 'Edit', edit_namespace_project_path(project.namespace, project), id: "edit_#{dom_id(project)}", class: "btn btn-sm" = link_to 'Remove', project, data: { confirm: remove_project_message(project)}, method: :delete, class: "btn btn-sm btn-remove" - if @projects.blank? diff --git a/app/views/projects/_dropdown.html.haml b/app/views/projects/_dropdown.html.haml index 2d5120f283b..3cdbbb7b043 100644 --- a/app/views/projects/_dropdown.html.haml +++ b/app/views/projects/_dropdown.html.haml @@ -15,7 +15,7 @@ %li = link_to new_namespace_project_snippet_path(@project.namespace, @project), title: "New Snippet" do New snippet - - if can?(current_user, :admin_team_member, @project) + - if can?(current_user, :admin_project_member, @project) %li = link_to new_namespace_project_team_member_path(@project.namespace, @project), title: "New project member" do New project member diff --git a/app/views/projects/_settings_nav.html.haml b/app/views/projects/_settings_nav.html.haml index 7fc3d44034f..6f19206b17f 100644 --- a/app/views/projects/_settings_nav.html.haml +++ b/app/views/projects/_settings_nav.html.haml @@ -4,8 +4,8 @@ %i.fa.fa-pencil-square-o %span Project - = nav_link(controller: [:team_members, :teams]) do = link_to namespace_project_team_index_path(@project.namespace, @project), title: 'Members', class: "team-tab tab" do + = nav_link(controller: [:project_members, :teams]) do %i.fa.fa-users %span Members diff --git a/config/routes.rb b/config/routes.rb index 889995e92a6..547d1aa8660 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -136,7 +136,7 @@ Gitlab::Application.routes.draw do resources :groups, constraints: { id: /[^\/]+/ } do member do - put :project_teams_update + put :members_update end end @@ -445,7 +445,7 @@ Gitlab::Application.routes.draw do end end - resources :team_members, except: [:index, :edit], constraints: { id: /[a-zA-Z.\/0-9_\-#%+]+/ } do + resources :project_members, except: [:new, :edit], constraints: { id: /[a-zA-Z.\/0-9_\-#%+]+/ } do collection do delete :leave diff --git a/features/steps/admin/groups.rb b/features/steps/admin/groups.rb index 6bcec48be88..721460b9371 100644 --- a/features/steps/admin/groups.rb +++ b/features/steps/admin/groups.rb @@ -38,7 +38,7 @@ class Spinach::Features::AdminGroups < Spinach::FeatureSteps When 'I select user "John Doe" from user list as "Reporter"' do select2(user_john.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 to group" diff --git a/lib/api/project_members.rb b/lib/api/project_members.rb index 73cf062155b..c756bb479fc 100644 --- a/lib/api/project_members.rb +++ b/lib/api/project_members.rb @@ -46,19 +46,19 @@ module API required_attributes! [:user_id, :access_level] # 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.project_members.new( + project_member = user_project.project_member_by_id(params[:user_id]) + if project_member.nil? + project_member = user_project.project_members.new( user_id: params[:user_id], access_level: params[:access_level] ) end - if team_member.save - @member = team_member.user + if project_member.save + @member = project_member.user present @member, with: Entities::ProjectMember, project: user_project else - handle_member_errors team_member.errors + handle_member_errors project_member.errors end end @@ -74,14 +74,14 @@ module API authorize! :admin_project, user_project required_attributes! [:access_level] - team_member = user_project.project_members.find_by(user_id: params[:user_id]) - not_found!("User can not be found") if team_member.nil? + project_member = user_project.project_members.find_by(user_id: params[:user_id]) + not_found!("User can not be found") if project_member.nil? - if team_member.update_attributes(access_level: params[:access_level]) - @member = team_member.user + if project_member.update_attributes(access_level: params[:access_level]) + @member = project_member.user present @member, with: Entities::ProjectMember, project: user_project else - handle_member_errors team_member.errors + handle_member_errors project_member.errors end end @@ -94,9 +94,9 @@ module API # DELETE /projects/:id/members/:user_id delete ":id/members/:user_id" do authorize! :admin_project, user_project - team_member = user_project.project_members.find_by(user_id: params[:user_id]) - unless team_member.nil? - team_member.destroy + project_member = user_project.project_members.find_by(user_id: params[:user_id]) + unless project_member.nil? + project_member.destroy else { message: "Access revoked", id: params[:user_id].to_i } end diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb index 2dfa18da482..c3a8d90ef54 100644 --- a/lib/gitlab/markdown.rb +++ b/lib/gitlab/markdown.rb @@ -200,7 +200,7 @@ module Gitlab def reference_user(identifier, project = @project, _ = nil) options = html_options.merge( - class: "gfm gfm-team_member #{html_options[:class]}" + class: "gfm gfm-project_member #{html_options[:class]}" ) if identifier == "all" diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb index fd80c615221..6ba27b536e4 100644 --- a/spec/helpers/gitlab_markdown_helper_spec.rb +++ b/spec/helpers/gitlab_markdown_helper_spec.rb @@ -180,7 +180,7 @@ describe GitlabMarkdownHelper do end it "should include standard gfm classes" do - expect(gfm(actual)).to match(/class="\s?gfm gfm-team_member\s?"/) + expect(gfm(actual)).to match(/class="\s?gfm gfm-project_member\s?"/) end end diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index 4308a765b56..d9bd91f5990 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -338,17 +338,14 @@ describe Projects::CommitsController, 'routing' do end end -# project_team_members GET /:project_id/team_members(.:format) team_members#index -# POST /:project_id/team_members(.:format) team_members#create -# new_project_team_member GET /:project_id/team_members/new(.:format) team_members#new -# edit_project_team_member GET /:project_id/team_members/:id/edit(.:format) team_members#edit -# project_team_member GET /:project_id/team_members/:id(.:format) team_members#show -# PUT /:project_id/team_members/:id(.:format) team_members#update -# DELETE /:project_id/team_members/:id(.:format) team_members#destroy -describe Projects::TeamMembersController, 'routing' do +# project_project_members GET /:project_id/project_members(.:format) project_members#index +# POST /:project_id/project_members(.:format) project_members#create +# PUT /:project_id/project_members/:id(.:format) project_members#update +# DELETE /:project_id/project_members/:id(.:format) project_members#destroy +describe Projects::ProjectMembersController, 'routing' do it_behaves_like 'RESTful project resources' do - let(:actions) { [:new, :create, :update, :destroy] } - let(:controller) { 'team_members' } + let(:actions) { [:index, :create, :update, :destroy] } + let(:controller) { 'project_members' } end end