diff options
25 files changed, 383 insertions, 76 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ac2dcf3c99..ef6bb64215b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Fix filtering of milestones with quotes in title (airatshigapov) - Refactor less readable existance checking code from CoffeeScript !6289 (jlogandavison) - Update mail_room and enable sentinel support to Reply By Email (!7101) + - Add task completion status in Issues and Merge Requests tabs: "X of Y tasks completed" (!6527, @gmesalazar) - Simpler arguments passed to named_route on toggle_award_url helper method - Fix typo in framework css class. !7086 (Daniel Voogsgerd) - New issue board list dropdown stays open after adding a new list @@ -33,14 +34,14 @@ Please view this file on the master branch, on stable branches it's out of date. - Fix Sign in page 'Forgot your password?' link overlaps on medium-large screens - Show full status link on MR & commit pipelines - Fix documents and comments on Build API `scope` - - Fix applying labels for GitHub-imported MRs - - Fix importing MR comments from GitHub - - Modify GitHub importer to be retryable - Refactor email, use setter method instead AR callbacks for email attribute (Semyon Pupkov) - Shortened merge request modal to let clipboard button not overlap ## 8.13.2 - Fix builds dropdown overlapping bug !7124 + - Fix applying labels for GitHub-imported MRs !7139 + - Fix importing MR comments from GitHub !7139 + - Modify GitHub importer to be retryable !7003 - Fix and improve `Sortable.highest_label_priority` ## 8.13.1 (2016-10-25) diff --git a/app/assets/javascripts/issue.js b/app/assets/javascripts/issue.js index e83dae2bb3c..67ace697936 100644 --- a/app/assets/javascripts/issue.js +++ b/app/assets/javascripts/issue.js @@ -95,7 +95,11 @@ return $.ajax({ type: 'PATCH', url: $('form.js-issuable-update').attr('action'), - data: patchData + data: patchData, + success: function(issue) { + document.querySelector('#task_status').innerText = issue.task_status; + document.querySelector('#task_status_short').innerText = issue.task_status_short; + } }); // TODO (rspeicher): Make the issue description inline-editable like a note so // that we can re-use its form here diff --git a/app/assets/javascripts/merge_request.js b/app/assets/javascripts/merge_request.js index a0bce6ef381..d3bd1e846c1 100644 --- a/app/assets/javascripts/merge_request.js +++ b/app/assets/javascripts/merge_request.js @@ -97,7 +97,11 @@ return $.ajax({ type: 'PATCH', url: $('form.js-issuable-update').attr('action'), - data: patchData + data: patchData, + success: function(mergeRequest) { + document.querySelector('#task_status').innerText = mergeRequest.task_status; + document.querySelector('#task_status_short').innerText = mergeRequest.task_status_short; + } }); // TODO (rspeicher): Make the merge request description inline-editable like a // note so that we can re-use its form here diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index cb649264146..3f1a1d1c511 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -112,7 +112,7 @@ class Projects::IssuesController < Projects::ApplicationController end format.json do - render json: @issue.to_json(include: { milestone: {}, assignee: { methods: :avatar_url }, labels: { methods: :text_color } }) + render json: @issue.to_json(include: { milestone: {}, assignee: { methods: :avatar_url }, labels: { methods: :text_color } }, methods: [:task_status, :task_status_short]) end end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 2ee53f7ceda..30f1cf4e5be 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -278,7 +278,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController @merge_request.target_project, @merge_request]) end format.json do - render json: @merge_request.to_json(include: { milestone: {}, assignee: { methods: :avatar_url }, labels: { methods: :text_color } }) + render json: @merge_request.to_json(include: { milestone: {}, assignee: { methods: :avatar_url }, labels: { methods: :text_color } }, methods: [:task_status, :task_status_short]) end end else diff --git a/app/finders/labels_finder.rb b/app/finders/labels_finder.rb index 44484d64567..865f093f04a 100644 --- a/app/finders/labels_finder.rb +++ b/app/finders/labels_finder.rb @@ -4,9 +4,8 @@ class LabelsFinder < UnionFinder @params = params end - def execute(authorized_only: true) - @authorized_only = authorized_only - + def execute(skip_authorization: false) + @skip_authorization = skip_authorization items = find_union(label_ids, Label) items = with_title(items) sort(items) @@ -14,7 +13,7 @@ class LabelsFinder < UnionFinder private - attr_reader :current_user, :params, :authorized_only + attr_reader :current_user, :params, :skip_authorization def label_ids label_ids = [] @@ -70,17 +69,17 @@ class LabelsFinder < UnionFinder end def find_project - if authorized_only - available_projects.find_by(id: project_id) - else + if skip_authorization Project.find_by(id: project_id) + else + available_projects.find_by(id: project_id) end end def projects return @projects if defined?(@projects) - @projects = authorized_only ? available_projects : Project.all + @projects = skip_authorization ? Project.all : available_projects @projects = @projects.in_namespace(group_id) if group_id @projects = @projects.where(id: projects_ids) if projects_ids @projects = @projects.reorder(nil) diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index 03b2db1bc91..ef6cfb235a9 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -71,6 +71,14 @@ module IssuablesHelper author_output = link_to_member(project, issuable.author, size: 24, mobile_classes: "hidden-xs", tooltip: true) author_output << link_to_member(project, issuable.author, size: 24, by_username: true, avatar: false, mobile_classes: "hidden-sm hidden-md hidden-lg") end + + if issuable.tasks? + output << " ".html_safe + output << content_tag(:span, issuable.task_status, id: "task_status", class: "hidden-xs") + output << content_tag(:span, issuable.task_status_short, id: "task_status_short", class: "hidden-sm hidden-md hidden-lg") + end + + output end def issuable_todo(issuable) diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 17c3b526c97..613444e0d70 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -12,6 +12,7 @@ module Issuable include Subscribable include StripAttribute include Awardable + include Taskable included do cache_markdown_field :title, pipeline: :single_line diff --git a/app/models/concerns/taskable.rb b/app/models/concerns/taskable.rb index a3ac577cf3e..ebc75100a54 100644 --- a/app/models/concerns/taskable.rb +++ b/app/models/concerns/taskable.rb @@ -53,10 +53,22 @@ module Taskable # Return a string that describes the current state of this Taskable's task # list items, e.g. "12 of 20 tasks completed" - def task_status + def task_status(short: false) return '' if description.blank? + prep, completed = if short + ['/', ''] + else + [' of ', ' completed'] + end + sum = tasks.summary - "#{sum.complete_count} of #{sum.item_count} #{'task'.pluralize(sum.item_count)} completed" + "#{sum.complete_count}#{prep}#{sum.item_count} #{'task'.pluralize(sum.item_count)}#{completed}" + end + + # Return a short string that describes the current state of this Taskable's + # task list items -- for small screens + def task_status_short + task_status(short: true) end end diff --git a/app/models/issue.rb b/app/models/issue.rb index e356fe06363..4f02b02c488 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -5,7 +5,6 @@ class Issue < ActiveRecord::Base include Issuable include Referable include Sortable - include Taskable include Spammable include FasterCacheKeys diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 4872f8b8649..0397c57f935 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -3,7 +3,6 @@ class MergeRequest < ActiveRecord::Base include Issuable include Referable include Sortable - include Taskable include Importable belongs_to :target_project, class_name: "Project" diff --git a/app/models/project.rb b/app/models/project.rb index fbf7012972e..ae57815c620 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -738,7 +738,7 @@ class Project < ActiveRecord::Base def create_labels Label.templates.each do |label| params = label.attributes.except('id', 'template', 'created_at', 'updated_at') - Labels::FindOrCreateService.new(owner, self, params).execute + Labels::FindOrCreateService.new(nil, self, params).execute(skip_authorization: true) end end diff --git a/app/models/project_team.rb b/app/models/project_team.rb index 79d041d2775..a6e911df9bd 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -125,14 +125,8 @@ class ProjectTeam max_member_access(user.id) == Gitlab::Access::MASTER end - def member?(user, min_member_access = nil) - member = !!find_member(user.id) - - if min_member_access - member && max_member_access(user.id) >= min_member_access - else - member - end + def member?(user, min_member_access = Gitlab::Access::GUEST) + max_member_access(user.id) >= min_member_access end def human_max_access(user_id) diff --git a/app/services/labels/find_or_create_service.rb b/app/services/labels/find_or_create_service.rb index 74291312c4e..d622f9edd33 100644 --- a/app/services/labels/find_or_create_service.rb +++ b/app/services/labels/find_or_create_service.rb @@ -2,21 +2,24 @@ module Labels class FindOrCreateService def initialize(current_user, project, params = {}) @current_user = current_user - @group = project.group @project = project @params = params.dup end - def execute + def execute(skip_authorization: false) + @skip_authorization = skip_authorization find_or_create_label end private - attr_reader :current_user, :group, :project, :params + attr_reader :current_user, :project, :params, :skip_authorization def available_labels - @available_labels ||= LabelsFinder.new(current_user, project_id: project.id).execute + @available_labels ||= LabelsFinder.new( + current_user, + project_id: project.id + ).execute(skip_authorization: skip_authorization) end def find_or_create_label diff --git a/app/services/members/approve_access_request_service.rb b/app/services/members/approve_access_request_service.rb index 416aee2ab51..c13f289f61e 100644 --- a/app/services/members/approve_access_request_service.rb +++ b/app/services/members/approve_access_request_service.rb @@ -4,17 +4,25 @@ module Members attr_accessor :source + # source - The source object that respond to `#requesters` (i.g. project or group) + # current_user - The user that performs the access request approval + # params - A hash of parameters + # :user_id - User ID used to retrieve the access requester + # :id - Member ID used to retrieve the access requester + # :access_level - Optional access level set when the request is accepted def initialize(source, current_user, params = {}) @source = source @current_user = current_user - @params = params + @params = params.slice(:user_id, :id, :access_level) end - def execute + # opts - A hash of options + # :force - Bypass permission check: current_user can be nil in that case + def execute(opts = {}) condition = params[:user_id] ? { user_id: params[:user_id] } : { id: params[:id] } access_requester = source.requesters.find_by!(condition) - raise Gitlab::Access::AccessDeniedError unless can_update_access_requester?(access_requester) + raise Gitlab::Access::AccessDeniedError unless can_update_access_requester?(access_requester, opts) access_requester.access_level = params[:access_level] if params[:access_level] access_requester.accept_request @@ -24,8 +32,11 @@ module Members private - def can_update_access_requester?(access_requester) - access_requester && can?(current_user, action_member_permission(:update, access_requester), access_requester) + def can_update_access_requester?(access_requester, opts = {}) + access_requester && ( + opts[:force] || + can?(current_user, action_member_permission(:update, access_requester), access_requester) + ) end end end diff --git a/lib/banzai/filter/label_reference_filter.rb b/lib/banzai/filter/label_reference_filter.rb index c24831e68ee..9f9a96cdc65 100644 --- a/lib/banzai/filter/label_reference_filter.rb +++ b/lib/banzai/filter/label_reference_filter.rb @@ -39,7 +39,7 @@ module Banzai end def find_labels(project) - LabelsFinder.new(nil, project_id: project.id).execute(authorized_only: false) + LabelsFinder.new(nil, project_id: project.id).execute(skip_authorization: true) end # Parameters to pass to `Label.find_by` based on the given arguments diff --git a/lib/gitlab/fogbugz_import/importer.rb b/lib/gitlab/fogbugz_import/importer.rb index 65ee85ca5a9..222bcdcbf9c 100644 --- a/lib/gitlab/fogbugz_import/importer.rb +++ b/lib/gitlab/fogbugz_import/importer.rb @@ -75,7 +75,7 @@ module Gitlab def create_label(name) params = { title: name, color: nice_label_color(name) } - ::Labels::FindOrCreateService.new(project.owner, project, params).execute + ::Labels::FindOrCreateService.new(nil, project, params).execute(skip_authorization: true) end def user_info(person_id) @@ -133,7 +133,7 @@ module Gitlab updated_at: DateTime.parse(bug['dtLastUpdated']) ) - issue_labels = ::LabelsFinder.new(project.owner, project_id: project.id, title: labels).execute + issue_labels = ::LabelsFinder.new(nil, project_id: project.id, title: labels).execute(skip_authorization: true) issue.update_attribute(:label_ids, issue_labels.pluck(:id)) import_issue_comments(issue, comments) diff --git a/lib/gitlab/github_import/label_formatter.rb b/lib/gitlab/github_import/label_formatter.rb index 002494739e9..211ccdc51bb 100644 --- a/lib/gitlab/github_import/label_formatter.rb +++ b/lib/gitlab/github_import/label_formatter.rb @@ -15,8 +15,8 @@ module Gitlab def create! params = attributes.except(:project) - service = ::Labels::FindOrCreateService.new(project.owner, project, params) - label = service.execute + service = ::Labels::FindOrCreateService.new(nil, project, params) + label = service.execute(skip_authorization: true) raise ActiveRecord::RecordInvalid.new(label) unless label.persisted? diff --git a/lib/gitlab/google_code_import/importer.rb b/lib/gitlab/google_code_import/importer.rb index 6a68e786b4f..1f4edc36928 100644 --- a/lib/gitlab/google_code_import/importer.rb +++ b/lib/gitlab/google_code_import/importer.rb @@ -101,7 +101,7 @@ module Gitlab state: raw_issue['state'] == 'closed' ? 'closed' : 'opened' ) - issue_labels = ::LabelsFinder.new(project.owner, project_id: project.id, title: labels).execute + issue_labels = ::LabelsFinder.new(nil, project_id: project.id, title: labels).execute(skip_authorization: true) issue.update_attribute(:label_ids, issue_labels.pluck(:id)) import_issue_comments(issue, comments) @@ -235,7 +235,7 @@ module Gitlab def create_label(name) params = { name: name, color: nice_label_color(name) } - ::Labels::FindOrCreateService.new(project.owner, project, params).execute + ::Labels::FindOrCreateService.new(nil, project, params).execute(skip_authorization: true) end def format_content(raw_content) diff --git a/lib/gitlab/issues_labels.rb b/lib/gitlab/issues_labels.rb index dbc759367eb..b8ca7f2f55f 100644 --- a/lib/gitlab/issues_labels.rb +++ b/lib/gitlab/issues_labels.rb @@ -19,7 +19,7 @@ module Gitlab ] labels.each do |params| - ::Labels::FindOrCreateService.new(project.owner, project, params).execute + ::Labels::FindOrCreateService.new(nil, project, params).execute(skip_authorization: true) end end end diff --git a/spec/controllers/projects/labels_controller_spec.rb b/spec/controllers/projects/labels_controller_spec.rb index 41df63d445a..8faecec0063 100644 --- a/spec/controllers/projects/labels_controller_spec.rb +++ b/spec/controllers/projects/labels_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Projects::LabelsController do let(:group) { create(:group) } - let(:project) { create(:project, namespace: group) } + let(:project) { create(:empty_project, namespace: group) } let(:user) { create(:user) } before do @@ -73,16 +73,27 @@ describe Projects::LabelsController do describe 'POST #generate' do let(:admin) { create(:admin) } - let(:project) { create(:empty_project) } before do sign_in(admin) end - it 'creates labels' do - post :generate, namespace_id: project.namespace.to_param, project_id: project.to_param + context 'personal project' do + let(:personal_project) { create(:empty_project) } - expect(response).to have_http_status(302) + it 'creates labels' do + post :generate, namespace_id: personal_project.namespace.to_param, project_id: personal_project.to_param + + expect(response).to have_http_status(302) + end + end + + context 'project belonging to a group' do + it 'creates labels' do + post :generate, namespace_id: project.namespace.to_param, project_id: project.to_param + + expect(response).to have_http_status(302) + end end end end diff --git a/spec/policies/issues_policy_spec.rb b/spec/policies/issues_policy_spec.rb new file mode 100644 index 00000000000..2b7b6cad654 --- /dev/null +++ b/spec/policies/issues_policy_spec.rb @@ -0,0 +1,193 @@ +require 'spec_helper' + +describe IssuePolicy, models: true do + let(:guest) { create(:user) } + let(:author) { create(:user) } + let(:assignee) { create(:user) } + let(:reporter) { create(:user) } + let(:group) { create(:group, :public) } + let(:reporter_from_group_link) { create(:user) } + + def permissions(user, issue) + IssuePolicy.abilities(user, issue).to_set + end + + context 'a private project' do + let(:non_member) { create(:user) } + let(:project) { create(:empty_project, :private) } + let(:issue) { create(:issue, project: project, assignee: assignee, author: author) } + let(:issue_no_assignee) { create(:issue, project: project) } + + before do + project.team << [guest, :guest] + project.team << [author, :guest] + project.team << [assignee, :guest] + project.team << [reporter, :reporter] + + group.add_reporter(reporter_from_group_link) + + create(:project_group_link, group: group, project: project) + end + + it 'does not allow non-members to read issues' do + expect(permissions(non_member, issue)).not_to include(:read_issue, :update_issue, :admin_issue) + expect(permissions(non_member, issue_no_assignee)).not_to include(:read_issue, :update_issue, :admin_issue) + end + + it 'allows guests to read issues' do + expect(permissions(guest, issue)).to include(:read_issue) + expect(permissions(guest, issue)).not_to include(:update_issue, :admin_issue) + + expect(permissions(guest, issue_no_assignee)).to include(:read_issue) + expect(permissions(guest, issue_no_assignee)).not_to include(:update_issue, :admin_issue) + end + + it 'allows reporters to read, update, and admin issues' do + expect(permissions(reporter, issue)).to include(:read_issue, :update_issue, :admin_issue) + expect(permissions(reporter, issue_no_assignee)).to include(:read_issue, :update_issue, :admin_issue) + end + + it 'allows reporters from group links to read, update, and admin issues' do + expect(permissions(reporter_from_group_link, issue)).to include(:read_issue, :update_issue, :admin_issue) + expect(permissions(reporter_from_group_link, issue_no_assignee)).to include(:read_issue, :update_issue, :admin_issue) + end + + it 'allows issue authors to read and update their issues' do + expect(permissions(author, issue)).to include(:read_issue, :update_issue) + expect(permissions(author, issue)).not_to include(:admin_issue) + + expect(permissions(author, issue_no_assignee)).to include(:read_issue) + expect(permissions(author, issue_no_assignee)).not_to include(:update_issue, :admin_issue) + end + + it 'allows issue assignees to read and update their issues' do + expect(permissions(assignee, issue)).to include(:read_issue, :update_issue) + expect(permissions(assignee, issue)).not_to include(:admin_issue) + + expect(permissions(assignee, issue_no_assignee)).to include(:read_issue) + expect(permissions(assignee, issue_no_assignee)).not_to include(:update_issue, :admin_issue) + end + + context 'with confidential issues' do + let(:confidential_issue) { create(:issue, :confidential, project: project, assignee: assignee, author: author) } + let(:confidential_issue_no_assignee) { create(:issue, :confidential, project: project) } + + it 'does not allow non-members to read confidential issues' do + expect(permissions(non_member, confidential_issue)).not_to include(:read_issue, :update_issue, :admin_issue) + expect(permissions(non_member, confidential_issue_no_assignee)).not_to include(:read_issue, :update_issue, :admin_issue) + end + + it 'does not allow guests to read confidential issues' do + expect(permissions(guest, confidential_issue)).not_to include(:read_issue, :update_issue, :admin_issue) + expect(permissions(guest, confidential_issue_no_assignee)).not_to include(:read_issue, :update_issue, :admin_issue) + end + + it 'allows reporters to read, update, and admin confidential issues' do + expect(permissions(reporter, confidential_issue)).to include(:read_issue, :update_issue, :admin_issue) + expect(permissions(reporter, confidential_issue_no_assignee)).to include(:read_issue, :update_issue, :admin_issue) + end + + it 'allows reporters from group links to read, update, and admin confidential issues' do + expect(permissions(reporter_from_group_link, confidential_issue)).to include(:read_issue, :update_issue, :admin_issue) + expect(permissions(reporter_from_group_link, confidential_issue_no_assignee)).to include(:read_issue, :update_issue, :admin_issue) + end + + it 'allows issue authors to read and update their confidential issues' do + expect(permissions(author, confidential_issue)).to include(:read_issue, :update_issue) + expect(permissions(author, confidential_issue)).not_to include(:admin_issue) + + expect(permissions(author, confidential_issue_no_assignee)).not_to include(:read_issue, :update_issue, :admin_issue) + end + + it 'allows issue assignees to read and update their confidential issues' do + expect(permissions(assignee, confidential_issue)).to include(:read_issue, :update_issue) + expect(permissions(assignee, confidential_issue)).not_to include(:admin_issue) + + expect(permissions(assignee, confidential_issue_no_assignee)).not_to include(:read_issue, :update_issue, :admin_issue) + end + end + end + + context 'a public project' do + let(:project) { create(:empty_project, :public) } + let(:issue) { create(:issue, project: project, assignee: assignee, author: author) } + let(:issue_no_assignee) { create(:issue, project: project) } + + before do + project.team << [guest, :guest] + project.team << [reporter, :reporter] + + group.add_reporter(reporter_from_group_link) + + create(:project_group_link, group: group, project: project) + end + + it 'allows guests to read issues' do + expect(permissions(guest, issue)).to include(:read_issue) + expect(permissions(guest, issue)).not_to include(:update_issue, :admin_issue) + + expect(permissions(guest, issue_no_assignee)).to include(:read_issue) + expect(permissions(guest, issue_no_assignee)).not_to include(:update_issue, :admin_issue) + end + + it 'allows reporters to read, update, and admin issues' do + expect(permissions(reporter, issue)).to include(:read_issue, :update_issue, :admin_issue) + expect(permissions(reporter, issue_no_assignee)).to include(:read_issue, :update_issue, :admin_issue) + end + + it 'allows reporters from group links to read, update, and admin issues' do + expect(permissions(reporter_from_group_link, issue)).to include(:read_issue, :update_issue, :admin_issue) + expect(permissions(reporter_from_group_link, issue_no_assignee)).to include(:read_issue, :update_issue, :admin_issue) + end + + it 'allows issue authors to read and update their issues' do + expect(permissions(author, issue)).to include(:read_issue, :update_issue) + expect(permissions(author, issue)).not_to include(:admin_issue) + + expect(permissions(author, issue_no_assignee)).to include(:read_issue) + expect(permissions(author, issue_no_assignee)).not_to include(:update_issue, :admin_issue) + end + + it 'allows issue assignees to read and update their issues' do + expect(permissions(assignee, issue)).to include(:read_issue, :update_issue) + expect(permissions(assignee, issue)).not_to include(:admin_issue) + + expect(permissions(assignee, issue_no_assignee)).to include(:read_issue) + expect(permissions(assignee, issue_no_assignee)).not_to include(:update_issue, :admin_issue) + end + + context 'with confidential issues' do + let(:confidential_issue) { create(:issue, :confidential, project: project, assignee: assignee, author: author) } + let(:confidential_issue_no_assignee) { create(:issue, :confidential, project: project) } + + it 'does not allow guests to read confidential issues' do + expect(permissions(guest, confidential_issue)).not_to include(:read_issue, :update_issue, :admin_issue) + expect(permissions(guest, confidential_issue_no_assignee)).not_to include(:read_issue, :update_issue, :admin_issue) + end + + it 'allows reporters to read, update, and admin confidential issues' do + expect(permissions(reporter, confidential_issue)).to include(:read_issue, :update_issue, :admin_issue) + expect(permissions(reporter, confidential_issue_no_assignee)).to include(:read_issue, :update_issue, :admin_issue) + end + + it 'allows reporter from group links to read, update, and admin confidential issues' do + expect(permissions(reporter_from_group_link, confidential_issue)).to include(:read_issue, :update_issue, :admin_issue) + expect(permissions(reporter_from_group_link, confidential_issue_no_assignee)).to include(:read_issue, :update_issue, :admin_issue) + end + + it 'allows issue authors to read and update their confidential issues' do + expect(permissions(author, confidential_issue)).to include(:read_issue, :update_issue) + expect(permissions(author, confidential_issue)).not_to include(:admin_issue) + + expect(permissions(author, confidential_issue_no_assignee)).not_to include(:read_issue, :update_issue, :admin_issue) + end + + it 'allows issue assignees to read and update their confidential issues' do + expect(permissions(assignee, confidential_issue)).to include(:read_issue, :update_issue) + expect(permissions(assignee, confidential_issue)).not_to include(:admin_issue) + + expect(permissions(assignee, confidential_issue_no_assignee)).not_to include(:read_issue, :update_issue, :admin_issue) + end + end + end +end diff --git a/spec/services/labels/find_or_create_service_spec.rb b/spec/services/labels/find_or_create_service_spec.rb index cbfc63de811..7a9b34f9f96 100644 --- a/spec/services/labels/find_or_create_service_spec.rb +++ b/spec/services/labels/find_or_create_service_spec.rb @@ -2,7 +2,6 @@ require 'spec_helper' describe Labels::FindOrCreateService, services: true do describe '#execute' do - let(:user) { create(:user) } let(:group) { create(:group) } let(:project) { create(:project, namespace: group) } @@ -14,37 +13,49 @@ describe Labels::FindOrCreateService, services: true do } end - subject(:service) { described_class.new(user, project, params) } - - before do - project.team << [user, :developer] - end + context 'when acting on behalf of a specific user' do + let(:user) { create(:user) } + subject(:service) { described_class.new(user, project, params) } + before do + project.team << [user, :developer] + end - context 'when label does not exist at group level' do - it 'creates a new label at project level' do - expect { service.execute }.to change(project.labels, :count).by(1) + context 'when label does not exist at group level' do + it 'creates a new label at project level' do + expect { service.execute }.to change(project.labels, :count).by(1) + end end - end - context 'when label exists at group level' do - it 'returns the group label' do - group_label = create(:group_label, group: group, title: 'Security') + context 'when label exists at group level' do + it 'returns the group label' do + group_label = create(:group_label, group: group, title: 'Security') - expect(service.execute).to eq group_label + expect(service.execute).to eq group_label + end end - end - context 'when label does not exist at group level' do - it 'creates a new label at project leve' do - expect { service.execute }.to change(project.labels, :count).by(1) + context 'when label does not exist at group level' do + it 'creates a new label at project leve' do + expect { service.execute }.to change(project.labels, :count).by(1) + end + end + + context 'when label exists at project level' do + it 'returns the project label' do + project_label = create(:label, project: project, title: 'Security') + + expect(service.execute).to eq project_label + end end end - context 'when label exists at project level' do + context 'when authorization is not required' do + subject(:service) { described_class.new(nil, project, params) } + it 'returns the project label' do project_label = create(:label, project: project, title: 'Security') - expect(service.execute).to eq project_label + expect(service.execute(skip_authorization: true)).to eq project_label end end end diff --git a/spec/services/members/approve_access_request_service_spec.rb b/spec/services/members/approve_access_request_service_spec.rb index 03e296259f9..7b090343a3e 100644 --- a/spec/services/members/approve_access_request_service_spec.rb +++ b/spec/services/members/approve_access_request_service_spec.rb @@ -5,36 +5,37 @@ describe Members::ApproveAccessRequestService, services: true do let(:access_requester) { create(:user) } let(:project) { create(:project, :public) } let(:group) { create(:group, :public) } + let(:opts) { {} } shared_examples 'a service raising ActiveRecord::RecordNotFound' do it 'raises ActiveRecord::RecordNotFound' do - expect { described_class.new(source, user, params).execute }.to raise_error(ActiveRecord::RecordNotFound) + expect { described_class.new(source, user, params).execute(opts) }.to raise_error(ActiveRecord::RecordNotFound) end end shared_examples 'a service raising Gitlab::Access::AccessDeniedError' do it 'raises Gitlab::Access::AccessDeniedError' do - expect { described_class.new(source, user, params).execute }.to raise_error(Gitlab::Access::AccessDeniedError) + expect { described_class.new(source, user, params).execute(opts) }.to raise_error(Gitlab::Access::AccessDeniedError) end end shared_examples 'a service approving an access request' do it 'succeeds' do - expect { described_class.new(source, user, params).execute }.to change { source.requesters.count }.by(-1) + expect { described_class.new(source, user, params).execute(opts) }.to change { source.requesters.count }.by(-1) end it 'returns a <Source>Member' do - member = described_class.new(source, user, params).execute + member = described_class.new(source, user, params).execute(opts) expect(member).to be_a "#{source.class}Member".constantize expect(member.requested_at).to be_nil end context 'with a custom access level' do - let(:params) { { user_id: access_requester.id, access_level: Gitlab::Access::MASTER } } + let(:params2) { params.merge(user_id: access_requester.id, access_level: Gitlab::Access::MASTER) } it 'returns a ProjectMember with the custom access level' do - member = described_class.new(source, user, params).execute + member = described_class.new(source, user, params2).execute(opts) expect(member.access_level).to eq Gitlab::Access::MASTER end @@ -60,6 +61,56 @@ describe Members::ApproveAccessRequestService, services: true do end let(:params) { { user_id: access_requester.id } } + context 'when current user is nil' do + let(:user) { nil } + + context 'and :force option is not given' do + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let(:source) { project } + end + + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let(:source) { group } + end + end + + context 'and :force option is false' do + let(:opts) { { force: false } } + + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let(:source) { project } + end + + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let(:source) { group } + end + end + + context 'and :force option is true' do + let(:opts) { { force: true } } + + it_behaves_like 'a service approving an access request' do + let(:source) { project } + end + + it_behaves_like 'a service approving an access request' do + let(:source) { group } + end + end + + context 'and :force param is true' do + let(:params) { { user_id: access_requester.id, force: true } } + + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let(:source) { project } + end + + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let(:source) { group } + end + end + end + context 'when current user cannot approve access request to the project' do it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do let(:source) { project } diff --git a/spec/support/taskable_shared_examples.rb b/spec/support/taskable_shared_examples.rb index 201614e45a4..ad1c783df4d 100644 --- a/spec/support/taskable_shared_examples.rb +++ b/spec/support/taskable_shared_examples.rb @@ -17,6 +17,8 @@ shared_examples 'a Taskable' do it 'returns the correct task status' do expect(subject.task_status).to match('2 of') expect(subject.task_status).to match('5 tasks completed') + expect(subject.task_status_short).to match('2/') + expect(subject.task_status_short).to match('5 tasks') end describe '#tasks?' do @@ -41,6 +43,8 @@ shared_examples 'a Taskable' do it 'returns the correct task status' do expect(subject.task_status).to match('0 of') expect(subject.task_status).to match('1 task completed') + expect(subject.task_status_short).to match('0/') + expect(subject.task_status_short).to match('1 task') end end @@ -54,6 +58,8 @@ shared_examples 'a Taskable' do it 'returns the correct task status' do expect(subject.task_status).to match('1 of') expect(subject.task_status).to match('1 task completed') + expect(subject.task_status_short).to match('1/') + expect(subject.task_status_short).to match('1 task') end end end |