Merge branch '438966-update-branch-rule-by-global-id' into 'master'

Allow branch rule mutation to use branch rule global id

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/144632



Merged-by: Tetiana Chupryna <tchupryna@gitlab.com>
Approved-by: Tetiana Chupryna <tchupryna@gitlab.com>
Co-authored-by: ghinfey <ghinfey@gitlab.com>

app/graphql/mutations/branch_rules/update.rb

@@ -5,45 +5,27 @@ module BranchRules
     class Update < BaseMutation
       graphql_name 'BranchRuleUpdate'
 
-      include FindsProject
+      authorize :update_branch_rule
 
-      authorize :admin_project
-
-      argument :id, ::Types::GlobalIDType[::ProtectedBranch],
+      argument :id, ::Types::GlobalIDType[::Projects::BranchRule],
         required: true,
-        description: 'Global ID of the protected branch.'
+        description: 'Global ID of the branch rule to update.'
 
       argument :name, GraphQL::Types::String,
         required: true,
         description: 'Branch name, with wildcards, for the branch rules.'
 
-      argument :project_path, GraphQL::Types::ID,
-        required: true,
-        description: 'Full path to the project that the branch is associated with.'
-
       field :branch_rule,
         Types::Projects::BranchRuleType,
         null: true,
         description: 'Branch rule after mutation.'
 
-      def resolve(id:, project_path:, name:)
-        protected_branch = ::Gitlab::Graphql::Lazy.force(GitlabSchema.object_from_id(id,
-          expected_type: ::ProtectedBranch))
-        raise_resource_not_available_error! unless protected_branch
-
-        project = authorized_find!(project_path)
+      def resolve(id:, name:)
+        branch_rule = authorized_find!(id: id)
 
-        protected_branch = ::ProtectedBranches::UpdateService.new(project, current_user,
-          { name: name }).execute(protected_branch)
+        response = ::BranchRules::UpdateService.new(branch_rule, current_user, { name: name }).execute
 
-        if protected_branch.errors.empty?
-          {
-            branch_rule: ::Projects::BranchRule.new(project, protected_branch),
-            errors: []
-          }
-        else
-          { errors: errors_on_object(protected_branch) }
-        end
+        { branch_rule: branch_rule, errors: response.errors }
       end
     end
   end

doc/api/graphql/reference/index.md

@@ -2101,9 +2101,8 @@ Input type: `BranchRuleUpdateInput`
 | Name | Type | Description |
 | ---- | ---- | ----------- |
 | <a id="mutationbranchruleupdateclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
-| <a id="mutationbranchruleupdateid"></a>`id` | [`ProtectedBranchID!`](#protectedbranchid) | Global ID of the protected branch. |
+| <a id="mutationbranchruleupdateid"></a>`id` | [`ProjectsBranchRuleID!`](#projectsbranchruleid) | Global ID of the branch rule to update. |
 | <a id="mutationbranchruleupdatename"></a>`name` | [`String!`](#string) | Branch name, with wildcards, for the branch rules. |
-| <a id="mutationbranchruleupdateprojectpath"></a>`projectPath` | [`ID!`](#id) | Full path to the project that the branch is associated with. |
 #### Fields
@@ -33857,12 +33856,6 @@ A `ProjectsBranchRuleID` is a global ID. It is encoded as a string.
 An example `ProjectsBranchRuleID` is: `"gid://gitlab/Projects::BranchRule/1"`.
-### `ProtectedBranchID`
-
-A `ProtectedBranchID` is a global ID. It is encoded as a string.
-
-An example `ProtectedBranchID` is: `"gid://gitlab/ProtectedBranch/1"`.
-
 ### `ReleaseID`
 A `ReleaseID` is a global ID. It is encoded as a string.

ee/spec/requests/api/graphql/branch_rules/update_spec.rb

@@ -7,14 +7,14 @@
 
   let_it_be(:project) { create(:project, :public) }
   let_it_be(:user) { create(:user) }
-  let!(:branch_rule) { create(:protected_branch, project: project) }
+  let!(:protected_branch) { create(:protected_branch, project: project) }
+  let(:branch_rule) { Projects::BranchRule.new(project, protected_branch) }
   let(:current_user) { user }
   let(:mutation) { graphql_mutation(:branch_rule_update, params) }
 
   let(:params) do
     {
       id: branch_rule.to_global_id,
-      project_path: project.full_path,
       name: branch_rule.name.reverse
     }
   end

spec/requests/api/graphql/mutations/branch_rules/update_spec.rb

@@ -7,18 +7,17 @@
 
   let_it_be(:project) { create(:project, :public) }
   let_it_be(:user) { create(:user) }
-  let!(:branch_rule_1) { create(:protected_branch, project: project, name: name_1) }
-  let!(:branch_rule_2) { create(:protected_branch, project: project, name: name_2) }
+  let!(:protected_branch_1) { create(:protected_branch, project: project, name: name_1) }
+  let!(:protected_branch_2) { create(:protected_branch, project: project, name: name_2) }
+  let(:branch_rule) { Projects::BranchRule.new(project, protected_branch_1) }
   let(:name_1) { "name_1" }
   let(:name_2) { "name_2" }
   let(:new_name) { "new name" }
-  let(:id) { branch_rule_1.to_global_id }
-  let(:project_path) { project.full_path }
+  let(:global_id) { branch_rule.to_global_id }
   let(:name) { new_name }
   let(:params) do
     {
-      id: id,
-      project_path: project_path,
+      id: global_id,
       name: name
     }
   end
@@ -37,7 +36,9 @@ def mutation_response
     end
 
     it 'does not update the branch rule' do
-      expect { post_mutation }.not_to change { branch_rule_1 }
+      post_mutation
+
+      expect(protected_branch_1.reload.name).to eq(name_1)
     end
   end
 
@@ -48,10 +49,10 @@ def mutation_response
       project.add_maintainer(user)
     end
 
-    it 'updates the protected branch' do
+    it 'updates the branch rule' do
       post_mutation
 
-      expect(branch_rule_1.reload.name).to eq(new_name)
+      expect(protected_branch_1.reload.name).to eq(new_name)
     end
 
     it 'returns the updated branch rule' do
@@ -65,8 +66,7 @@ def mutation_response
     context 'when name already exists for the project' do
       let(:params) do
         {
-          id: id,
-          project_path: project_path,
+          id: global_id,
           name: name_2
         }
       end
@@ -78,18 +78,16 @@ def mutation_response
       end
     end
 
-    context 'when the protected branch cannot be found' do
-      let(:id) { "gid://gitlab/ProtectedBranch/#{non_existing_record_id}" }
-
-      it_behaves_like 'a mutation that returns top-level errors',
-        errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR]
-    end
+    context 'when branch rule cannot be found' do
+      let(:global_id) { project.to_gid.to_s }
+      let(:error_message) { %("#{global_id}" does not represent an instance of Projects::BranchRule) }
+      let(:global_id_error) { a_hash_including('message' => a_string_including(error_message)) }
 
-    context 'when the project cannot be found' do
-      let(:project_path) { 'not a project path' }
+      it 'returns an error' do
+        post_mutation
 
-      it_behaves_like 'a mutation that returns top-level errors',
-        errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR]
+        expect(graphql_errors).to include(global_id_error)
+      end
     end
   end
 end