From 13fc0efa5725d94cda527dc487d8dfdb7e90ed21 Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Wed, 4 Sep 2019 16:33:02 +0000 Subject: Let project reporters create issue from group boards The current state of group issue boards does not show the "Add issues" button on the UI for users that are reporters of group child projects. --- .../boards/components/board_new_issue.vue | 2 +- .../boards/components/project_select.vue | 12 +++++ .../pages/projects/shared/permissions/constants.js | 2 +- app/helpers/boards_helper.rb | 2 +- app/policies/board_policy.rb | 18 ++++++++ app/policies/concerns/find_group_projects.rb | 13 ++++++ app/policies/group_policy.rb | 4 +- changelogs/unreleased/issue_54042.yml | 5 ++ doc/api/groups.md | 3 +- lib/api/groups.rb | 2 + spec/features/boards/new_issue_spec.rb | 40 ++++++++++++++++ spec/helpers/boards_helper_spec.rb | 2 +- spec/policies/board_policy_spec.rb | 53 ++++++++++++++++++++++ spec/requests/api/groups_spec.rb | 16 +++++++ 14 files changed, 168 insertions(+), 6 deletions(-) create mode 100644 app/policies/concerns/find_group_projects.rb create mode 100644 changelogs/unreleased/issue_54042.yml diff --git a/app/assets/javascripts/boards/components/board_new_issue.vue b/app/assets/javascripts/boards/components/board_new_issue.vue index 4180023b7db..f9284266b72 100644 --- a/app/assets/javascripts/boards/components/board_new_issue.vue +++ b/app/assets/javascripts/boards/components/board_new_issue.vue @@ -114,7 +114,7 @@ export default { name="issue_title" autocomplete="off" /> - +
{ this.loading = true; + const additionalAttrs = {}; + + if (this.list.type && this.list.type !== 'backlog') { + additionalAttrs.min_access_level = featureAccessLevel.EVERYONE; + } + return Api.groupProjects( this.groupId, term, @@ -56,6 +67,7 @@ export default { with_issues_enabled: true, with_shared: false, include_subgroups: true, + ...additionalAttrs, }, projects => { this.loading = false; diff --git a/app/assets/javascripts/pages/projects/shared/permissions/constants.js b/app/assets/javascripts/pages/projects/shared/permissions/constants.js index 73269c6f3ba..6771391254e 100644 --- a/app/assets/javascripts/pages/projects/shared/permissions/constants.js +++ b/app/assets/javascripts/pages/projects/shared/permissions/constants.js @@ -16,7 +16,7 @@ export const visibilityLevelDescriptions = { ), }; -const featureAccessLevel = { +export const featureAccessLevel = { NOT_ENABLED: 0, PROJECT_MEMBERS: 10, EVERYONE: 20, diff --git a/app/helpers/boards_helper.rb b/app/helpers/boards_helper.rb index bbe05f40999..8ef3ed9e8a5 100644 --- a/app/helpers/boards_helper.rb +++ b/app/helpers/boards_helper.rb @@ -10,7 +10,7 @@ module BoardsHelper boards_endpoint: @boards_endpoint, lists_endpoint: board_lists_path(board), board_id: board.id, - disabled: "#{!can?(current_user, :admin_list, current_board_parent)}", + disabled: (!can?(current_user, :create_non_backlog_issues, board)).to_s, issue_link_base: build_issue_link_base, root_path: root_path, bulk_update_path: @bulk_issues_path, diff --git a/app/policies/board_policy.rb b/app/policies/board_policy.rb index 4bf1e7bd3e1..b8435dad3f1 100644 --- a/app/policies/board_policy.rb +++ b/app/policies/board_policy.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class BoardPolicy < BasePolicy + include FindGroupProjects + delegate { @subject.parent } condition(:is_group_board) { @subject.group_board? } @@ -13,4 +15,20 @@ class BoardPolicy < BasePolicy enable :read_milestone enable :read_issue end + + condition(:reporter_of_group_projects) do + next unless @user + + group_projects_for(user: @user, group: @subject.parent) + .visible_to_user_and_access_level(@user, ::Gitlab::Access::REPORTER) + .exists? + end + + rule { is_group_board & reporter_of_group_projects }.policy do + enable :create_non_backlog_issues + end + + rule { is_project_board & can?(:admin_issue) }.policy do + enable :create_non_backlog_issues + end end diff --git a/app/policies/concerns/find_group_projects.rb b/app/policies/concerns/find_group_projects.rb new file mode 100644 index 00000000000..e2cb90079c7 --- /dev/null +++ b/app/policies/concerns/find_group_projects.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module FindGroupProjects + extend ActiveSupport::Concern + + def group_projects_for(user:, group:) + GroupProjectsFinder.new( + group: group, + current_user: user, + options: { include_subgroups: true, only_owned: true } + ).execute + end +end diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index 5d2b74b17a2..c726c7c24a7 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class GroupPolicy < BasePolicy + include FindGroupProjects + desc "Group is public" with_options scope: :subject, score: 0 condition(:public_group) { @subject.public? } @@ -22,7 +24,7 @@ class GroupPolicy < BasePolicy condition(:can_change_parent_share_with_group_lock) { can?(:change_share_with_group_lock, @subject.parent) } condition(:has_projects) do - GroupProjectsFinder.new(group: @subject, current_user: @user, options: { include_subgroups: true, only_owned: true }).execute.any? + group_projects_for(user: @user, group: @subject).any? end with_options scope: :subject, score: 0 diff --git a/changelogs/unreleased/issue_54042.yml b/changelogs/unreleased/issue_54042.yml new file mode 100644 index 00000000000..465c7426e93 --- /dev/null +++ b/changelogs/unreleased/issue_54042.yml @@ -0,0 +1,5 @@ +--- +title: Let project reporters create issue from group boards +merge_request: 29866 +author: +type: fixed diff --git a/doc/api/groups.md b/doc/api/groups.md index d7f5b1b463b..8b13462b887 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -156,7 +156,8 @@ Parameters: | `with_issues_enabled` | boolean | no | Limit by projects with issues feature enabled. Default is `false` | | `with_merge_requests_enabled` | boolean | no | Limit by projects with merge requests feature enabled. Default is `false` | | `with_shared` | boolean | no | Include projects shared to this group. Default is `true` | -| `include_subgroups` | boolean | no | Include projects in subgroups of this group. Default is `false` | +| `include_subgroups` | boolean | no | Include projects in subgroups of this group. Default is `false` | +| `min_access_level` | integer | no | Limit to projects where current user has at least this [access level](members.md) | | `with_custom_attributes` | boolean | no | Include [custom attributes](custom_attributes.md) in response (admins only) | | `with_security_reports` | boolean | no | **(ULTIMATE)** Return only projects that have security reports artifacts present in any of their builds. This means "projects with security reports enabled". Default is `false` | diff --git a/lib/api/groups.rb b/lib/api/groups.rb index 0bcd09d3977..0b086f2e36d 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -75,6 +75,7 @@ module API ).execute projects = projects.with_issues_available_for_user(current_user) if params[:with_issues_enabled] projects = projects.with_merge_requests_enabled if params[:with_merge_requests_enabled] + projects = projects.visible_to_user_and_access_level(current_user, params[:min_access_level]) if params[:min_access_level] projects = reorder_projects(projects) paginate(projects) end @@ -213,6 +214,7 @@ module API optional :with_merge_requests_enabled, type: Boolean, default: false, desc: 'Limit by enabled merge requests feature' optional :with_shared, type: Boolean, default: true, desc: 'Include projects shared to this group' optional :include_subgroups, type: Boolean, default: false, desc: 'Includes projects in subgroups of this group' + optional :min_access_level, type: Integer, values: Gitlab::Access.all_values, desc: 'Limit by minimum access level of authenticated user on projects' use :pagination use :with_custom_attributes diff --git a/spec/features/boards/new_issue_spec.rb b/spec/features/boards/new_issue_spec.rb index 36743650270..730887370dd 100644 --- a/spec/features/boards/new_issue_spec.rb +++ b/spec/features/boards/new_issue_spec.rb @@ -127,4 +127,44 @@ describe 'Issue Boards new issue', :js do end end end + + context 'group boards' do + set(:group) { create(:group, :public) } + set(:project) { create(:project, namespace: group) } + set(:group_board) { create(:board, group: group) } + set(:list) { create(:list, board: group_board, position: 0) } + + context 'for unauthorized users' do + before do + sign_in(user) + visit group_board_path(group, group_board) + wait_for_requests + end + + it 'displays new issue button in open list' do + expect(first('.board')).to have_selector('.issue-count-badge-add-button', count: 1) + end + + it 'does not display new issue button in label list' do + page.within('.board.is-draggable') do + expect(page).not_to have_selector('.issue-count-badge-add-button') + end + end + end + + context 'for authorized users' do + it 'display new issue button in label list' do + project = create(:project, namespace: group) + project.add_reporter(user) + + sign_in(user) + visit group_board_path(group, group_board) + wait_for_requests + + page.within('.board.is-draggable') do + expect(page).to have_selector('.issue-count-badge-add-button') + end + end + end + end end diff --git a/spec/helpers/boards_helper_spec.rb b/spec/helpers/boards_helper_spec.rb index f014537eb54..ad088398ce9 100644 --- a/spec/helpers/boards_helper_spec.rb +++ b/spec/helpers/boards_helper_spec.rb @@ -40,7 +40,7 @@ describe BoardsHelper do assign(:project, project) allow(helper).to receive(:current_user) { user } - allow(helper).to receive(:can?).with(user, :admin_list, project).and_return(true) + allow(helper).to receive(:can?).with(user, :create_non_backlog_issues, board).and_return(true) end it 'returns a board_lists_path as lists_endpoint' do diff --git a/spec/policies/board_policy_spec.rb b/spec/policies/board_policy_spec.rb index 52c23951e37..35eac8a02c4 100644 --- a/spec/policies/board_policy_spec.rb +++ b/spec/policies/board_policy_spec.rb @@ -56,4 +56,57 @@ describe BoardPolicy do end end end + + context 'create_non_backlog_issues' do + context 'for project boards' do + let!(:current_user) { create(:user) } + + subject { described_class.new(current_user, project_board) } + + context 'when user can admin project issues' do + it 'allows to add non backlog issues from issue board' do + project.add_reporter(current_user) + + expect_allowed(:create_non_backlog_issues) + end + end + + context 'when user cannot admin project issues' do + it 'does not allow to add non backlog issues from issue board' do + project.add_guest(current_user) + + expect_disallowed(:create_non_backlog_issues) + end + end + end + + context 'for group boards' do + let!(:current_user) { create(:user) } + let!(:project_1) { create(:project, namespace: group) } + let!(:project_2) { create(:project, namespace: group) } + let!(:group_board) { create(:board, group: group) } + + subject { described_class.new(current_user, group_board) } + + before do + project_1.add_guest(current_user) + end + + context 'when user is at least reporter in one of the child projects' do + it 'allows to add non backlog issues from issue board' do + project_2.add_reporter(current_user) + + expect_allowed(:create_non_backlog_issues) + end + end + + context 'when user is not a reporter from any child projects' do + it 'does not allow to add non backlog issues from issue board' do + project_2.add_guest(current_user) + + expect_disallowed(:create_non_backlog_issues) + end + end + end + end end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 50f36141aed..0893dcb39b6 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -483,6 +483,22 @@ describe API::Groups do describe "GET /groups/:id/projects" do context "when authenticated as user" do + context 'with min access level' do + it 'returns projects with min access level or higher' do + group_guest = create(:user) + group1.add_guest(group_guest) + project4 = create(:project, group: group1) + project1.add_guest(group_guest) + project3.add_reporter(group_guest) + project4.add_developer(group_guest) + + get api("/groups/#{group1.id}/projects", group_guest), params: { min_access_level: Gitlab::Access::REPORTER } + + project_ids = json_response.map { |proj| proj['id'] } + expect(project_ids).to match_array([project3.id, project4.id]) + end + end + it "returns the group's projects" do get api("/groups/#{group1.id}/projects", user1) -- cgit v1.2.1