Merge branch '346042-add-lock-version-to-work-item-type' into 'master'

Improve WorkItemPolicy

See merge request gitlab-org/gitlab!80235

app/graphql/resolvers/work_item_resolver.rb

@@ -4,7 +4,7 @@ module Resolvers
   class WorkItemResolver < BaseResolver
     include Gitlab::Graphql::Authorize::AuthorizeResource
 
-    authorize :read_issue
+    authorize :read_work_item
 
     type Types::WorkItemType, null: true
 

app/graphql/types/work_item_type.rb

@@ -4,7 +4,7 @@ module Types
   class WorkItemType < BaseObject
     graphql_name 'WorkItem'
 
-    authorize :read_issue
+    authorize :read_work_item
 
     field :description, GraphQL::Types::String, null: true,
           description: 'Description of the work item.'

app/policies/project_policy.rb

@@ -264,8 +264,6 @@ class ProjectPolicy < BasePolicy
     enable :create_work_item
   end
 
-  rule { can?(:update_issue) }.enable :update_work_item
-
   # These abilities are not allowed to admins that are not members of the project,
   # that's why they are defined separately.
   rule { guest & can?(:download_code) }.enable :build_download_code

app/policies/work_item_policy.rb

 # frozen_string_literal: true
 
-class WorkItemPolicy < BasePolicy
-  delegate { @subject.project }
+class WorkItemPolicy < IssuePolicy
+  rule { can?(:owner_access) | is_author }.enable :delete_work_item
 
-  desc 'User is author of the work item'
-  condition(:author) do
-    @user && @user == @subject.author
-  end
+  rule { can?(:update_issue) }.enable :update_work_item
 
-  rule { can?(:owner_access) | author }.enable :delete_work_item
+  rule { can?(:read_issue) }.enable :read_work_item
 end

spec/graphql/types/work_item_type_spec.rb

@@ -5,7 +5,7 @@ require 'spec_helper'
 RSpec.describe GitlabSchema.types['WorkItem'] do
   specify { expect(described_class.graphql_name).to eq('WorkItem') }
 
-  specify { expect(described_class).to require_graphql_authorizations(:read_issue) }
+  specify { expect(described_class).to require_graphql_authorizations(:read_work_item) }
 
   it 'has specific fields' do
     fields = %i[description description_html id iid lock_version state title title_html work_item_type]

spec/policies/work_item_policy_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe WorkItemPolicy do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:public_project) { create(:project, :public) }
+  let_it_be(:guest) { create(:user).tap { |user| project.add_guest(user) } }
+  let_it_be(:guest_author) { create(:user).tap { |user| project.add_guest(user) } }
+  let_it_be(:reporter) { create(:user).tap { |user| project.add_reporter(user) } }
+  let_it_be(:non_member_user) { create(:user) }
+  let_it_be(:work_item) { create(:work_item, project: project) }
+  let_it_be(:authored_work_item) { create(:work_item, project: project, author: guest_author) }
+  let_it_be(:public_work_item) { create(:work_item, project: public_project) }
+
+  let(:work_item_subject) { work_item }
+
+  subject { described_class.new(current_user, work_item_subject) }
+
+  before_all do
+    public_project.add_developer(guest_author)
+  end
+
+  describe 'read_work_item' do
+    context 'when project is public' do
+      let(:work_item_subject) { public_work_item }
+
+      context 'when user is not a member of the project' do
+        let(:current_user) { non_member_user }
+
+        it { is_expected.to be_allowed(:read_work_item) }
+      end
+
+      context 'when user is a member of the project' do
+        let(:current_user) { guest_author }
+
+        it { is_expected.to be_allowed(:read_work_item) }
+      end
+    end
+
+    context 'when project is private' do
+      let(:work_item_subject) { work_item }
+
+      context 'when user is not a member of the project' do
+        let(:current_user) { non_member_user }
+
+        it { is_expected.to be_disallowed(:read_work_item) }
+      end
+
+      context 'when user is a member of the project' do
+        let(:current_user) { guest_author }
+
+        it { is_expected.to be_allowed(:read_work_item) }
+      end
+    end
+  end
+
+  describe 'update_work_item' do
+    context 'when user is reporter' do
+      let(:current_user) { reporter }
+
+      it { is_expected.to be_allowed(:update_work_item) }
+    end
+
+    context 'when user is guest' do
+      let(:current_user) { guest }
+
+      it { is_expected.to be_disallowed(:update_work_item) }
+
+      context 'when guest authored the work item' do
+        let(:work_item_subject) { authored_work_item }
+        let(:current_user) { guest_author }
+
+        it { is_expected.to be_allowed(:update_work_item) }
+      end
+    end
+  end
+
+  describe 'delete_work_item' do
+    context 'when user is a member of the project' do
+      let(:work_item_subject) { work_item }
+      let(:current_user) { reporter }
+
+      it { is_expected.to be_disallowed(:delete_work_item) }
+
+      context 'when guest authored the work item' do
+        let(:work_item_subject) { authored_work_item }
+        let(:current_user) { guest_author }
+
+        it { is_expected.to be_allowed(:delete_work_item) }
+      end
+    end
+  end
+end