diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index 0bdab796daf4fed7496e66beccf68d7eb9a0c5e4..f385fe24d0b315400ec4ad7dca34e5a599270473 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -126,6 +126,7 @@ new NotificationsDropdown(); break; case 'groups:group_members:index': + new MemberExpirationDate(); new GroupMembers(); new UsersSelect(); break; diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index 9fc41a125364f74a8b0f7651fcb4cb995d01d02c..272164cd0ccc8a6cd32222789cb63afeff1222cf 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -21,7 +21,12 @@ class Groups::GroupMembersController < Groups::ApplicationController end def create - @group.add_users(params[:user_ids].split(','), params[:access_level], current_user) + @group.add_users( + params[:user_ids].split(','), + params[:access_level], + current_user: current_user, + expires_at: params[:expires_at] + ) redirect_to group_group_members_path(@group), notice: 'Users were successfully added.' end @@ -63,7 +68,7 @@ class Groups::GroupMembersController < Groups::ApplicationController protected def member_params - params.require(:group_member).permit(:access_level, :user_id) + params.require(:group_member).permit(:access_level, :user_id, :expires_at) end # MembershipActions concern diff --git a/app/models/group.rb b/app/models/group.rb index 11c39bbdfe40ced54a92530ae9ec22606fa1e78d..c48869ae465ea904f7b18b6af8ff120bef03ea12 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -95,34 +95,40 @@ class Group < Namespace end end - def add_users(user_ids, access_level, current_user = nil) + def add_users(user_ids, access_level, current_user: nil, expires_at: nil) user_ids.each do |user_id| - Member.add_user(self.group_members, user_id, access_level, current_user: current_user) + Member.add_user( + self.group_members, + user_id, + access_level, + current_user: current_user, + expires_at: expires_at + ) end end - def add_user(user, access_level, current_user = nil) - add_users([user], access_level, current_user) + def add_user(user, access_level, current_user: nil, expires_at: nil) + add_users([user], access_level, current_user: current_user, expires_at: expires_at) end def add_guest(user, current_user = nil) - add_user(user, Gitlab::Access::GUEST, current_user) + add_user(user, Gitlab::Access::GUEST, current_user: current_user) end def add_reporter(user, current_user = nil) - add_user(user, Gitlab::Access::REPORTER, current_user) + add_user(user, Gitlab::Access::REPORTER, current_user: current_user) end def add_developer(user, current_user = nil) - add_user(user, Gitlab::Access::DEVELOPER, current_user) + add_user(user, Gitlab::Access::DEVELOPER, current_user: current_user) end def add_master(user, current_user = nil) - add_user(user, Gitlab::Access::MASTER, current_user) + add_user(user, Gitlab::Access::MASTER, current_user: current_user) end def add_owner(user, current_user = nil) - add_user(user, Gitlab::Access::OWNER, current_user) + add_user(user, Gitlab::Access::OWNER, current_user: current_user) end def has_owner?(user) diff --git a/app/models/project.rb b/app/models/project.rb index 043da030468c5927c35bd9279dce20038b8dc1d9..f9c48a546e6d1c9aaa0328edfb711e7d00343aba 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1003,8 +1003,8 @@ class Project < ActiveRecord::Base project_members.find_by(user_id: user) end - def add_user(user, access_level, current_user = nil) - team.add_user(user, access_level, current_user) + def add_user(user, access_level, current_user: nil, expires_at: nil) + team.add_user(user, access_level, current_user: current_user, expires_at: expires_at) end def default_branch diff --git a/app/models/project_team.rb b/app/models/project_team.rb index a65d271d26214673a0214ba469b62b2c26698172..ab6ea2aae36b0e362dc1b5900d4c8846d9188493 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -17,7 +17,7 @@ class ProjectTeam if users.respond_to?(:each) add_users(users, access, current_user: current_user) else - add_user(users, access, current_user) + add_user(users, access, current_user: current_user) end end @@ -43,8 +43,8 @@ class ProjectTeam ) end - def add_user(user, access, current_user = nil) - add_users([user], access, current_user: current_user) + def add_user(user, access, current_user: nil, expires_at: nil) + add_users([user], access, current_user: current_user, expires_at: expires_at) end # Remove all users from project team diff --git a/app/views/groups/group_members/_new_group_member.html.haml b/app/views/groups/group_members/_new_group_member.html.haml index 9bb9f96217770455302e3e0b5a0022f442ccaf27..33228f317862f31fea1cd61c733fb86f26e6259b 100644 --- a/app/views/groups/group_members/_new_group_member.html.haml +++ b/app/views/groups/group_members/_new_group_member.html.haml @@ -14,5 +14,12 @@ Read more about role permissions %strong= link_to "here", help_page_path("user/permissions"), class: "vlink" + .form-group + = f.label :expires_at, 'Access expiration date', class: 'control-label' + .col-sm-10 + .clearable-input + = text_field_tag :expires_at, nil, class: 'form-control js-access-expiration-date', placeholder: 'Select access expiration date' + %i.clear-icon.js-clear-input + .form-actions = f.submit 'Add users to group', class: "btn btn-create" diff --git a/app/views/groups/group_members/update.js.haml b/app/views/groups/group_members/update.js.haml index da71de4cd1e52ec2e6e940404770e62d4238905d..742f9d7a433edd00e623c4d5c3d317405244ca1b 100644 --- a/app/views/groups/group_members/update.js.haml +++ b/app/views/groups/group_members/update.js.haml @@ -1,2 +1,3 @@ :plain $("##{dom_id(@group_member)}").replaceWith('#{escape_javascript(render('shared/members/member', member: @group_member))}'); + new MemberExpirationDate(); diff --git a/app/views/shared/members/_member.html.haml b/app/views/shared/members/_member.html.haml index 2adbf9277fa629da2e43a01ddb8b58aa6c9972cc..09f5326874774ab4347e03217983d5985149be90 100644 --- a/app/views/shared/members/_member.html.haml +++ b/app/views/shared/members/_member.html.haml @@ -82,12 +82,11 @@ = label_tag "member_access_level_#{member.id}", 'Project access', class: 'control-label' .col-sm-10 = f.select :access_level, options_for_select(member.class.access_level_roles, member.access_level), {}, class: 'form-control', id: "member_access_level_#{member.id}" - - if member.is_a?(ProjectMember) - .form-group - = label_tag "member_expires_at_#{member.id}", 'Access expiration date', class: 'control-label' - .col-sm-10 - .clearable-input - = f.text_field :expires_at, class: 'form-control js-access-expiration-date', placeholder: 'Select access expiration date', id: "member_expires_at_#{member.id}" - %i.clear-icon.js-clear-input + .form-group + = label_tag "member_expires_at_#{member.id}", 'Access expiration date', class: 'control-label' + .col-sm-10 + .clearable-input + = f.text_field :expires_at, class: 'form-control js-access-expiration-date', placeholder: 'Select access expiration date', id: "member_expires_at_#{member.id}" + %i.clear-icon.js-clear-input .prepend-top-10 = f.submit 'Save', class: 'btn btn-save btn-sm' diff --git a/features/steps/group/members.rb b/features/steps/group/members.rb index 956a0026e1a409c595217c9540d610ed00b8b91b..e9b45823c67a72e95476541fe638bb7962b8a532 100644 --- a/features/steps/group/members.rb +++ b/features/steps/group/members.rb @@ -117,7 +117,7 @@ class Spinach::Features::GroupMembers < Spinach::FeatureSteps page.within "#group_member_#{member.id}" do click_button 'Edit' - select 'Developer', from: 'group_member_access_level' + select 'Developer', from: "member_access_level_#{member.id}" click_on 'Save' end end diff --git a/features/steps/project/team_management.rb b/features/steps/project/team_management.rb index 334ce7dd3dbb37bb96a80a930a747eadcfaa19ac..e920f5a706ba44d60cfb5e78bcbab4f82cda9878 100644 --- a/features/steps/project/team_management.rb +++ b/features/steps/project/team_management.rb @@ -66,7 +66,7 @@ class Spinach::Features::ProjectTeamManagement < Spinach::FeatureSteps project_member = project.project_members.find_by(user_id: user.id) page.within "#project_member_#{project_member.id}" do click_button 'Edit' - select "Reporter", from: "project_member_access_level" + select "Reporter", from: "member_access_level_#{project_member.id}" click_button "Save" end end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index dd9d85158535e1a851024dd0d31290d6a32cc857..64ee511bbd75709eb89605c5fe0626d4e94f04d6 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -97,6 +97,10 @@ module API member = options[:member] || options[:members].find { |m| m.user_id == user.id } member.access_level end + expose :expires_at do |user, options| + member = options[:member] || options[:members].find { |m| m.user_id == user.id } + member.expires_at + end end class AccessRequester < UserBasic @@ -104,9 +108,6 @@ module API access_requester = options[:access_requester] || options[:access_requesters].find { |m| m.user_id == user.id } access_requester.requested_at end - expose :expires_at do |user, options| - options[:project].project_members.find_by(user_id: user.id).expires_at - end end class Group < Grape::Entity diff --git a/lib/api/members.rb b/lib/api/members.rb index a45285b9c3da10cd567f3aba4eab9751831c4c91..94c16710d9a5b5cf0fa1ef89c90836ba1a11ebc9 100644 --- a/lib/api/members.rb +++ b/lib/api/members.rb @@ -49,6 +49,7 @@ module API # id (required) - The group/project ID # user_id (required) - The user ID of the new member # access_level (required) - A valid access level + # expires_at (optional) - Date string in the format YEAR-MONTH-DAY # # Example Request: # POST /groups/:id/members @@ -72,7 +73,7 @@ module API conflict!('Member already exists') if source_type == 'group' && member unless member - source.add_user(params[:user_id], params[:access_level], current_user) + source.add_user(params[:user_id], params[:access_level], current_user: current_user, expires_at: params[:expires_at]) member = source.members.find_by(user_id: params[:user_id]) end @@ -81,7 +82,7 @@ module API else # Since `source.add_user` doesn't return a member object, we have to # build a new one and populate its errors in order to render them. - member = source.members.build(attributes_for_keys([:user_id, :access_level])) + member = source.members.build(attributes_for_keys([:user_id, :access_level, :expires_at])) member.valid? # populate the errors # This is to ensure back-compatibility but 400 behavior should be used @@ -97,6 +98,7 @@ module API # id (required) - The group/project ID # user_id (required) - The user ID of the member # access_level (required) - A valid access level + # expires_at (optional) - Date string in the format YEAR-MONTH-DAY # # Example Request: # PUT /groups/:id/members/:user_id @@ -107,7 +109,7 @@ module API required_attributes! [:user_id, :access_level] member = source.members.find_by!(user_id: params[:user_id]) - attrs = attributes_for_keys [:access_level] + attrs = attributes_for_keys [:access_level, :expires_at] if member.update_attributes(attrs) present member.user, with: Entities::Member, member: member diff --git a/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb b/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb index 5dd38de34d288889b75f74288b8c583f8e989af9..430c384ac2ee824412a7051d55d80b927af3e6f5 100644 --- a/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb +++ b/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb @@ -19,7 +19,7 @@ feature 'Projects > Members > Master adds member with expiration date', feature: page.within '.users-project-form' do select2(new_member.id, from: '#user_ids', multiple: true) - fill_in 'Access expiration date', with: '2016-08-10' + fill_in 'expires_at', with: '2016-08-10' click_on 'Add users to project' end diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index a56ee30f7b15eb0bc8b9000cdb14491e568eff35..1e365bf353a9e133de342456237ef9a65e379021 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -122,12 +122,13 @@ describe API::Members, api: true do it 'creates a new member' do expect do post api("/#{source_type.pluralize}/#{source.id}/members", master), - user_id: stranger.id, access_level: Member::DEVELOPER + user_id: stranger.id, access_level: Member::DEVELOPER, expires_at: '2016-08-05' expect(response).to have_http_status(201) end.to change { source.members.count }.by(1) expect(json_response['id']).to eq(stranger.id) expect(json_response['access_level']).to eq(Member::DEVELOPER) + expect(json_response['expires_at']).to eq('2016-08-05') end end @@ -183,11 +184,12 @@ describe API::Members, api: true do context 'when authenticated as a master/owner' do it 'updates the member' do put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", master), - access_level: Member::MASTER + access_level: Member::MASTER, expires_at: '2016-08-05' expect(response).to have_http_status(200) expect(json_response['id']).to eq(developer.id) expect(json_response['access_level']).to eq(Member::MASTER) + expect(json_response['expires_at']).to eq('2016-08-05') end end