From 04ca758459b33d66a12a29cc6e7d9d75e3e23fd9 Mon Sep 17 00:00:00 2001
From: Briley Sandlin <bsandlin@gitlab.com>
Date: Wed, 16 Oct 2024 12:06:12 -0500
Subject: [PATCH 1/6] PA team paper cuts 17.6

Completing various UX polish issues for pipeline authoring

Changelog: changed
---
 .../ci/ci_variable_list/components/ci_variable_drawer.vue  | 6 +++---
 .../components/table/cells/pipeline_schedule_actions.vue   | 6 +++---
 .../pipelines_page/components/empty_state/ci_templates.vue | 2 +-
 locale/gitlab.pot                                          | 7 +++++--
 4 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_drawer.vue b/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_drawer.vue
index ce7a29c85d2832..9ffbda6f7329a4 100644
--- a/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_drawer.vue
+++ b/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_drawer.vue
@@ -400,6 +400,7 @@ export default {
     },
     submit() {
       this.$emit(this.isEditing ? 'update-variable' : 'add-variable', this.variableToEmit);
+      this.$refs.drawer.$el.scrollTo({ top: 0, behavior: 'smooth' });
     },
     trackVariableValidationErrors() {
       const property = this.getTrackingErrorProperty();
@@ -440,6 +441,7 @@ export default {
 <template>
   <div>
     <gl-drawer
+      ref="drawer"
       open
       data-testid="ci-variable-drawer"
       :header-height="getDrawerHeaderHeight"
@@ -663,10 +665,9 @@ export default {
       >
         {{ $options.i18n.variableReferenceDescription }}
       </gl-alert>
-      <div class="gl-flex">
+      <div class="gl-mb-5 gl-flex gl-gap-3">
         <gl-button
           category="primary"
-          class="gl-mr-3"
           variant="confirm"
           :disabled="!canSubmit"
           data-testid="ci-variable-confirm-button"
@@ -678,7 +679,6 @@ export default {
           v-gl-modal-directive="`delete-variable-${variable.key}`"
           variant="danger"
           category="secondary"
-          class="gl-mr-3"
           data-testid="ci-variable-delete-button"
           >{{ $options.i18n.deleteVariable }}</gl-button
         >
diff --git a/app/assets/javascripts/ci/pipeline_schedules/components/table/cells/pipeline_schedule_actions.vue b/app/assets/javascripts/ci/pipeline_schedules/components/table/cells/pipeline_schedule_actions.vue
index d61b0dd45d68cd..7e5784db74868f 100644
--- a/app/assets/javascripts/ci/pipeline_schedules/components/table/cells/pipeline_schedule_actions.vue
+++ b/app/assets/javascripts/ci/pipeline_schedules/components/table/cells/pipeline_schedule_actions.vue
@@ -4,9 +4,9 @@ import { s__ } from '~/locale';
 import { getIdFromGraphQLId } from '~/graphql_shared/utils';
 
 export const i18n = {
-  playTooltip: s__('PipelineSchedules|Run pipeline schedule'),
-  editTooltip: s__('PipelineSchedules|Edit pipeline schedule'),
-  deleteTooltip: s__('PipelineSchedules|Delete pipeline schedule'),
+  playTooltip: s__('PipelineSchedules|Run scheduled pipeline'),
+  editTooltip: s__('PipelineSchedules|Edit scheduled pipeline'),
+  deleteTooltip: s__('PipelineSchedules|Delete scheduled pipeline'),
   takeOwnershipTooltip: s__('PipelineSchedules|Take ownership of pipeline schedule'),
 };
 
diff --git a/app/assets/javascripts/ci/pipelines_page/components/empty_state/ci_templates.vue b/app/assets/javascripts/ci/pipelines_page/components/empty_state/ci_templates.vue
index 3b46e50b5d5852..edb440c4e37dc0 100644
--- a/app/assets/javascripts/ci/pipelines_page/components/empty_state/ci_templates.vue
+++ b/app/assets/javascripts/ci/pipelines_page/components/empty_state/ci_templates.vue
@@ -93,7 +93,7 @@ export default {
         <gl-button
           :disabled="disabled"
           category="primary"
-          variant="confirm"
+          variant="default"
           :href="template.link"
           data-testid="template-link"
           @click="trackEvent(template.name)"
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index d51b2aba9dc1cd..d7b6c9a5caa8f8 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -40208,10 +40208,13 @@ msgstr ""
 msgid "PipelineSchedules|Delete pipeline schedule"
 msgstr ""
 
+msgid "PipelineSchedules|Delete scheduled pipeline"
+msgstr ""
+
 msgid "PipelineSchedules|Description"
 msgstr ""
 
-msgid "PipelineSchedules|Edit pipeline schedule"
+msgid "PipelineSchedules|Edit scheduled pipeline"
 msgstr ""
 
 msgid "PipelineSchedules|Explore plan limits"
@@ -40259,7 +40262,7 @@ msgstr ""
 msgid "PipelineSchedules|Provide a short description for this pipeline"
 msgstr ""
 
-msgid "PipelineSchedules|Run pipeline schedule"
+msgid "PipelineSchedules|Run scheduled pipeline"
 msgstr ""
 
 msgid "PipelineSchedules|Runs for a specific branch or tag."
-- 
GitLab


From fd10674f68a85565bec9ccf34d5fcf1bde732a2f Mon Sep 17 00:00:00 2001
From: Briley Sandlin <bsandlin@gitlab.com>
Date: Mon, 21 Oct 2024 12:36:32 -0500
Subject: [PATCH 2/6] MR feedback - MM

---
 .../components/ci_variable_drawer_spec.js            |  4 ++++
 .../pipeline_failed_jobs_widget_spec.js              | 12 +++++++++---
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/spec/frontend/ci/ci_variable_list/components/ci_variable_drawer_spec.js b/spec/frontend/ci/ci_variable_list/components/ci_variable_drawer_spec.js
index 7a11487279d0e8..3853b33391c787 100644
--- a/spec/frontend/ci/ci_variable_list/components/ci_variable_drawer_spec.js
+++ b/spec/frontend/ci/ci_variable_list/components/ci_variable_drawer_spec.js
@@ -569,8 +569,11 @@ describe('CI Variable Drawer', () => {
     });
 
     describe('when adding a variable', () => {
+      let scrollSpy;
+
       beforeEach(() => {
         createComponent({ stubs: { GlDrawer } });
+        scrollSpy = jest.spyOn(wrapper.vm.$refs.drawer.$el, 'scrollTo');
       });
 
       it('title and confirm button renders the correct text', () => {
@@ -609,6 +612,7 @@ describe('CI Variable Drawer', () => {
           ],
         ]);
         expect(wrapper.emitted('close-form')).toBeUndefined();
+        expect(scrollSpy).toHaveBeenCalled();
       });
     });
 
diff --git a/spec/frontend/ci/pipelines_page/components/failure_widget/pipeline_failed_jobs_widget_spec.js b/spec/frontend/ci/pipelines_page/components/failure_widget/pipeline_failed_jobs_widget_spec.js
index a764143b00267c..d9cebe143bc72b 100644
--- a/spec/frontend/ci/pipelines_page/components/failure_widget/pipeline_failed_jobs_widget_spec.js
+++ b/spec/frontend/ci/pipelines_page/components/failure_widget/pipeline_failed_jobs_widget_spec.js
@@ -41,16 +41,22 @@ describe('PipelineFailedJobsWidget component', () => {
   const findCount = () => wrapper.findComponent(GlBadge);
 
   describe('when there are failed jobs', () => {
-    beforeEach(() => {
+    it('renders the show failed jobs button with correct count', () => {
       createComponent();
-    });
 
-    it('renders the show failed jobs button with correct count', () => {
       expect(findFailedJobsButton().exists()).toBe(true);
       expect(findCount().text()).toBe(String(defaultProps.failedJobsCount));
     });
 
+    it('renders the maximum job count label when count is > 100', () => {
+      createComponent({ props: { failedJobsCount: 102 } });
+
+      expect(findFailedJobsButton().text()).toContain('(100+)');
+    });
+
     it('does not render the failed jobs widget', () => {
+      createComponent();
+
       expect(findFailedJobsList().exists()).toBe(false);
     });
   });
-- 
GitLab


From c0997ac0100ef8552a8969c4eb1538c81cabb697 Mon Sep 17 00:00:00 2001
From: Briley Sandlin <bsandlin@gitlab.com>
Date: Tue, 22 Oct 2024 13:43:32 -0500
Subject: [PATCH 3/6] MR feedback 2 - MM

---
 .../ci_variable_list/components/ci_variable_drawer_spec.js   | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/spec/frontend/ci/ci_variable_list/components/ci_variable_drawer_spec.js b/spec/frontend/ci/ci_variable_list/components/ci_variable_drawer_spec.js
index 3853b33391c787..de332d56476c68 100644
--- a/spec/frontend/ci/ci_variable_list/components/ci_variable_drawer_spec.js
+++ b/spec/frontend/ci/ci_variable_list/components/ci_variable_drawer_spec.js
@@ -569,11 +569,8 @@ describe('CI Variable Drawer', () => {
     });
 
     describe('when adding a variable', () => {
-      let scrollSpy;
-
       beforeEach(() => {
         createComponent({ stubs: { GlDrawer } });
-        scrollSpy = jest.spyOn(wrapper.vm.$refs.drawer.$el, 'scrollTo');
       });
 
       it('title and confirm button renders the correct text', () => {
@@ -612,7 +609,7 @@ describe('CI Variable Drawer', () => {
           ],
         ]);
         expect(wrapper.emitted('close-form')).toBeUndefined();
-        expect(scrollSpy).toHaveBeenCalled();
+        expect(wrapper.findComponent(GlDrawer).element.scrollTo).toHaveBeenCalledTimes(1);
       });
     });
 
-- 
GitLab


From 5e27612512ba6573d8de79d48eb974aa17b0a671 Mon Sep 17 00:00:00 2001
From: Briley Sandlin <bsandlin@gitlab.com>
Date: Wed, 23 Oct 2024 09:07:58 -0500
Subject: [PATCH 4/6] fix unit tests

---
 .../components/delete_pipeline_schedule_modal.vue        | 4 ++--
 app/views/projects/pipeline_schedules/edit.html.haml     | 2 +-
 locale/gitlab.pot                                        | 9 +++------
 spec/features/projects/pipeline_schedules_spec.rb        | 6 +++---
 4 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/app/assets/javascripts/ci/pipeline_schedules/components/delete_pipeline_schedule_modal.vue b/app/assets/javascripts/ci/pipeline_schedules/components/delete_pipeline_schedule_modal.vue
index 92c824fb5a1c49..99a2ec3b9f8636 100644
--- a/app/assets/javascripts/ci/pipeline_schedules/components/delete_pipeline_schedule_modal.vue
+++ b/app/assets/javascripts/ci/pipeline_schedules/components/delete_pipeline_schedule_modal.vue
@@ -6,10 +6,10 @@ export default {
   modal: {
     id: 'delete-pipeline-schedule-modal',
     deleteConfirmation: s__(
-      'PipelineSchedules|Are you sure you want to delete this pipeline schedule?',
+      'PipelineSchedules|Are you sure you want to delete this scheduled pipeline?',
     ),
     actionPrimary: {
-      text: s__('PipelineSchedules|Delete pipeline schedule'),
+      text: s__('PipelineSchedules|Delete scheduled pipeline'),
       attributes: { variant: 'danger' },
     },
     actionCancel: {
diff --git a/app/views/projects/pipeline_schedules/edit.html.haml b/app/views/projects/pipeline_schedules/edit.html.haml
index 89330b6ce25148..a697f2b9a51b16 100644
--- a/app/views/projects/pipeline_schedules/edit.html.haml
+++ b/app/views/projects/pipeline_schedules/edit.html.haml
@@ -3,6 +3,6 @@
 - page_title _("Edit"), @schedule.description, _("Pipeline Schedule")
 
 %h1.page-title.gl-text-size-h-display
-  = _("Edit Pipeline Schedule")
+  = _("Edit Scheduled Pipeline")
 
 #pipeline-schedules-form-edit{ data: js_pipeline_schedules_form_data(@project, @schedule) }
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index d7b6c9a5caa8f8..17d772005bb5de 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -20656,10 +20656,10 @@ msgstr ""
 msgid "Edit Password"
 msgstr ""
 
-msgid "Edit Pipeline Schedule"
+msgid "Edit Release"
 msgstr ""
 
-msgid "Edit Release"
+msgid "Edit Scheduled Pipeline"
 msgstr ""
 
 msgid "Edit Slack integration"
@@ -40190,7 +40190,7 @@ msgstr ""
 msgid "PipelineSchedules|An error occurred while updating the pipeline schedule."
 msgstr ""
 
-msgid "PipelineSchedules|Are you sure you want to delete this pipeline schedule?"
+msgid "PipelineSchedules|Are you sure you want to delete this scheduled pipeline?"
 msgstr ""
 
 msgid "PipelineSchedules|Can have custom CI/CD variables."
@@ -40205,9 +40205,6 @@ msgstr ""
 msgid "PipelineSchedules|Cron timezone"
 msgstr ""
 
-msgid "PipelineSchedules|Delete pipeline schedule"
-msgstr ""
-
 msgid "PipelineSchedules|Delete scheduled pipeline"
 msgstr ""
 
diff --git a/spec/features/projects/pipeline_schedules_spec.rb b/spec/features/projects/pipeline_schedules_spec.rb
index f714b7fa773c23..62fba30dd765d7 100644
--- a/spec/features/projects/pipeline_schedules_spec.rb
+++ b/spec/features/projects/pipeline_schedules_spec.rb
@@ -27,7 +27,7 @@
       it 'edits the pipeline' do
         find_by_testid('edit-pipeline-schedule-btn').click
 
-        expect(page).to have_content(s_('PipelineSchedules|Edit Pipeline Schedule'))
+        expect(page).to have_content(s_('PipelineSchedules|Edit Scheduled Pipeline'))
         expect(page).to have_button(s_('PipelineSchedules|Save changes'))
       end
 
@@ -147,10 +147,10 @@
 
         it 'deletes the pipeline' do
           within_testid('pipeline-schedule-table-row') do
-            click_button s_('PipelineSchedules|Delete pipeline schedule')
+            click_button s_('PipelineSchedules|Delete scheduled pipeline')
           end
 
-          accept_gl_confirm(button_text: s_('PipelineSchedules|Delete pipeline schedule'))
+          accept_gl_confirm(button_text: s_('PipelineSchedules|Delete scheduled pipeline'))
 
           expect(page).not_to have_css('[data-testid="pipeline-schedule-table-row"]')
         end
-- 
GitLab


From 32526e1c1dd1957b090aad6f730efa63822d7495 Mon Sep 17 00:00:00 2001
From: Briley Sandlin <bsandlin@gitlab.com>
Date: Mon, 28 Oct 2024 09:58:46 -0500
Subject: [PATCH 5/6] MR feedback - AF

---
 .../components/ci_variable_drawer_spec.js            |  2 +-
 .../pipeline_failed_jobs_widget_spec.js              | 12 +++---------
 .../components/merge_checks_spec.js                  | 12 ++++++------
 3 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/spec/frontend/ci/ci_variable_list/components/ci_variable_drawer_spec.js b/spec/frontend/ci/ci_variable_list/components/ci_variable_drawer_spec.js
index de332d56476c68..73c484a3e1cba4 100644
--- a/spec/frontend/ci/ci_variable_list/components/ci_variable_drawer_spec.js
+++ b/spec/frontend/ci/ci_variable_list/components/ci_variable_drawer_spec.js
@@ -254,7 +254,7 @@ describe('CI Variable Drawer', () => {
           },
         });
 
-        expect(findVisibilityRadioGroup().attributes('disabled')).toBe('true');
+        expect(findVisibilityRadioGroup().attributes().disabled).toBe('true');
       });
     });
 
diff --git a/spec/frontend/ci/pipelines_page/components/failure_widget/pipeline_failed_jobs_widget_spec.js b/spec/frontend/ci/pipelines_page/components/failure_widget/pipeline_failed_jobs_widget_spec.js
index d9cebe143bc72b..a764143b00267c 100644
--- a/spec/frontend/ci/pipelines_page/components/failure_widget/pipeline_failed_jobs_widget_spec.js
+++ b/spec/frontend/ci/pipelines_page/components/failure_widget/pipeline_failed_jobs_widget_spec.js
@@ -41,22 +41,16 @@ describe('PipelineFailedJobsWidget component', () => {
   const findCount = () => wrapper.findComponent(GlBadge);
 
   describe('when there are failed jobs', () => {
-    it('renders the show failed jobs button with correct count', () => {
+    beforeEach(() => {
       createComponent();
+    });
 
+    it('renders the show failed jobs button with correct count', () => {
       expect(findFailedJobsButton().exists()).toBe(true);
       expect(findCount().text()).toBe(String(defaultProps.failedJobsCount));
     });
 
-    it('renders the maximum job count label when count is > 100', () => {
-      createComponent({ props: { failedJobsCount: 102 } });
-
-      expect(findFailedJobsButton().text()).toContain('(100+)');
-    });
-
     it('does not render the failed jobs widget', () => {
-      createComponent();
-
       expect(findFailedJobsList().exists()).toBe(false);
     });
   });
diff --git a/spec/frontend/vue_merge_request_widget/components/merge_checks_spec.js b/spec/frontend/vue_merge_request_widget/components/merge_checks_spec.js
index 083b23ddef59a7..07d78ba01065ef 100644
--- a/spec/frontend/vue_merge_request_widget/components/merge_checks_spec.js
+++ b/spec/frontend/vue_merge_request_widget/components/merge_checks_spec.js
@@ -189,13 +189,13 @@ describe('Merge request merge checks component', () => {
 
     await wrapper.findByTestId('widget-toggle').trigger('click');
 
-    const mergeChecks = wrapper.findAllByTestId('merge-check');
+    await waitForPromises();
+
+    const mergeChecks = wrapper.findAllComponentsByTestId('merge-check');
 
     expect(mergeChecks.length).toBe(2);
-    expect(mergeChecks.at(0).props('check')).toEqual(expect.objectContaining({ status: 'FAILED' }));
-    expect(mergeChecks.at(1).props('check')).toEqual(
-      expect.objectContaining({ status: 'SUCCESS' }),
-    );
+    expect(mergeChecks.at(0).html()).toContain('Failed');
+    expect(mergeChecks.at(1).html()).toContain('Success');
   });
 
   it('does not render check component if no message exists', async () => {
@@ -266,7 +266,7 @@ describe('Merge request merge checks component', () => {
     });
 
     it('renders checking text', () => {
-      expect(wrapper.text()).toBe('Checking if merge request can be merged...');
+      expect(wrapper.text()).toContain('Checking if merge request can be merged...');
     });
 
     it('renders checks expanded by default', () => {
-- 
GitLab


From f3d9944c3ef02855340cecd6f0c04c571e7c4859 Mon Sep 17 00:00:00 2001
From: Briley Sandlin <bsandlin@gitlab.com>
Date: Wed, 30 Oct 2024 18:02:23 -0500
Subject: [PATCH 6/6] fix vue3 tests

---
 .../components/ci_variable_shared.vue            |  2 +-
 scripts/frontend/quarantined_vue3_specs.txt      |  3 ---
 .../components/merge_checks_spec.js              | 16 +++++++---------
 3 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_shared.vue b/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_shared.vue
index 9d4ba4fef2a51d..727cb47d37a536 100644
--- a/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_shared.vue
+++ b/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_shared.vue
@@ -99,6 +99,7 @@ export default {
   data() {
     return {
       ciVariables: [],
+      environments: [],
       hasNextPage: false,
       isInitialLoading: true,
       isLoadingMoreItems: false,
@@ -158,7 +159,6 @@ export default {
         }
       },
     },
-    // eslint-disable-next-line @gitlab/vue-no-undef-apollo-properties
     environments: {
       query() {
         return this.queryData?.environments?.query || {};
diff --git a/scripts/frontend/quarantined_vue3_specs.txt b/scripts/frontend/quarantined_vue3_specs.txt
index a33d6be8b5e914..74ab9e39605899 100644
--- a/scripts/frontend/quarantined_vue3_specs.txt
+++ b/scripts/frontend/quarantined_vue3_specs.txt
@@ -204,8 +204,6 @@ spec/frontend/ci/catalog/components/ci_catalog_home_spec.js
 spec/frontend/ci/catalog/components/list/ci_resources_list_item_spec.js
 spec/frontend/ci/catalog/components/pages/ci_resource_details_page_spec.js
 spec/frontend/ci/catalog/index_spec.js
-spec/frontend/ci/ci_variable_list/components/ci_variable_drawer_spec.js
-spec/frontend/ci/ci_variable_list/components/ci_variable_shared_spec.js
 spec/frontend/ci/job_details/components/log/line_header_spec.js
 spec/frontend/ci/job_details/components/log/line_spec.js
 spec/frontend/ci/job_details/components/log/log_spec.js
@@ -426,7 +424,6 @@ spec/frontend/usage_quotas/storage/components/namespace_storage_app_spec.js
 spec/frontend/users_select/index_spec.js
 spec/frontend/vue_alerts_spec.js
 spec/frontend/vue_merge_request_widget/components/checks/conflicts_spec.js
-spec/frontend/vue_merge_request_widget/components/merge_checks_spec.js
 spec/frontend/vue_merge_request_widget/components/mr_widget_pipeline_spec.js
 spec/frontend/vue_merge_request_widget/components/states/merge_failed_pipeline_confirmation_dialog_spec.js
 spec/frontend/vue_merge_request_widget/components/states/mr_widget_merging_spec.js
diff --git a/spec/frontend/vue_merge_request_widget/components/merge_checks_spec.js b/spec/frontend/vue_merge_request_widget/components/merge_checks_spec.js
index 07d78ba01065ef..a6ff6a9fb90831 100644
--- a/spec/frontend/vue_merge_request_widget/components/merge_checks_spec.js
+++ b/spec/frontend/vue_merge_request_widget/components/merge_checks_spec.js
@@ -177,7 +177,7 @@ describe('Merge request merge checks component', () => {
   });
 
   it('sorts merge checks', async () => {
-    mountComponent({
+    shallowMountComponent({
       mergeabilityChecks: [
         { identifier: 'discussions_not_resolved', status: 'SUCCESS' },
         { identifier: 'status_checks_must_pass', status: 'INACTIVE' },
@@ -187,15 +187,11 @@ describe('Merge request merge checks component', () => {
 
     await waitForPromises();
 
-    await wrapper.findByTestId('widget-toggle').trigger('click');
-
-    await waitForPromises();
+    wrapper.vm.toggleCollapsed();
 
-    const mergeChecks = wrapper.findAllComponentsByTestId('merge-check');
-
-    expect(mergeChecks.length).toBe(2);
-    expect(mergeChecks.at(0).html()).toContain('Failed');
-    expect(mergeChecks.at(1).html()).toContain('Success');
+    expect(wrapper.vm.sortedChecks.length).toBe(2);
+    expect(wrapper.vm.sortedChecks[0].status).toBe('FAILED');
+    expect(wrapper.vm.sortedChecks[1].status).toBe('SUCCESS');
   });
 
   it('does not render check component if no message exists', async () => {
@@ -210,6 +206,8 @@ describe('Merge request merge checks component', () => {
 
     await wrapper.findByTestId('widget-toggle').trigger('click');
 
+    await waitForPromises();
+
     const mergeChecks = wrapper.findAllByTestId('merge-check');
 
     expect(mergeChecks.length).toBe(1);
-- 
GitLab