diff options
-rw-r--r-- | app/assets/javascripts/dispatcher.js | 1 | ||||
-rw-r--r-- | app/controllers/projects/group_links_controller.rb | 4 | ||||
-rw-r--r-- | app/helpers/members_helper.rb | 4 | ||||
-rw-r--r-- | app/models/concerns/expirable.rb | 15 | ||||
-rw-r--r-- | app/models/member.rb | 6 | ||||
-rw-r--r-- | app/models/project_group_link.rb | 4 | ||||
-rw-r--r-- | app/views/projects/group_links/index.html.haml | 9 | ||||
-rw-r--r-- | app/views/shared/members/_member.html.haml | 2 | ||||
-rw-r--r-- | app/workers/remove_expired_group_links_worker.rb | 7 | ||||
-rw-r--r-- | config/initializers/1_settings.rb | 3 | ||||
-rw-r--r-- | db/migrate/20160818205718_add_expires_at_to_project_group_links.rb | 29 | ||||
-rw-r--r-- | db/schema.rb | 3 | ||||
-rw-r--r-- | doc/workflow/share_projects_with_other_groups.md | 18 | ||||
-rw-r--r-- | spec/features/projects/group_links_spec.rb | 32 | ||||
-rw-r--r-- | spec/workers/remove_expired_group_links_worker_spec.rb | 24 | ||||
-rw-r--r-- | spec/workers/remove_expired_members_worker_spec.rb | 8 |
16 files changed, 144 insertions, 25 deletions
diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index f385fe24d0b..78713d379e7 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -173,6 +173,7 @@ new BuildArtifacts(); break; case 'projects:group_links:index': + new MemberExpirationDate(); new GroupsSelect(); break; case 'search:show': diff --git a/app/controllers/projects/group_links_controller.rb b/app/controllers/projects/group_links_controller.rb index 606552fa853..d0c4550733c 100644 --- a/app/controllers/projects/group_links_controller.rb +++ b/app/controllers/projects/group_links_controller.rb @@ -11,7 +11,9 @@ class Projects::GroupLinksController < Projects::ApplicationController return render_404 unless can?(current_user, :read_group, group) project.project_group_links.create( - group: group, group_access: params[:link_group_access] + group: group, + group_access: params[:link_group_access], + expires_at: params[:expires_at] ) redirect_to namespace_project_group_links_path(project.namespace, project) diff --git a/app/helpers/members_helper.rb b/app/helpers/members_helper.rb index e9f6172f802..877c77050be 100644 --- a/app/helpers/members_helper.rb +++ b/app/helpers/members_helper.rb @@ -36,8 +36,4 @@ module MembersHelper "Are you sure you want to leave the " \ "\"#{member_source.human_name}\" #{member_source.class.to_s.humanize(capitalize: false)}?" end - - def member_expires_soon?(member) - member.expires_at < 7.days.from_now - end end diff --git a/app/models/concerns/expirable.rb b/app/models/concerns/expirable.rb new file mode 100644 index 00000000000..be93435453b --- /dev/null +++ b/app/models/concerns/expirable.rb @@ -0,0 +1,15 @@ +module Expirable + extend ActiveSupport::Concern + + included do + scope :expired, -> { where('expires_at <= ?', Time.current) } + end + + def expires? + expires_at.present? + end + + def expires_soon? + expires_at < 7.days.from_now + end +end diff --git a/app/models/member.rb b/app/models/member.rb index 84bbbffe718..64e0d33fb20 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -1,6 +1,7 @@ class Member < ActiveRecord::Base include Sortable include Importable + include Expirable include Gitlab::Access attr_accessor :raw_invite_token @@ -31,7 +32,6 @@ class Member < ActiveRecord::Base scope :non_invite, -> { where(invite_token: nil) } scope :request, -> { where.not(requested_at: nil) } scope :has_access, -> { where('access_level > 0') } - scope :expired, -> { where('expires_at <= ?', Time.current) } scope :guests, -> { where(access_level: GUEST) } scope :reporters, -> { where(access_level: REPORTER) } @@ -125,10 +125,6 @@ class Member < ActiveRecord::Base invite? || request? end - def expires? - expires_at.present? - end - def accept_request return false unless request? diff --git a/app/models/project_group_link.rb b/app/models/project_group_link.rb index e52a6bd7c84..7613cbdea93 100644 --- a/app/models/project_group_link.rb +++ b/app/models/project_group_link.rb @@ -1,4 +1,6 @@ class ProjectGroupLink < ActiveRecord::Base + include Expirable + GUEST = 10 REPORTER = 20 DEVELOPER = 30 @@ -26,7 +28,7 @@ class ProjectGroupLink < ActiveRecord::Base self.class.access_options.key(self.group_access) end - private + private def different_group if self.group && self.project && self.project.group == self.group diff --git a/app/views/projects/group_links/index.html.haml b/app/views/projects/group_links/index.html.haml index 2b904544f28..7a93b87df1c 100644 --- a/app/views/projects/group_links/index.html.haml +++ b/app/views/projects/group_links/index.html.haml @@ -17,6 +17,11 @@ .select-wrapper = select_tag :link_group_access, options_for_select(ProjectGroupLink.access_options, ProjectGroupLink.default_access), class: "form-control select-control" %span.caret + .form-group + = label_tag :expires_at, 'Access expiration date', class: 'label-light' + .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 = submit_tag "Share", class: "btn btn-create" .col-lg-9.col-lg-offset-3 %hr @@ -35,6 +40,10 @@ = group.name %br up to #{group_link.human_access} + - if group_link.expires? + · + %span{ class: ('text-warning' if group_link.expires_soon?) } + expires in #{distance_of_time_in_words_to_now(group_link.expires_at)} .pull-right = link_to namespace_project_group_link_path(@project.namespace, @project, group_link), method: :delete, class: "btn btn-transparent" do %span.sr-only disable sharing diff --git a/app/views/shared/members/_member.html.haml b/app/views/shared/members/_member.html.haml index 09f53268747..5f20e4bd42a 100644 --- a/app/views/shared/members/_member.html.haml +++ b/app/views/shared/members/_member.html.haml @@ -61,7 +61,7 @@ Joined #{time_ago_with_tooltip(member.created_at)} - if member.expires? · - %span{ class: ('text-warning' if member_expires_soon?(member)) } + %span{ class: ('text-warning' if member.expires_soon?) } Expires in #{distance_of_time_in_words_to_now(member.expires_at)} - else diff --git a/app/workers/remove_expired_group_links_worker.rb b/app/workers/remove_expired_group_links_worker.rb new file mode 100644 index 00000000000..246c8b6650a --- /dev/null +++ b/app/workers/remove_expired_group_links_worker.rb @@ -0,0 +1,7 @@ +class RemoveExpiredGroupLinksWorker + include Sidekiq::Worker + + def perform + ProjectGroupLink.expired.destroy_all + end +end diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 54fa6a2c679..a9139555907 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -296,6 +296,9 @@ Settings.cron_jobs['requests_profiles_worker']['job_class'] = 'RequestsProfilesW Settings.cron_jobs['remove_expired_members_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['remove_expired_members_worker']['cron'] ||= '10 0 * * *' Settings.cron_jobs['remove_expired_members_worker']['job_class'] = 'RemoveExpiredMembersWorker' +Settings.cron_jobs['remove_expired_members_worker'] ||= Settingslogic.new({}) +Settings.cron_jobs['remove_expired_members_worker']['cron'] ||= '10 0 * * *' +Settings.cron_jobs['remove_expired_members_worker']['job_class'] = 'RemoveExpiredGroupLinksWorker' # # GitLab Shell diff --git a/db/migrate/20160818205718_add_expires_at_to_project_group_links.rb b/db/migrate/20160818205718_add_expires_at_to_project_group_links.rb new file mode 100644 index 00000000000..0ed538b0df8 --- /dev/null +++ b/db/migrate/20160818205718_add_expires_at_to_project_group_links.rb @@ -0,0 +1,29 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddExpiresAtToProjectGroupLinks < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index" or "add_column_with_default" + # you must disable the use of transactions as these methods can not run in an + # existing transaction. When using "add_concurrent_index" make sure that this + # method is the _only_ method called in the migration, any other changes + # should go in a separate migration. This ensures that upon failure _only_ the + # index creation fails and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def change + add_column :project_group_links, :expires_at, :date + end +end diff --git a/db/schema.rb b/db/schema.rb index ae6433494d0..02964d71351 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: 20160810142633) do +ActiveRecord::Schema.define(version: 20160818205718) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -780,6 +780,7 @@ ActiveRecord::Schema.define(version: 20160810142633) do t.datetime "created_at" t.datetime "updated_at" t.integer "group_access", default: 30, null: false + t.date "expires_at" end create_table "project_import_data", force: :cascade do |t| diff --git a/doc/workflow/share_projects_with_other_groups.md b/doc/workflow/share_projects_with_other_groups.md index 4c59f59c587..8e50cb03e63 100644 --- a/doc/workflow/share_projects_with_other_groups.md +++ b/doc/workflow/share_projects_with_other_groups.md @@ -1,22 +1,24 @@ # Share Projects with other Groups -In GitLab Enterprise Edition you can share projects with other groups. -This makes it possible to add a group of users to a project with a single action. +You can share projects with other groups. This makes it possible to add a group of users +to a project with a single action. ## Groups as collections of users -In GitLab Community Edition groups are used primarily to [create collections of projects](groups.md). -In GitLab Enterprise Edition you can also take advantage of the fact that groups define collections of _users_, namely the group members. +Groups are used primarily to [create collections of projects](groups.md), but you can also +take advantage of the fact that groups define collections of _users_, namely the group +members. ## Sharing a project with a group of users -The primary mechanism to give a group of users, say 'Engineering', access to a project, say 'Project Acme', in GitLab is to make the 'Engineering' group the owner of 'Project Acme'. -But what if 'Project Acme' already belongs to another group, say 'Open Source'? -This is where the (Enterprise Edition only) group sharing feature can be of use. +The primary mechanism to give a group of users, say 'Engineering', access to a project, +say 'Project Acme', in GitLab is to make the 'Engineering' group the owner of 'Project +Acme'. But what if 'Project Acme' already belongs to another group, say 'Open Source'? +This is where the group sharing feature can be of use. To share 'Project Acme' with the 'Engineering' group, go to the project settings page for 'Project Acme' and use the left navigation menu to go to the 'Groups' section. -![The 'Groups' section in the project settings screen (Enterprise Edition only)](groups/share_project_with_groups.png) +![The 'Groups' section in the project settings screen](groups/share_project_with_groups.png) Now you can add the 'Engineering' group with the maximum access level of your choice. After sharing 'Project Acme' with 'Engineering', the project is listed on the group dashboard. diff --git a/spec/features/projects/group_links_spec.rb b/spec/features/projects/group_links_spec.rb new file mode 100644 index 00000000000..1a71a03fbd9 --- /dev/null +++ b/spec/features/projects/group_links_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +feature 'Project group links', feature: true, js: true do + include Select2Helper + + let(:master) { create(:user) } + let(:project) { create(:project) } + let!(:group) { create(:group) } + + background do + project.team << [master, :master] + login_as(master) + end + + context 'setting an expiration date for a group link' do + before do + visit namespace_project_group_links_path(project.namespace, project) + + select2 group.id, from: '#link_group_id' + fill_in 'expires_at', with: (Time.current + 4.5.days).strftime('%Y-%m-%d') + page.find('body').click + click_on 'Share' + end + + it 'shows the expiration time with a warning class' do + page.within('.enabled-groups') do + expect(page).to have_content('expires in 4 days') + expect(page).to have_selector('.text-warning') + end + end + end +end diff --git a/spec/workers/remove_expired_group_links_worker_spec.rb b/spec/workers/remove_expired_group_links_worker_spec.rb new file mode 100644 index 00000000000..689bc3d27b4 --- /dev/null +++ b/spec/workers/remove_expired_group_links_worker_spec.rb @@ -0,0 +1,24 @@ +require 'spec_helper' + +describe RemoveExpiredGroupLinksWorker do + describe '#perform' do + let!(:expired_project_group_link) { create(:project_group_link, expires_at: 1.hour.ago) } + let!(:project_group_link_expiring_in_future) { create(:project_group_link, expires_at: 10.days.from_now) } + let!(:non_expiring_project_group_link) { create(:project_group_link, expires_at: nil) } + + it 'removes expired group links' do + expect { subject.perform }.to change { ProjectGroupLink.count }.by(-1) + expect(ProjectGroupLink.find_by(id: expired_project_group_link.id)).to be_nil + end + + it 'leaves group links that expire in the future' do + subject.perform + expect(project_group_link_expiring_in_future.reload).to be_present + end + + it 'leaves group links that do not expire at all' do + subject.perform + expect(non_expiring_project_group_link.reload).to be_present + end + end +end diff --git a/spec/workers/remove_expired_members_worker_spec.rb b/spec/workers/remove_expired_members_worker_spec.rb index a6ce926b925..402aa1e714e 100644 --- a/spec/workers/remove_expired_members_worker_spec.rb +++ b/spec/workers/remove_expired_members_worker_spec.rb @@ -14,12 +14,12 @@ describe RemoveExpiredMembersWorker do expect(Member.find_by(id: expired_project_member.id)).to be_nil end - it 'leaves members who expire in the future' do + it 'leaves members that expire in the future' do worker.perform expect(project_member_expiring_in_future.reload).to be_present end - it 'leaves members who do not expire at all' do + it 'leaves members that do not expire at all' do worker.perform expect(non_expiring_project_member.reload).to be_present end @@ -35,12 +35,12 @@ describe RemoveExpiredMembersWorker do expect(Member.find_by(id: expired_group_member.id)).to be_nil end - it 'leaves members who expire in the future' do + it 'leaves members that expire in the future' do worker.perform expect(group_member_expiring_in_future.reload).to be_present end - it 'leaves members who do not expire at all' do + it 'leaves members that do not expire at all' do worker.perform expect(non_expiring_group_member.reload).to be_present end |