Add expiration date to group memberships

14 files changed, 58 insertions, 34 deletions

diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js
index 0bdab796daf..f385fe24d0b 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 9fc41a12536..272164cd0cc 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 11c39bbdfe4..c48869ae465 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 043da030468..f9c48a546e6 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 a65d271d262..ab6ea2aae36 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 9bb9f962177..33228f31786 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 da71de4cd1e..742f9d7a433 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 2adbf9277fa..09f53268747 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 956a0026e1a..e9b45823c67 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 334ce7dd3db..e920f5a706b 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 dd9d8515853..64ee511bbd7 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 a45285b9c3d..94c16710d9a 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 5dd38de34d2..430c384ac2e 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 a56ee30f7b1..1e365bf353a 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