Automatic merge of gitlab-org/gitlab master

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

@@ -3,13 +3,11 @@ import { GlButton, GlSkeletonLoader } from '@gitlab/ui';
 import createFlash from '~/flash';
 import { __ } from '~/locale';
 import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
-import ActionsButton from '~/vue_shared/components/actions_button.vue';
 import simplePoll from '../../../lib/utils/simple_poll';
 import eventHub from '../../event_hub';
 import mergeRequestQueryVariablesMixin from '../../mixins/merge_request_query_variables';
 import rebaseQuery from '../../queries/states/rebase.query.graphql';
 import statusIcon from '../mr_widget_status_icon.vue';
-import { REBASE_BUTTON_KEY, REBASE_WITHOUT_CI_BUTTON_KEY } from '../../constants';
 
 export default {
   name: 'MRWidgetRebase',
@@ -28,7 +26,6 @@ export default {
   components: {
     statusIcon,
     GlSkeletonLoader,
-    ActionsButton,
     GlButton,
   },
   mixins: [glFeatureFlagMixin(), mergeRequestQueryVariablesMixin],
@@ -47,7 +44,6 @@ export default {
       state: {},
       isMakingRequest: false,
       rebasingError: null,
-      selectedRebaseAction: REBASE_BUTTON_KEY,
     };
   },
   computed: {
@@ -93,28 +89,6 @@ export default {
     fastForwardMergeText() {
       return __('Merge blocked: the source branch must be rebased onto the target branch.');
     },
-    actions() {
-      return [this.rebaseAction, this.rebaseWithoutCiAction].filter((action) => action);
-    },
-    rebaseAction() {
-      return {
-        key: REBASE_BUTTON_KEY,
-        text: __('Rebase'),
-        secondaryText: __('Rebases and triggers a pipeline'),
-        attrs: {
-          'data-qa-selector': 'mr_rebase_button',
-        },
-        handle: () => this.rebase(),
-      };
-    },
-    rebaseWithoutCiAction() {
-      return {
-        key: REBASE_WITHOUT_CI_BUTTON_KEY,
-        text: __('Rebase without CI'),
-        secondaryText: __('Performs a rebase but skips triggering a new pipeline'),
-        handle: () => this.rebase({ skipCi: true }),
-      };
-    },
   },
   methods: {
     rebase({ skipCi = false } = {}) {
@@ -138,8 +112,8 @@ export default {
           }
         });
     },
-    selectRebaseAction(key) {
-      this.selectedRebaseAction = key;
+    rebaseWithoutCi() {
+      return this.rebase({ skipCi: true });
     },
     checkRebaseStatus(continuePolling, stopPolling) {
       this.service
@@ -198,10 +172,10 @@ export default {
         >
         <div
           v-if="!rebaseInProgress && canPushToSourceBranch && !isMakingRequest"
-          class="accept-merge-holder clearfix js-toggle-container accept-action media space-children"
+          class="accept-merge-holder clearfix js-toggle-container accept-action media space-children gl-align-items-center"
         >
           <gl-button
-            v-if="!glFeatures.restructuredMrWidget && !showRebaseWithoutCi"
+            v-if="!glFeatures.restructuredMrWidget"
             :loading="isMakingRequest"
             variant="confirm"
             data-qa-selector="mr_rebase_button"
@@ -210,14 +184,16 @@ export default {
           >
             {{ __('Rebase') }}
           </gl-button>
-          <actions-button
+          <gl-button
             v-if="!glFeatures.restructuredMrWidget && showRebaseWithoutCi"
-            :actions="actions"
-            :selected-key="selectedRebaseAction"
+            :loading="isMakingRequest"
             variant="confirm"
-            category="primary"
-            @select="selectRebaseAction"
-          />
+            category="secondary"
+            data-testid="rebase-without-ci-button"
+            @click="rebaseWithoutCi"
+          >
+            {{ __('Rebase without pipeline') }}
+          </gl-button>
           <span
             v-if="!rebasingError"
             :class="{ 'gl-ml-0! gl-text-body!': glFeatures.restructuredMrWidget }"

app/assets/javascripts/vue_merge_request_widget/constants.js

@@ -166,6 +166,3 @@ export const EXTENSION_SUMMARY_FAILED_CLASS = 'gl-text-red-500';
 export const EXTENSION_SUMMARY_NEUTRAL_CLASS = 'gl-text-gray-700';
 
 export { STATE_MACHINE };
-
-export const REBASE_BUTTON_KEY = 'rebase';
-export const REBASE_WITHOUT_CI_BUTTON_KEY = 'rebaseWithoutCi';

app/controllers/concerns/uploads_actions.rb

@@ -143,7 +143,7 @@ module UploadsActions
   end
 
   def bypass_auth_checks_on_uploads?
-    if ::Feature.enabled?(:enforce_auth_checks_on_uploads, default_enabled: :yaml)
+    if ::Feature.enabled?(:enforce_auth_checks_on_uploads, project, default_enabled: :yaml)
       false
     else
       action_name == 'show' && embeddable?

app/controllers/groups/uploads_controller.rb

@@ -4,7 +4,7 @@ class Groups::UploadsController < Groups::ApplicationController
   include UploadsActions
   include WorkhorseRequest
 
-  skip_before_action :group, if: -> { bypass_auth_checks_on_uploads? }
+  skip_before_action :group, if: -> { action_name == 'show' && embeddable? }
 
   before_action :authorize_upload_file!, only: [:create, :authorize]
   before_action :verify_workhorse_api!, only: [:authorize]

doc/user/project/merge_requests/fast_forward_merge.md

@@ -43,7 +43,7 @@ You can also rebase without running a CI/CD pipeline.
 
 The rebase action is also available as a [quick action command: `/rebase`](../../../topics/git/git_rebase.md#rebase-from-the-gitlab-ui).
 
-![Fast forward merge request](img/ff_merge_rebase_v14_7.png)
+![Fast forward merge request](img/ff_merge_rebase_v14_9.png)
 
 If the target branch is ahead of the source branch and a conflict free rebase is
 not possible, you need to rebase the

locale/gitlab.pot

@@ -26531,9 +26531,6 @@ msgstr ""
 msgid "PerformanceBar|wall"
 msgstr ""
 
-msgid "Performs a rebase but skips triggering a new pipeline"
-msgstr ""
-
 msgid "Period in seconds"
 msgstr ""
 
@@ -30041,10 +30038,7 @@ msgstr ""
 msgid "Rebase source branch on the target branch."
 msgstr ""
 
-msgid "Rebase without CI"
-msgstr ""
-
-msgid "Rebases and triggers a pipeline"
+msgid "Rebase without pipeline"
 msgstr ""
 
 msgid "Recaptcha verified?"

spec/frontend/vue_mr_widget/components/mr_widget_rebase_spec.js

@@ -2,11 +2,6 @@ import { shallowMount } from '@vue/test-utils';
 import { nextTick } from 'vue';
 import WidgetRebase from '~/vue_merge_request_widget/components/states/mr_widget_rebase.vue';
 import eventHub from '~/vue_merge_request_widget/event_hub';
-import ActionsButton from '~/vue_shared/components/actions_button.vue';
-import {
-  REBASE_BUTTON_KEY,
-  REBASE_WITHOUT_CI_BUTTON_KEY,
-} from '~/vue_merge_request_widget/constants';
 
 let wrapper;
 
@@ -38,8 +33,8 @@ function createWrapper(propsData, mergeRequestWidgetGraphql, rebaseWithoutCiUi)
 describe('Merge request widget rebase component', () => {
   const findRebaseMessage = () => wrapper.find('[data-testid="rebase-message"]');
   const findRebaseMessageText = () => findRebaseMessage().text();
-  const findRebaseButtonActions = () => wrapper.find(ActionsButton);
   const findStandardRebaseButton = () => wrapper.find('[data-testid="standard-rebase-button"]');
+  const findRebaseWithoutCiButton = () => wrapper.find('[data-testid="rebase-without-ci-button"]');
 
   afterEach(() => {
     wrapper.destroy();
@@ -112,7 +107,7 @@ describe('Merge request widget rebase component', () => {
           expect(findRebaseMessageText()).toContain('Something went wrong!');
         });
 
-        describe('Rebase button with flag rebaseWithoutCiUi', () => {
+        describe('Rebase buttons with flag rebaseWithoutCiUi', () => {
           beforeEach(() => {
             createWrapper(
               {
@@ -130,30 +125,13 @@ describe('Merge request widget rebase component', () => {
             );
           });
 
-          it('rebase button with actions is rendered', () => {
-            expect(findRebaseButtonActions().exists()).toBe(true);
-            expect(findStandardRebaseButton().exists()).toBe(false);
-          });
-
-          it('has rebase and rebase without CI actions', () => {
-            const actionNames = findRebaseButtonActions()
-              .props('actions')
-              .map((action) => action.key);
-
-            expect(actionNames).toStrictEqual([REBASE_BUTTON_KEY, REBASE_WITHOUT_CI_BUTTON_KEY]);
-          });
-
-          it('defaults to rebase action', () => {
-            expect(findRebaseButtonActions().props('selectedKey')).toStrictEqual(REBASE_BUTTON_KEY);
+          it('renders both buttons', () => {
+            expect(findRebaseWithoutCiButton().exists()).toBe(true);
+            expect(findStandardRebaseButton().exists()).toBe(true);
           });
 
           it('starts the rebase when clicking', async () => {
-            // ActionButtons use the actions props instead of emitting
-            // a click event, therefore simulating the behavior here:
-            findRebaseButtonActions()
-              .props('actions')
-              .find((x) => x.key === REBASE_BUTTON_KEY)
-              .handle();
+            findStandardRebaseButton().vm.$emit('click');
 
             await nextTick();
 
@@ -161,12 +139,7 @@ describe('Merge request widget rebase component', () => {
           });
 
           it('starts the CI-skipping rebase when clicking on "Rebase without CI"', async () => {
-            // ActionButtons use the actions props instead of emitting
-            // a click event, therefore simulating the behavior here:
-            findRebaseButtonActions()
-              .props('actions')
-              .find((x) => x.key === REBASE_WITHOUT_CI_BUTTON_KEY)
-              .handle();
+            findRebaseWithoutCiButton().vm.$emit('click');
 
             await nextTick();
 
@@ -193,7 +166,7 @@ describe('Merge request widget rebase component', () => {
 
           it('standard rebase button is rendered', () => {
             expect(findStandardRebaseButton().exists()).toBe(true);
-            expect(findRebaseButtonActions().exists()).toBe(false);
+            expect(findRebaseWithoutCiButton().exists()).toBe(false);
           });
 
           it('calls rebase method with skip_ci false', () => {
@@ -240,7 +213,7 @@ describe('Merge request widget rebase component', () => {
           });
         });
 
-        it('does not render the rebase actions button with rebaseWithoutCiUI flag enabled', () => {
+        it('does not render the "Rebase without pipeline" button with rebaseWithoutCiUI flag enabled', () => {
           createWrapper(
             {
               mr: {
@@ -254,7 +227,7 @@ describe('Merge request widget rebase component', () => {
             { rebaseWithoutCiUi: true },
           );
 
-          expect(findRebaseButtonActions().exists()).toBe(false);
+          expect(findRebaseWithoutCiButton().exists()).toBe(false);
         });
 
         it('does not render the standard rebase button with rebaseWithoutCiUI flag disabled', () => {

spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb

@@ -211,10 +211,22 @@ RSpec.shared_examples 'handle uploads' do
                   stub_feature_flags(enforce_auth_checks_on_uploads: true)
                 end
 
-                it "responds with status 302" do
+                it "responds with appropriate status" do
                   show_upload
 
-                  expect(response).to have_gitlab_http_status(:redirect)
+                  # We're switching here based on the class due to the feature
+                  #   flag :enforce_auth_checks_on_uploads switching on project.
+                  #   When it is enabled fully, we will apply the code it guards
+                  #   to both Projects::UploadsController as well as
+                  #   Groups::UploadsController.
+                  #
+                  # https://gitlab.com/gitlab-org/gitlab/-/issues/352291
+                  #
+                  if model.instance_of?(Group)
+                    expect(response).to have_gitlab_http_status(:ok)
+                  else
+                    expect(response).to have_gitlab_http_status(:redirect)
+                  end
                 end
               end
 
@@ -305,7 +317,19 @@ RSpec.shared_examples 'handle uploads' do
                   it "responds with status 404" do
                     show_upload
 
-                    expect(response).to have_gitlab_http_status(:not_found)
+                    # We're switching here based on the class due to the feature
+                    #   flag :enforce_auth_checks_on_uploads switching on
+                    #   project. When it is enabled fully, we will apply the
+                    #   code it guards to both Projects::UploadsController as
+                    #   well as Groups::UploadsController.
+                    #
+                    # https://gitlab.com/gitlab-org/gitlab/-/issues/352291
+                    #
+                    if model.instance_of?(Group)
+                      expect(response).to have_gitlab_http_status(:ok)
+                    else
+                      expect(response).to have_gitlab_http_status(:not_found)
+                    end
                   end
                 end