Update ExternalPullRequest on :synchronize action

In order to keep the ExternalPullRequest record in sync
with GitHub we need to update the info when any metadata
we track changes. When the source_sha changes because of
a branch push, GitHub emits the :synchronize action on
the pull_request webhook.

ee/app/services/process_github_pull_request_event_service.rb

@@ -4,10 +4,10 @@ class ProcessGithubPullRequestEventService < ::BaseService
   # All possible statuses available here:
   # https://developer.github.com/v3/activity/events/types/#pullrequestevent
   GITHUB_ACTIONS_TO_STATUS = {
-    'opened'   => :open,
-    'reopened' => :open,
-    'edited'   => :open,
-    'closed'   => :closed
+    'opened'      => :open,
+    'reopened'    => :open,
+    'synchronize' => :open,
+    'closed'      => :closed
   }.freeze
 
   def execute(webhook_params)

ee/changelogs/unreleased/update-external-pull-request-on-synchronize-event.yml

+---
+title: Update ExternalPullRequest on :synchronize action to ensure source_sha is updated locally
+merge_request: 16318
+author:
+type: fixed

ee/spec/services/process_github_pull_request_event_service_spec.rb

@@ -42,101 +42,116 @@ describe ProcessGithubPullRequestEventService do
         allow(project).to receive(:mirror?).and_return(true)
       end
 
-      context 'when mirror update occurs before the pull request webhook' do
-        let(:branch) { project.repository.branches.first }
-        let(:source_branch) { branch.name }
-        let(:source_sha) { branch.target }
-
-        let(:create_pipeline_service) { instance_double(Ci::CreatePipelineService) }
-
-        it 'creates a pipeline and saves pull request info' do
-          pipeline_params = {
-            ref: Gitlab::Git::BRANCH_REF_PREFIX + branch.name,
-            source_sha: branch.target,
-            target_sha: 'a09386439ca39abe575675ffd4b89ae824fec22f'
-          }
-          expect(Ci::CreatePipelineService).to receive(:new)
-            .with(project, user, pipeline_params)
-            .and_return(create_pipeline_service)
-          expect(create_pipeline_service).to receive(:execute)
-            .with(:external_pull_request_event, any_args)
-
-          expect { subject.execute(params) }.to change { ExternalPullRequest.count }.by(1)
+      context 'when the pull request record does not exist' do
+        context 'when the pull request webhook occurs after mirror update' do
+          let(:branch) { project.repository.branches.first }
+          let(:source_branch) { branch.name }
+          let(:source_sha) { branch.target }
+
+          let(:create_pipeline_service) { instance_double(Ci::CreatePipelineService) }
+
+          it 'creates a pipeline and the external pull request' do
+            pipeline_params = {
+              ref: Gitlab::Git::BRANCH_REF_PREFIX + branch.name,
+              source_sha: branch.target,
+              target_sha: 'a09386439ca39abe575675ffd4b89ae824fec22f'
+            }
+            expect(Ci::CreatePipelineService).to receive(:new)
+              .with(project, user, pipeline_params)
+              .and_return(create_pipeline_service)
+            expect(create_pipeline_service).to receive(:execute)
+              .with(:external_pull_request_event, any_args)
+
+            expect { subject.execute(params) }.to change { ExternalPullRequest.count }.by(1)
+          end
         end
-      end
 
-      context 'when the pull request webhook occurs before mirror update' do
-        let(:source_branch) { 'a-new-branch' }
-        let(:source_sha) { 'non-existent-sha' }
+        context 'when the pull request webhook occurs before mirror update' do
+          let(:source_branch) { 'a-new-branch' }
+          let(:source_sha) { 'non-existent-sha' }
 
-        it 'only saves pull request info' do
-          expect(Ci::CreatePipelineService).not_to receive(:new)
+          it 'only saves pull request info' do
+            expect(Ci::CreatePipelineService).not_to receive(:new)
 
-          expect { subject.execute(params) }.to change { ExternalPullRequest.count }.by(1)
+            expect { subject.execute(params) }.to change { ExternalPullRequest.count }.by(1)
 
-          pull_request = ExternalPullRequest.last
+            pull_request = ExternalPullRequest.last
 
