Merge branch 'ph/widgetGraphqlTestRun' into 'master'

[RUN AS-IF-FOSS] Fixes some bugs in the widgets GraphQL state

See merge request gitlab-org/gitlab!51471

app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals.vue

@@ -159,6 +159,7 @@ export default {
         .then((data) => {
           this.mr.setApprovals(data);
           eventHub.$emit('MRWidgetUpdateRequested');
+          eventHub.$emit('ApprovalUpdated');
           this.$emit('updated');
         })
         .catch(errFn)

app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_auto_merge_enabled.vue

@@ -4,6 +4,7 @@ import autoMergeMixin from 'ee_else_ce/vue_merge_request_widget/mixins/auto_merg
 import autoMergeEnabledQuery from 'ee_else_ce/vue_merge_request_widget/queries/states/auto_merge_enabled.query.graphql';
 import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import { __ } from '~/locale';
+import { getIdFromGraphQLId } from '~/graphql_shared/utils';
 import { deprecatedCreateFlash as Flash } from '../../../flash';
 import statusIcon from '../mr_widget_status_icon.vue';
 import MrWidgetAuthor from '../mr_widget_author.vue';
@@ -53,7 +54,11 @@ export default {
   },
   computed: {
     loading() {
-      return this.glFeatures.mergeRequestWidgetGraphql && this.$apollo.queries.state.loading;
+      return (
+        this.glFeatures.mergeRequestWidgetGraphql &&
+        this.$apollo.queries.state.loading &&
+        Object.keys(this.state).length === 0
+      );
     },
     mergeUser() {
       if (this.glFeatures.mergeRequestWidgetGraphql) {
@@ -78,7 +83,7 @@ export default {
     canRemoveSourceBranch() {
       const { currentUserId } = this.mr;
       const mergeUserId = this.glFeatures.mergeRequestWidgetGraphql
-        ? this.state.mergeUser?.id
+        ? getIdFromGraphQLId(this.state.mergeUser?.id)
         : this.mr.mergeUserId;
       const canRemoveSourceBranch = this.glFeatures.mergeRequestWidgetGraphql
         ? this.state.userPermissions.removeSourceBranch
@@ -96,7 +101,11 @@ export default {
         .cancelAutomaticMerge()
         .then((res) => res.data)
         .then((data) => {
-          eventHub.$emit('UpdateWidgetData', data);
+          if (this.glFeatures.mergeRequestWidgetGraphql) {
+            eventHub.$emit('MRWidgetUpdateRequested');
+          } else {
+            eventHub.$emit('UpdateWidgetData', data);
+          }
         })
         .catch(() => {
           this.isCancellingAutoMerge = false;
@@ -119,6 +128,11 @@ export default {
             eventHub.$emit('MRWidgetUpdateRequested');
           }
         })
+        .then(() => {
+          if (this.glFeatures.mergeRequestWidgetGraphql) {
+            this.$apollo.queries.state.refetch();
+          }
+        })
         .catch(() => {
           this.isRemovingSourceBranch = false;
           Flash(__('Something went wrong. Please try again.'));

app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_rebase.vue

@@ -7,7 +7,7 @@ import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import simplePoll from '../../../lib/utils/simple_poll';
 import eventHub from '../../event_hub';
 import statusIcon from '../mr_widget_status_icon.vue';
-import rebaseQuery from '../../queries/states/ready_to_merge.query.graphql';
+import rebaseQuery from '../../queries/states/rebase.query.graphql';
 import mergeRequestQueryVariablesMixin from '../../mixins/merge_request_query_variables';
 import { deprecatedCreateFlash as Flash } from '../../../flash';
 

app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue

@@ -53,8 +53,8 @@ export default {
       result({ data }) {
         this.state = {
           ...data.project.mergeRequest,
-          mergeRequestsFfOnlyEnabled: data.mergeRequestsFfOnlyEnabled,
-          onlyAllowMergeIfPipelineSucceeds: data.onlyAllowMergeIfPipelineSucceeds,
+          mergeRequestsFfOnlyEnabled: data.project.mergeRequestsFfOnlyEnabled,
+          onlyAllowMergeIfPipelineSucceeds: data.project.onlyAllowMergeIfPipelineSucceeds,
         };
         this.removeSourceBranch = data.project.mergeRequest.shouldRemoveSourceBranch;
         this.commitMessage = data.project.mergeRequest.defaultMergeCommitMessage;
@@ -277,7 +277,20 @@ export default {
       return this.mr.mergeRequestDiffsPath;
     },
   },
+  mounted() {
+    if (this.glFeatures.mergeRequestWidgetGraphql) {
+      eventHub.$on('ApprovalUpdated', this.updateGraphqlState);
+    }
+  },
+  beforeDestroy() {
+    if (this.glFeatures.mergeRequestWidgetGraphql) {
+      eventHub.$off('ApprovalUpdated', this.updateGraphqlState);
+    }
+  },
   methods: {
+    updateGraphqlState() {
+      return this.$apollo.queries.state.refetch();
+    },
     updateMergeCommitMessage(includeDescription) {
       const commitMessage = this.glFeatures.mergeRequestWidgetGraphql
         ? this.state.defaultMergeCommitMessage
@@ -326,6 +339,10 @@ export default {
           } else if (hasError) {
             eventHub.$emit('FailedToMerge', data.merge_error);
           }
+
+          if (this.glFeatures.mergeRequestWidgetGraphql) {
+            this.updateGraphqlState();
+          }
         })
         .catch(() => {
           this.isMakingRequest = false;
@@ -532,7 +549,7 @@ export default {
       </div>
       <merge-train-helper-text
         v-if="shouldRenderMergeTrainHelperText"
-        :pipeline-id="pipeline.id"
+        :pipeline-id="pipelineId"
         :pipeline-link="pipeline.path"
         :merge-train-length="stateData.mergeTrainsCount"
         :merge-train-when-pipeline-succeeds-docs-path="mr.mergeTrainWhenPipelineSucceedsDocsPath"

app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js

@@ -35,5 +35,8 @@ export default {
     shouldRenderMergeTrainHelperText() {
       return false;
     },
+    pipelineId() {
+      return this.pipeline.id;
+    },
   },
 };

app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue

@@ -94,7 +94,6 @@ export default {
     state: {
       query: getStateQuery,
       manual: true,
-      pollInterval: 10 * 1000,
       skip() {
         return !this.mr || !window.gon?.features?.mergeRequestWidgetGraphql;
       },
@@ -286,6 +285,10 @@ export default {
       return new MRWidgetService(this.getServiceEndpoints(store));
     },
     checkStatus(cb, isRebased) {
+      if (window.gon?.features?.mergeRequestWidgetGraphql) {
+        this.$apollo.queries.state.refetch();
+      }
+
       return this.service
         .checkStatus()
         .then(({ data }) => {

app/assets/javascripts/vue_merge_request_widget/queries/get_state.query.graphql

@@ -18,6 +18,7 @@ query getState($projectPath: ID!, $iid: String!) {
       }
       shouldBeRebased
       sourceBranchExists
+      state
       targetBranchExists
       userPermissions {
         canMerge

app/assets/javascripts/vue_merge_request_widget/queries/states/auto_merge_enabled.fragment.graphql

 fragment autoMergeEnabled on MergeRequest {
   autoMergeStrategy
   mergeUser {
+    id
     name
     username
     webUrl

app/assets/javascripts/vue_merge_request_widget/queries/states/auto_merge_enabled.query.graphql

@@ -4,7 +4,6 @@ query autoMergeEnabledQuery($projectPath: ID!, $iid: String!) {
   project(fullPath: $projectPath) {
     mergeRequest(iid: $iid) {
       ...autoMergeEnabled
-      mergeTrainsCount
     }
   }
 }

app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js

@@ -156,9 +156,9 @@ export default class MergeRequestStore {
 
     this.setState();
 
-    mrEventHub.$emit('mr.state.updated', {
-      state: this.mergeRequestState,
-    });
+    if (!window.gon?.features?.mergeRequestWidgetGraphql) {
+      this.emitUpdatedState();
+    }
   }
 
   setGraphqlData(project) {
@@ -182,7 +182,9 @@ export default class MergeRequestStore {
     this.isSHAMismatch = this.sha !== mergeRequest.diffHeadSha;
     this.shouldBeRebased = mergeRequest.shouldBeRebased;
     this.workInProgress = mergeRequest.workInProgress;
+    this.mergeRequestState = mergeRequest.state;
 
+    this.emitUpdatedState();
     this.setState();
   }
 
@@ -208,6 +210,12 @@ export default class MergeRequestStore {
     }
   }
 
+  emitUpdatedState() {
+    mrEventHub.$emit('mr.state.updated', {
+      state: this.mergeRequestState,
+    });
+  }
+
   setPaths(data) {
     // Paths are set on the first load of the page and not auto-refreshed
     this.squashBeforeMergeHelpPath = data.squash_before_merge_help_path;

ee/app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js

@@ -2,6 +2,7 @@ import { isNumber, isString } from 'lodash';
 import { MTWPS_MERGE_STRATEGY, MT_MERGE_STRATEGY } from '~/vue_merge_request_widget/constants';
 import { __ } from '~/locale';
 import base from '~/vue_merge_request_widget/mixins/ready_to_merge';
+import { getIdFromGraphQLId } from '~/graphql_shared/utils';
 
 export const MERGE_DISABLED_TEXT_UNAPPROVED = __(
   'You can only merge once this merge request is approved.',
@@ -49,10 +50,17 @@ export default {
       }
       return __('Merge when pipeline succeeds');
     },
+    pipelineId() {
+      if (this.glFeatures.mergeRequestWidgetGraphql) {
+        return getIdFromGraphQLId(this.pipeline.id);
+      }
+
+      return this.pipeline.id;
+    },
     shouldRenderMergeTrainHelperText() {
       return (
         this.pipeline &&
-        isNumber(this.pipeline.id) &&
+        isNumber(this.pipelineId) &&
         isString(this.pipeline.path) &&
         this.preferredAutoMergeStrategy === MTWPS_MERGE_STRATEGY &&
         !this.stateData.autoMergeEnabled

ee/app/assets/javascripts/vue_merge_request_widget/queries/states/auto_merge_enabled.query.graphql

@@ -4,6 +4,7 @@ query autoMergeEnabledQuery($projectPath: ID!, $iid: String!) {
   project(fullPath: $projectPath) {
     mergeRequest(iid: $iid) {
       ...autoMergeEnabled
+      mergeTrainsCount
     }
   }
 }

ee/spec/features/projects/merge_requests/user_approves_merge_request_spec.rb

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 RSpec.describe 'User approves a merge request', :js do
   let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
-  let(:project) { create(:project, :repository, approvals_before_merge: 1) }
+  let(:project) { create(:project, :repository, :public, approvals_before_merge: 1) }
   let(:user) { create(:user) }
   let(:user2) { create(:user) }
   let(:user3) { create(:user) }
@@ -77,7 +77,8 @@ RSpec.describe 'User approves a merge request', :js do
 
   context 'when user cannot approve' do
     before do
-      merge_request.approvers.create!(user_id: user2.id)
+      sign_out(user)
+      sign_in(create(:user))
 
       visit(merge_request_path(merge_request))
     end

spec/features/merge_request/user_merges_when_pipeline_succeeds_spec.rb

@@ -68,7 +68,7 @@ RSpec.describe 'Merge request > User merges when pipeline succeeds', :js do
 
           wait_for_requests
 
-          expect(page).to have_content 'Merge when pipeline succeeds', wait: 0
+          expect(page).to have_content 'Merge when pipeline succeeds'
         end
 
         it_behaves_like 'Merge when pipeline succeeds activator'

spec/frontend/vue_mr_widget/components/states/mr_widget_auto_merge_enabled_spec.js

@@ -202,7 +202,11 @@ describe('MRWidgetAutoMergeEnabled', () => {
             wrapper.vm.cancelAutomaticMerge();
             setImmediate(() => {
               expect(wrapper.vm.isCancellingAutoMerge).toBeTruthy();
-              expect(eventHub.$emit).toHaveBeenCalledWith('UpdateWidgetData', mrObj);
+              if (mergeRequestWidgetGraphql) {
+                expect(eventHub.$emit).toHaveBeenCalledWith('MRWidgetUpdateRequested');
+              } else {
+                expect(eventHub.$emit).toHaveBeenCalledWith('UpdateWidgetData', mrObj);
+              }
               done();
             });
           });

spec/spec_helper.rb

@@ -220,7 +220,7 @@ RSpec.configure do |config|
 
       # Merge request widget GraphQL requests are disabled in the tests
       # for now whilst we migrate as much as we can over the GraphQL
-      stub_feature_flags(merge_request_widget_graphql: false)
+      # stub_feature_flags(merge_request_widget_graphql: false)
 
       # Using FortiAuthenticator as OTP provider is disabled by default in
       # tests, until we introduce it in user settings