-          expect(pull_request).to be_persisted
-          expect(pull_request.project).to eq(project)
-          expect(pull_request.source_branch).to eq('a-new-branch')
-          expect(pull_request.source_repository).to eq('the-repo')
-          expect(pull_request.target_branch).to eq('the-target-branch')
-          expect(pull_request.target_repository).to eq('the-repo')
-          expect(pull_request.status).to eq('open')
+            expect(pull_request).to be_persisted
+            expect(pull_request.project).to eq(project)
+            expect(pull_request.source_branch).to eq('a-new-branch')
+            expect(pull_request.source_repository).to eq('the-repo')
+            expect(pull_request.target_branch).to eq('the-target-branch')
+            expect(pull_request.target_repository).to eq('the-repo')
+            expect(pull_request.status).to eq('open')
+          end
         end
       end
 
-      context 'when pull request webhook action is "closed"' do
-        let(:source_branch) { 'a-new-branch' }
-        let(:source_sha) { 'non-existent-sha' }
-        let(:action) { 'closed' }
+      context 'when the pull request record already exists' do
+        let(:source_branch) { 'feature' }
+        let(:source_sha) { '3d8151901da736dc432dff1f3d8e8f2bb59310e3' }
+        let(:external_pull_request_status) { :open }
 
-        it 'only saves pull request info' do
-          expect(Ci::CreatePipelineService).not_to receive(:new)
+        before do
+          create(:external_pull_request,
+            project: project,
+            source_branch: 'feature',
+            source_sha: 'ce85aeb75247ab87b5f974adc78a924a335966fe',
+            status: external_pull_request_status)
+        end
 
-          expect { subject.execute(params) }.to change { ExternalPullRequest.count }.by(1)
+        shared_examples 'updates pull request' do |pull_request_status|
+          it 'updates the pull request without creating any pipeline' do
+            expect(Ci::CreatePipelineService).not_to receive(:new)
+
+            pull_request = subject.execute(params)
+
+            expect(pull_request).to be_persisted
+            expect(pull_request.project).to eq(project)
+            expect(pull_request.source_branch).to eq('feature')
+            expect(pull_request.source_sha).to eq(source_sha)
+            expect(pull_request.source_repository).to eq('the-repo')
+            expect(pull_request.target_branch).to eq('the-target-branch')
+            expect(pull_request.target_repository).to eq('the-repo')
+            expect(pull_request.status).to eq(pull_request_status)
+          end
+        end
 
-          pull_request = ExternalPullRequest.last
+        context 'when pull request webhook action is "synchronize"' do
+          let(:action) { 'synchronize' }
 
-          expect(pull_request).to be_persisted
-          expect(pull_request.project).to eq(project)
-          expect(pull_request.source_branch).to eq('a-new-branch')
-          expect(pull_request.source_repository).to eq('the-repo')
-          expect(pull_request.target_branch).to eq('the-target-branch')
-          expect(pull_request.target_repository).to eq('the-repo')
-          expect(pull_request.status).to eq('closed')
+          it_behaves_like 'updates pull request', 'open'
         end
-      end
 
-      context 'when pull request webhook has unsupported action' do
-        let(:source_branch) { double }
-        let(:source_sha) { double }
-        let(:action) { 'assigned' }
+        context 'when pull request webhook action is "closed"' do
+          let(:action) { 'closed' }
 
-        it 'returns nil' do
-          expect(subject.execute(params)).to be_nil
+          it_behaves_like 'updates pull request', 'closed'
         end
-      end
 
-      context 'project is not a mirror' do
-        let(:source_branch) { double }
-        let(:source_sha) { double }
+        context 'when pull request webhook action is "reopened"' do
+          let(:external_pull_request_status) { :closed }
+          let(:action) { 'reopened' }
 
-        before do
-          allow(project).to receive(:mirror?).and_return(false)
+          it_behaves_like 'updates pull request', 'open'
         end
 
-        it 'returns nil' do
-          expect(subject.execute(params)).to be_nil
+        context 'when pull request webhook action is unsupported' do
+          let(:action) { 'assigned' }
+
+          it 'returns nil' do
+            expect(subject.execute(params)).to be_nil
+          end
         end
-      end
 
-      context 'when pull request webhook any missing params' do
-        let(:source_branch) { nil }
-        let(:source_sha) { nil }
+        context 'when pull request webhook any missing params' do
+          let(:source_branch) { nil }
+          let(:source_sha) { nil }
 
-        it 'returns a pull request with errors' do
-          expect(subject.execute(params).errors).not_to be_empty
+          it 'returns a pull request with errors' do
+            expect(subject.execute(params).errors).not_to be_empty
+          end
         end
       end
     end