diff options
45 files changed, 383 insertions, 321 deletions
diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index d928c645a6b..ac4763103b2 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -298,7 +298,6 @@ rules: - <<: *if-not-canonical-namespace when: never - - <<: *if-master-refs - changes: *ci-build-images-patterns - changes: *code-qa-patterns diff --git a/app/assets/javascripts/diffs/components/settings_dropdown.vue b/app/assets/javascripts/diffs/components/settings_dropdown.vue index acba0c5c8af..b1f8e583c1f 100644 --- a/app/assets/javascripts/diffs/components/settings_dropdown.vue +++ b/app/assets/javascripts/diffs/components/settings_dropdown.vue @@ -26,6 +26,9 @@ export default { toggleFileByFile() { this.setFileByFile({ fileByFile: !this.viewDiffsFileByFile }); }, + toggleWhitespace(updatedSetting) { + this.setShowWhitespace({ showWhitespace: updatedSetting, pushState: true }); + }, }, }; </script> @@ -80,26 +83,21 @@ export default { </gl-button> </gl-button-group> </div> - <div class="gl-mt-3 gl-px-3"> - <label class="gl-mb-0"> - <input - id="show-whitespace" - type="checkbox" - :checked="showWhitespace" - @change="setShowWhitespace({ showWhitespace: $event.target.checked, pushState: true })" - /> - {{ __('Show whitespace changes') }} - </label> - </div> - <div class="gl-mt-3 gl-px-3"> - <gl-form-checkbox - data-testid="file-by-file" - class="gl-mb-0" - :checked="viewDiffsFileByFile" - @input="toggleFileByFile" - > - {{ $options.i18n.fileByFile }} - </gl-form-checkbox> - </div> + <gl-form-checkbox + data-testid="show-whitespace" + class="gl-mt-3 gl-pl-3" + :checked="showWhitespace" + @input="toggleWhitespace" + > + {{ $options.i18n.whitespace }} + </gl-form-checkbox> + <gl-form-checkbox + data-testid="file-by-file" + class="gl-pl-3 gl-mb-0" + :checked="viewDiffsFileByFile" + @input="toggleFileByFile" + > + {{ $options.i18n.fileByFile }} + </gl-form-checkbox> </gl-dropdown> </template> diff --git a/app/assets/javascripts/diffs/i18n.js b/app/assets/javascripts/diffs/i18n.js index 2a061876937..b2354af1eec 100644 --- a/app/assets/javascripts/diffs/i18n.js +++ b/app/assets/javascripts/diffs/i18n.js @@ -21,5 +21,6 @@ export const DIFF_FILE = { }; export const SETTINGS_DROPDOWN = { + whitespace: __('Show whitespace changes'), fileByFile: __('Show one file at a time'), }; diff --git a/app/assets/javascripts/issue.js b/app/assets/javascripts/issue.js index 3f25682ab8b..8922c53564b 100644 --- a/app/assets/javascripts/issue.js +++ b/app/assets/javascripts/issue.js @@ -6,6 +6,9 @@ import axios from './lib/utils/axios_utils'; import { addDelimiter } from './lib/utils/text_utility'; import { __ } from './locale'; +// TODO: Update all references of "issuable_vue_app:change" https://gitlab.com/gitlab-org/gitlab/-/issues/322760 +export const EVENT_ISSUABLE_VUE_APP_CHANGE = 'issuable_vue_app:change'; + export default class Issue { constructor() { if ($('.js-alert-moved-from-service-desk-warning').length) { @@ -23,9 +26,13 @@ export default class Issue { } // Listen to state changes in the Vue app - document.addEventListener('issuable_vue_app:change', (event) => { + this.issuableVueAppChangeHandler = (event) => this.updateTopState(event.detail.isClosed, event.detail.data); - }); + document.addEventListener(EVENT_ISSUABLE_VUE_APP_CHANGE, this.issuableVueAppChangeHandler); + } + + dispose() { + document.removeEventListener(EVENT_ISSUABLE_VUE_APP_CHANGE, this.issuableVueAppChangeHandler); } /** diff --git a/app/assets/javascripts/pages/projects/compare/show/index.js b/app/assets/javascripts/pages/projects/compare/show/index.js index f1cf9caa28b..549e596cb8d 100644 --- a/app/assets/javascripts/pages/projects/compare/show/index.js +++ b/app/assets/javascripts/pages/projects/compare/show/index.js @@ -1,6 +1,9 @@ import Diff from '~/diff'; import GpgBadges from '~/gpg_badges'; import initChangesDropdown from '~/init_changes_dropdown'; +import initCompareSelector from '~/projects/compare'; + +initCompareSelector(); document.addEventListener('DOMContentLoaded', () => { new Diff(); // eslint-disable-line no-new diff --git a/app/assets/javascripts/pipelines/components/pipelines_list/pipelines_actions.vue b/app/assets/javascripts/pipelines/components/pipelines_list/pipelines_actions.vue index 6890cbb9bed..b94f1a42039 100644 --- a/app/assets/javascripts/pipelines/components/pipelines_list/pipelines_actions.vue +++ b/app/assets/javascripts/pipelines/components/pipelines_list/pipelines_actions.vue @@ -82,6 +82,7 @@ export default { :loading="isLoading" data-testid="pipelines-manual-actions-dropdown" right + lazy icon="play" > <gl-dropdown-item diff --git a/app/assets/javascripts/pipelines/components/pipelines_list/pipelines_artifacts.vue b/app/assets/javascripts/pipelines/components/pipelines_list/pipelines_artifacts.vue index b13460b4c68..9c3990f82df 100644 --- a/app/assets/javascripts/pipelines/components/pipelines_list/pipelines_artifacts.vue +++ b/app/assets/javascripts/pipelines/components/pipelines_list/pipelines_artifacts.vue @@ -31,6 +31,8 @@ export default { :text="$options.translations.artifacts" :aria-label="$options.translations.artifacts" icon="download" + right + lazy text-sr-only > <gl-dropdown-item diff --git a/app/assets/javascripts/projects/compare/components/revision_dropdown.vue b/app/assets/javascripts/projects/compare/components/revision_dropdown.vue index d8947d60a8e..a175af2f32e 100644 --- a/app/assets/javascripts/projects/compare/components/revision_dropdown.vue +++ b/app/assets/javascripts/projects/compare/components/revision_dropdown.vue @@ -69,7 +69,7 @@ export default { this.loading = true; if (reset) { - this.selectedRevision = emptyDropdownText; + this.selectedRevision = this.getDefaultBranch(); } return axios diff --git a/app/assets/javascripts/related_issues/components/related_issuable_input.vue b/app/assets/javascripts/related_issues/components/related_issuable_input.vue index 2dc56c3110b..46b97370d66 100644 --- a/app/assets/javascripts/related_issues/components/related_issuable_input.vue +++ b/app/assets/javascripts/related_issues/components/related_issuable_input.vue @@ -223,6 +223,7 @@ export default { type="text" class="js-add-issuable-form-input add-issuable-form-input" data-qa-selector="add_issue_field" + autocomplete="off" @input="onInput" @focus="onFocus" @blur="onBlur" diff --git a/app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/dropdown_value.vue b/app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/dropdown_value.vue index f173c8db540..46ccb9470e5 100644 --- a/app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/dropdown_value.vue +++ b/app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/dropdown_value.vue @@ -21,11 +21,14 @@ export default { 'allowLabelRemove', 'allowScopedLabels', 'labelsFilterBasePath', + 'labelsFilterParam', ]), }, methods: { labelFilterUrl(label) { - return `${this.labelsFilterBasePath}?label_name[]=${encodeURIComponent(label.title)}`; + return `${this.labelsFilterBasePath}?${this.labelsFilterParam}[]=${encodeURIComponent( + label.title, + )}`; }, scopedLabel(label) { return this.allowScopedLabels && isScopedLabel(label); diff --git a/app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/labels_select_root.vue b/app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/labels_select_root.vue index 93fdae19a8d..426ae430ce7 100644 --- a/app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/labels_select_root.vue +++ b/app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/labels_select_root.vue @@ -81,6 +81,11 @@ export default { required: false, default: '', }, + labelsFilterParam: { + type: String, + required: false, + default: 'label_name', + }, dropdownButtonText: { type: String, required: false, @@ -156,6 +161,7 @@ export default { labelsFetchPath: this.labelsFetchPath, labelsManagePath: this.labelsManagePath, labelsFilterBasePath: this.labelsFilterBasePath, + labelsFilterParam: this.labelsFilterParam, labelsListTitle: this.labelsListTitle, labelsCreateTitle: this.labelsCreateTitle, footerCreateLabelTitle: this.footerCreateLabelTitle, diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb index eecd338b7cc..81f80d37662 100644 --- a/app/controllers/projects/compare_controller.rb +++ b/app/controllers/projects/compare_controller.rb @@ -21,7 +21,7 @@ class Projects::CompareController < Projects::ApplicationController before_action :validate_refs! before_action do - push_frontend_feature_flag(:compare_repo_dropdown) + push_frontend_feature_flag(:compare_repo_dropdown, source_project, default_enabled: :yaml) end feature_category :source_code_management diff --git a/app/helpers/compare_helper.rb b/app/helpers/compare_helper.rb index c14aa44ced1..b07baf59114 100644 --- a/app/helpers/compare_helper.rb +++ b/app/helpers/compare_helper.rb @@ -28,4 +28,20 @@ module CompareHelper .new(current_user: current_user, source_project: source_project, project_feature: :repository) .execute(include_routes: true) end + + def project_compare_selector_data(project, merge_request, params) + { + project_compare_index_path: project_compare_index_path(project), + refs_project_path: refs_project_path(project), + params_from: params[:from], + params_to: params[:to], + project_merge_request_path: merge_request.present? ? project_merge_request_path(project, merge_request) : '', + create_mr_path: create_mr_button? ? create_mr_path : '' + }.tap do |data| + if Feature.enabled?(:compare_repo_dropdown, project, default_enabled: :yaml) + data[:project_to] = { id: project.id, name: project.full_path }.to_json + data[:projects_from] = target_projects(project).map { |project| { id: project.id, name: project.full_path } }.to_json + end + end + end end diff --git a/app/views/projects/compare/_form.html.haml b/app/views/projects/compare/_form.html.haml deleted file mode 100644 index 17134613b17..00000000000 --- a/app/views/projects/compare/_form.html.haml +++ /dev/null @@ -1,28 +0,0 @@ -= form_tag project_compare_index_path(@project), method: :post, class: 'form-inline js-requires-input js-signature-container', data: { 'signatures-path' => signatures_namespace_project_compare_index_path } do - .form-group.dropdown.compare-form-group.to.js-compare-to-dropdown - .input-group.inline-input-group - %span.input-group-prepend - .input-group-text - = s_("CompareBranches|Source") - = hidden_field_tag :to, params[:to] - = button_tag type: 'button', title: params[:to], class: "btn gl-button form-control compare-dropdown-toggle js-compare-dropdown has-tooltip", required: true, data: { refs_url: refs_project_path(@project), toggle: "dropdown", target: ".js-compare-to-dropdown", selected: params[:to], field_name: :to } do - .dropdown-toggle-text.str-truncated.monospace.float-left= params[:to] || _("Select branch/tag") - = sprite_icon('chevron-down', css_class: 'float-right') - = render 'shared/ref_dropdown' - .compare-ellipsis.inline ... - .form-group.dropdown.compare-form-group.from.js-compare-from-dropdown - .input-group.inline-input-group - %span.input-group-prepend - .input-group-text - = s_("CompareBranches|Target") - = hidden_field_tag :from, params[:from] - = button_tag type: 'button', title: params[:from], class: "btn gl-button form-control compare-dropdown-toggle js-compare-dropdown has-tooltip", required: true, data: { refs_url: refs_project_path(@project), toggle: "dropdown", target: ".js-compare-from-dropdown", selected: params[:from], field_name: :from } do - .dropdown-toggle-text.str-truncated.monospace.float-left= params[:from] || _("Select branch/tag") - = sprite_icon('chevron-down', css_class: 'float-right') - = render 'shared/ref_dropdown' - - = button_tag s_("CompareBranches|Compare"), class: "btn gl-button btn-success commits-compare-btn" - - if @merge_request.present? - = link_to _("View open merge request"), project_merge_request_path(@project, @merge_request), class: 'gl-ml-3 btn' - - elsif create_mr_button? - = link_to _("Create merge request"), create_mr_path, class: 'gl-ml-3 btn gl-button' diff --git a/app/views/projects/compare/index.html.haml b/app/views/projects/compare/index.html.haml index 39d4a3b2eb2..e3ab184ec6f 100644 --- a/app/views/projects/compare/index.html.haml +++ b/app/views/projects/compare/index.html.haml @@ -13,17 +13,4 @@ = html_escape(_("Changes are shown as if the %{b_open}source%{b_close} revision was being merged into the %{b_open}target%{b_close} revision.")) % { b_open: '<b>'.html_safe, b_close: '</b>'.html_safe } .prepend-top-20 - - if Feature.enabled?(:compare_repo_dropdown) - #js-compare-selector{ data: { project_compare_index_path: project_compare_index_path(@project), - refs_project_path: refs_project_path(@project), - params_from: params[:from], params_to: params[:to], - project_merge_request_path: @merge_request.present? ? project_merge_request_path(@project, @merge_request) : '', - create_mr_path: create_mr_button? ? create_mr_path : '', - project_to: { id: @project.id, name: @project.full_path }.to_json, - projects_from: target_projects(@project).map { |project| { id:project.id, name: project.full_path } }.to_json } } - - else - #js-compare-selector{ data: { project_compare_index_path: project_compare_index_path(@project), - refs_project_path: refs_project_path(@project), - params_from: params[:from], params_to: params[:to], - project_merge_request_path: @merge_request.present? ? project_merge_request_path(@project, @merge_request) : '', - create_mr_path: create_mr_button? ? create_mr_path : '' } } + #js-compare-selector{ data: project_compare_selector_data(@project, @merge_request, params) } diff --git a/app/views/projects/compare/show.html.haml b/app/views/projects/compare/show.html.haml index 51cf95dc84b..9e9c271e7be 100644 --- a/app/views/projects/compare/show.html.haml +++ b/app/views/projects/compare/show.html.haml @@ -2,7 +2,8 @@ - page_title "#{params[:from]}...#{params[:to]}" .sub-header-block.no-bottom-space - = render "form" + .js-signature-container{ data: { 'signatures-path' => signatures_namespace_project_compare_index_path } } + #js-compare-selector{ data: project_compare_selector_data(@project, @merge_request, params) } - if @commits.present? = render "projects/commits/commit_list" diff --git a/changelogs/unreleased/300853-convert-compare-show-to-vue.yml b/changelogs/unreleased/300853-convert-compare-show-to-vue.yml new file mode 100644 index 00000000000..2926ef2682b --- /dev/null +++ b/changelogs/unreleased/300853-convert-compare-show-to-vue.yml @@ -0,0 +1,5 @@ +--- +title: Restyle the repository compare show page +merge_request: 53523 +author: +type: changed diff --git a/changelogs/unreleased/321845-performance-pipeline-table-dropdowns.yml b/changelogs/unreleased/321845-performance-pipeline-table-dropdowns.yml new file mode 100644 index 00000000000..0fac7a05f7a --- /dev/null +++ b/changelogs/unreleased/321845-performance-pipeline-table-dropdowns.yml @@ -0,0 +1,5 @@ +--- +title: Reduce elements in Pipeline page dropdowns with lazy +merge_request: 54674 +author: +type: changed diff --git a/changelogs/unreleased/bugfix-linked-issue-autocomplete.yml b/changelogs/unreleased/bugfix-linked-issue-autocomplete.yml new file mode 100644 index 00000000000..20b36d33bbd --- /dev/null +++ b/changelogs/unreleased/bugfix-linked-issue-autocomplete.yml @@ -0,0 +1,5 @@ +--- +title: Improve Linked Issues Usability +merge_request: 50879 +author: Andrew Minion +type: changed diff --git a/changelogs/unreleased/philipcunningham-add-branch-to-dast-profile-322526.yml b/changelogs/unreleased/philipcunningham-add-branch-to-dast-profile-322526.yml new file mode 100644 index 00000000000..4ad204b7cd1 --- /dev/null +++ b/changelogs/unreleased/philipcunningham-add-branch-to-dast-profile-322526.yml @@ -0,0 +1,5 @@ +--- +title: Add branch_name to dast_profiles table +merge_request: 54891 +author: +type: added diff --git a/changelogs/unreleased/tor-defect-out-of-place-bold-label-in-diffs-dropdown-menu.yml b/changelogs/unreleased/tor-defect-out-of-place-bold-label-in-diffs-dropdown-menu.yml new file mode 100644 index 00000000000..8525204ee7e --- /dev/null +++ b/changelogs/unreleased/tor-defect-out-of-place-bold-label-in-diffs-dropdown-menu.yml @@ -0,0 +1,5 @@ +--- +title: Fix bold text mismatch in MR âš™ menu +merge_request: 54531 +author: +type: fixed diff --git a/db/migrate/20210223053451_add_branch_name_to_dast_profile.rb b/db/migrate/20210223053451_add_branch_name_to_dast_profile.rb new file mode 100644 index 00000000000..311e809103f --- /dev/null +++ b/db/migrate/20210223053451_add_branch_name_to_dast_profile.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class AddBranchNameToDastProfile < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + with_lock_retries do + add_column :dast_profiles, :branch_name, :text + end + + add_text_limit :dast_profiles, :branch_name, 255 + end + + def down + with_lock_retries do + remove_column :dast_profiles, :branch_name + end + end +end diff --git a/db/schema_migrations/20210223053451 b/db/schema_migrations/20210223053451 new file mode 100644 index 00000000000..ad40bb0fa71 --- /dev/null +++ b/db/schema_migrations/20210223053451 @@ -0,0 +1 @@ +1266bf92f23a42d96778bf546534882f03d2388f22640e4cfaa2a9a1aad19093
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index b829666c1d3..c3b46a15098 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -11634,7 +11634,9 @@ CREATE TABLE dast_profiles ( updated_at timestamp with time zone NOT NULL, name text NOT NULL, description text NOT NULL, + branch_name text, CONSTRAINT check_5fcf73bf61 CHECK ((char_length(name) <= 255)), + CONSTRAINT check_6c9d775949 CHECK ((char_length(branch_name) <= 255)), CONSTRAINT check_c34e505c24 CHECK ((char_length(description) <= 255)) ); diff --git a/doc/development/README.md b/doc/development/README.md index 3d5335feb11..c25782dbf84 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -287,6 +287,7 @@ See [database guidelines](database/index.md). ## Domain-specific guides - [CI/CD development documentation](cicd/index.md) +- [AppSec development documentation](appsec/index.md) ## Other Development guides diff --git a/doc/development/appsec/index.md b/doc/development/appsec/index.md new file mode 100644 index 00000000000..e8ce885e75d --- /dev/null +++ b/doc/development/appsec/index.md @@ -0,0 +1,32 @@ +--- +stage: Secure, Protect +group: all +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +type: index, dev, reference +--- + +# Application Security development documentation + +Development guides that are specific to the stages that work on Application Security features are listed here. + +Please go to [Application Security](../../user/application_security/index.md) if you are looking for documentation on how to use those features. + +## Namespaces + +Application Security code in the Rails monolith is organized into the following namespaces, which generally follows +the feature categories in the [Secure](https://about.gitlab.com/stages-devops-lifecycle/secure/) and [Protect](https://about.gitlab.com/stages-devops-lifecycle/protect/) stages. + +- `AppSec`: shared code. + - `AppSec::ContainerScanning`: Container Scanning code. + - `AppSec::Dast`: DAST code. + - `AppSec::DependencyScanning`: Dependency Scanning code. + - `AppSec::Fuzzing::Api`: API Fuzzing code. + - `AppSec::Fuzzing::Coverage`: Coverage Fuzzing code. + - `AppSec::Fuzzing`: Shared fuzzing code. + - `AppSec::LicenseCompliance`: License Compliance code. + - `AppSec::Sast`: SAST code. + - `AppSec::SecretDetection`: Secret Detection code. + - `AppSec::VulnMgmt`: Vulnerability Management code. + +Most AppSec code does not conform to these namespace guidelines. When developing, make an effort +to move existing code into the appropriate namespace whenever possible. diff --git a/doc/development/database/index.md b/doc/development/database/index.md index 367ef455898..870ae1542bd 100644 --- a/doc/development/database/index.md +++ b/doc/development/database/index.md @@ -69,3 +69,4 @@ info: To determine the technical writer assigned to the Stage/Group associated w ## Miscellaneous - [Maintenance operations](maintenance_operations.md) +- [Update multiple database objects](setting_multiple_values.md) diff --git a/doc/development/database/setting_multiple_values.md b/doc/development/database/setting_multiple_values.md index 54870380047..0f23aae9f79 100644 --- a/doc/development/database/setting_multiple_values.md +++ b/doc/development/database/setting_multiple_values.md @@ -4,24 +4,22 @@ group: Database info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- -# Setting Multiple Values +# Update multiple database objects > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/32921) in GitLab 13.5. -There's often a need to update multiple objects with new values for one -or more columns. One method of doing this is using `Relation#update_all`: +You can update multiple database objects with new values for one or more columns. +One method is to use `Relation#update_all`: ```ruby user.issues.open.update_all(due_date: 7.days.from_now) # (1) user.issues.update_all('relative_position = relative_position + 1') # (2) ``` -But what do you do if you cannot express the update as either a static value (1) -or as a calculation (2)? - -Thankfully we can use `UPDATE FROM` to express the need to update multiple rows -with distinct values in a single query. One can either use a temporary table, or -a Common Table Expression (CTE), and then use that as the source of the updates: +If you cannot express the update as either a static value (1) or as a calculation (2), +use `UPDATE FROM` to express the need to update multiple rows with distinct values +in a single query. Create a temporary table, or a Common Table Expression (CTE), +and use it as the source of the updates: ```sql with updates(obj_id, new_title, new_weight) as ( @@ -34,23 +32,22 @@ update issues where id = obj_id ``` -The bad news: there is no way to express this in ActiveRecord or even dropping -down to ARel. The `UpdateManager` does not support `update from`, so this -is not expressible. - -The good news: we supply an abstraction to help you generate these kinds of -updates, called `Gitlab::Database::BulkUpdate`. This constructs queries such as the -above, and uses binding parameters to avoid SQL injection. +You can't express this in ActiveRecord, or by dropping down to [Arel](https://api.rubyonrails.org/v6.1.0/classes/Arel.html), +because the `UpdateManager` does not support `update from`. However, we supply +an abstraction to help you generate these kinds of updates: `Gitlab::Database::BulkUpdate`. +This abstraction constructs queries like the previous example, and uses +binding parameters to avoid SQL injection. ## Usage -To use this, we need: +To use `Gitlab::Database::BulkUpdate`, we need: -- the list of columns to update -- a mapping from object/ID to the new values to set for that object -- a way to determine the table for each object +- The list of columns to update. +- A mapping from the object (or ID) to the new values to set for that object. +- A way to determine the table for each object. -For example, we can express the query above as: +For example, we can express the example query in a way that determines the +table by calling `object.class.table_name`: ```ruby issue_a = Issue.find(..) @@ -63,10 +60,7 @@ issue_b = Issue.find(..) }) ``` -Here the table can be determined automatically, from calling -`object.class.table_name`, so we don't need to provide anything. - -We can even pass heterogeneous sets of objects, if the updates all make sense +You can even pass heterogeneous sets of objects, if the updates all make sense for them: ```ruby @@ -82,8 +76,8 @@ merge_request = MergeRequest.find(..) }) ``` -If your objects do not return the correct model class (perhaps because they are -part of a union), then we need to specify this explicitly in a block: +If your objects do not return the correct model class, such as if they are part +of a union, then specify the model class explicitly in a block: ```ruby bazzes = params @@ -103,7 +97,10 @@ end ## Caveats -Note that this is a **very low level** tool, and operates on the raw column -values. Enumerations and state fields must be translated into their underlying -representations, for example, and nested associations are not supported. No -validations or hooks are called. +This tool is **very low level**, and operates directly on the raw column +values. You should consider these issues if you implement it: + +- Enumerations and state fields must be translated into their underlying + representations. +- Nested associations are not supported. +- No validations or hooks are called. diff --git a/doc/development/fe_guide/vuex.md b/doc/development/fe_guide/vuex.md index cc1d9ccab77..d44ab64ae5d 100644 --- a/doc/development/fe_guide/vuex.md +++ b/doc/development/fe_guide/vuex.md @@ -440,12 +440,11 @@ components, we need to include the store and provide the correct state: //component_spec.js import Vue from 'vue'; import Vuex from 'vuex'; -import { mount, createLocalVue } from '@vue/test-utils'; +import { mount } from '@vue/test-utils'; import { createStore } from './store'; import Component from './component.vue' -const localVue = createLocalVue(); -localVue.use(Vuex); +Vue.use(Vuex); describe('component', () => { let store; @@ -455,7 +454,6 @@ describe('component', () => { store = createStore(); wrapper = mount(Component, { - localVue, store, }); }; @@ -483,6 +481,11 @@ describe('component', () => { }); ``` +Some test files may still use the +[deprecated `createLocalVue` function](https://gitlab.com/gitlab-org/gitlab/-/issues/220482) +from `@vue/test-utils` and `localVue.use(Vuex)`. This is unnecessary, and should be +avoided or removed when possible. + ### Two way data binding When storing form data in Vuex, it is sometimes necessary to update the value stored. The store diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md index e1205346585..759b19db36f 100644 --- a/doc/development/migration_style_guide.md +++ b/doc/development/migration_style_guide.md @@ -605,9 +605,10 @@ When adding a foreign-key constraint to an existing column in a non-empty table, we have to employ `add_concurrent_foreign_key` and `add_concurrent_index` instead of `add_reference`. -For an empty table (such as a fresh one), it is recommended to use -`add_reference` in a single-transaction migration, combining it with other -operations that don't require `disable_ddl_transaction!`. +If you have a new or empty table that doesn't reference a +[high-traffic table](https://gitlab.com/gitlab-org/gitlab/-/blob/master/rubocop/rubocop-migrations.yml#L3), +we recommend that you use `add_reference` in a single-transaction migration. You can +combine it with other operations that don't require `disable_ddl_transaction!`. You can read more about adding [foreign key constraints to an existing column](database/add_foreign_key_to_existing_column.md). diff --git a/doc/user/project/merge_requests/img/comment-on-any-diff-line.png b/doc/user/project/merge_requests/img/comment-on-any-diff-line.png Binary files differdeleted file mode 100644 index cff5acb98ef..00000000000 --- a/doc/user/project/merge_requests/img/comment-on-any-diff-line.png +++ /dev/null diff --git a/doc/user/project/merge_requests/img/comment-on-any-diff-line_v13_10.png b/doc/user/project/merge_requests/img/comment-on-any-diff-line_v13_10.png Binary files differnew file mode 100644 index 00000000000..a31fea85be9 --- /dev/null +++ b/doc/user/project/merge_requests/img/comment-on-any-diff-line_v13_10.png diff --git a/doc/user/project/merge_requests/reviewing_and_managing_merge_requests.md b/doc/user/project/merge_requests/reviewing_and_managing_merge_requests.md index d7f7c3da000..406a79217d0 100644 --- a/doc/user/project/merge_requests/reviewing_and_managing_merge_requests.md +++ b/doc/user/project/merge_requests/reviewing_and_managing_merge_requests.md @@ -187,27 +187,31 @@ Feature.disable(:local_file_reviews) > [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/13950) in GitLab 11.5. In a merge request, you can leave comments in any part of the file being changed. -In the Merge Request Diff UI, click the **{comment}** **comment** icon in the gutter -to expand the diff lines and leave a comment, just as you would for a changed line. +In the Merge Request Diff UI, you can: -![Comment on any diff file line](img/comment-on-any-diff-line.png) +- **Comment on a single line**: Click the **{comment}** **comment** icon in the + gutter to expand the diff lines and display a comment box. +- [**Comment on multiple lines**](#commenting-on-multiple-lines). ### Commenting on multiple lines > - [Introduced](https://gitlab.com/gitlab-org/ux-research/-/issues/870) in GitLab 13.2. +> - [Added](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/49875) click-and-drag features in GitLab 13.8. > - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/299121) in GitLab 13.9. -GitLab provides a way to select which lines of code a comment refers to. After starting a comment -a dropdown selector is shown to select the first line that this comment refers to. -The last line is the line that the comment icon was initially clicked on. +When commenting on a diff, you can select which lines of code your comment refers +to by either: -New comments default to single line comments by having the first and last lines -the same. Selecting a different starting line turns this into a multiline comment. +![Comment on any diff file line](img/comment-on-any-diff-line_v13_10.png) -![Multiline comment selection highlighted](img/multiline-comment-highlighted.png) +- Clicking and dragging the **{comment}** **comment** icon in the gutter to highlight + lines in the diff. GitLab expands the diff lines and displays a comment box. +- After starting a comment by clicking the **{comment}** **comment** icon in the + gutter, select the first line number your comment refers to in the **Commenting on lines** + select box. New comments default to single-line comments, unless you select + a different starting line. -Once a multiline comment is saved the lines of code pertaining to that comment are listed directly -above it. +Multiline comments display the comment's line numbers above the body of the comment: ![Multiline comment selection displayed above comment](img/multiline-comment-saved.png) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index f1838bc3dc6..e0a624b2113 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -7559,15 +7559,6 @@ msgstr "" msgid "CompareBranches|%{source_branch} and %{target_branch} are the same." msgstr "" -msgid "CompareBranches|Compare" -msgstr "" - -msgid "CompareBranches|Source" -msgstr "" - -msgid "CompareBranches|Target" -msgstr "" - msgid "CompareBranches|There isn't anything to compare." msgstr "" @@ -26722,9 +26713,6 @@ msgstr "" msgid "Select branch" msgstr "" -msgid "Select branch/tag" -msgstr "" - msgid "Select due date" msgstr "" diff --git a/package.json b/package.json index 15dd7546b37..3d370c9259e 100644 --- a/package.json +++ b/package.json @@ -168,7 +168,7 @@ "devDependencies": { "@babel/plugin-transform-modules-commonjs": "^7.10.1", "@gitlab/eslint-plugin": "8.1.0", - "@gitlab/stylelint-config": "^2.2.0", + "@gitlab/stylelint-config": "2.2.0", "@testing-library/dom": "^7.16.2", "@vue/test-utils": "1.1.2", "acorn": "^6.3.0", diff --git a/spec/features/merge_request/user_toggles_whitespace_changes_spec.rb b/spec/features/merge_request/user_toggles_whitespace_changes_spec.rb index 05f4c16ef60..b72ac071ecb 100644 --- a/spec/features/merge_request/user_toggles_whitespace_changes_spec.rb +++ b/spec/features/merge_request/user_toggles_whitespace_changes_spec.rb @@ -21,13 +21,13 @@ RSpec.describe 'Merge request > User toggles whitespace changes', :js do describe 'clicking "Hide whitespace changes" button' do it 'toggles the "Hide whitespace changes" button' do - find('#show-whitespace').click + find('[data-testid="show-whitespace"]').click visit diffs_project_merge_request_path(project, merge_request) find('.js-show-diff-settings').click - expect(find('#show-whitespace')).not_to be_checked + expect(find('[data-testid="show-whitespace"]')).not_to be_checked end end end diff --git a/spec/features/projects/merge_request_button_spec.rb b/spec/features/projects/merge_request_button_spec.rb index 2a486e6e5e1..9547ba8a390 100644 --- a/spec/features/projects/merge_request_button_spec.rb +++ b/spec/features/projects/merge_request_button_spec.rb @@ -24,7 +24,7 @@ RSpec.describe 'Merge Request button' do project.add_developer(user) end - it 'shows Create merge request button' do + it 'shows Create merge request button', :js do href = project_new_merge_request_path( project, merge_request: { @@ -83,7 +83,7 @@ RSpec.describe 'Merge Request button' do end context 'on own fork of project' do - it 'shows Create merge request button' do + it 'shows Create merge request button', :js do href = project_new_merge_request_path( forked_project, merge_request: { @@ -120,7 +120,7 @@ RSpec.describe 'Merge Request button' do let(:fork_url) { project_compare_path(forked_project, from: 'master', to: 'feature') } end - it 'shows the correct merge request button when viewing across forks' do + it 'shows the correct merge request button when viewing across forks', :js do sign_in(user) project.add_developer(user) diff --git a/spec/frontend/diffs/components/settings_dropdown_spec.js b/spec/frontend/diffs/components/settings_dropdown_spec.js index 33f965f47df..feac88cb802 100644 --- a/spec/frontend/diffs/components/settings_dropdown_spec.js +++ b/spec/frontend/diffs/components/settings_dropdown_spec.js @@ -1,79 +1,66 @@ -import { mount, createLocalVue } from '@vue/test-utils'; -import Vuex from 'vuex'; +import { mount } from '@vue/test-utils'; + +import { extendedWrapper } from 'helpers/vue_test_utils_helper'; + import SettingsDropdown from '~/diffs/components/settings_dropdown.vue'; import { PARALLEL_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE } from '~/diffs/constants'; import eventHub from '~/diffs/event_hub'; -import diffModule from '~/diffs/store/modules'; -const localVue = createLocalVue(); -localVue.use(Vuex); +import createDiffsStore from '../create_diffs_store'; describe('Diff settings dropdown component', () => { let wrapper; let vm; - let actions; + let store; function createComponent(extendStore = () => {}) { - const store = new Vuex.Store({ - modules: { - diffs: { - namespaced: true, - actions, - state: diffModule().state, - getters: diffModule().getters, - }, - }, - }); + store = createDiffsStore(); extendStore(store); - wrapper = mount(SettingsDropdown, { - localVue, - store, - }); + wrapper = extendedWrapper( + mount(SettingsDropdown, { + store, + }), + ); vm = wrapper.vm; } function getFileByFileCheckbox(vueWrapper) { - return vueWrapper.find('[data-testid="file-by-file"]'); + return vueWrapper.findByTestId('file-by-file'); + } + + function setup({ storeUpdater } = {}) { + createComponent(storeUpdater); + jest.spyOn(store, 'dispatch').mockImplementation(() => {}); } beforeEach(() => { - actions = { - setInlineDiffViewType: jest.fn(), - setParallelDiffViewType: jest.fn(), - setRenderTreeList: jest.fn(), - setShowWhitespace: jest.fn(), - setFileByFile: jest.fn(), - }; + setup(); }); afterEach(() => { + store.dispatch.mockRestore(); wrapper.destroy(); }); describe('tree view buttons', () => { it('list view button dispatches setRenderTreeList with false', () => { - createComponent(); - wrapper.find('.js-list-view').trigger('click'); - expect(actions.setRenderTreeList).toHaveBeenCalledWith(expect.anything(), false); + expect(store.dispatch).toHaveBeenCalledWith('diffs/setRenderTreeList', false); }); it('tree view button dispatches setRenderTreeList with true', () => { - createComponent(); - wrapper.find('.js-tree-view').trigger('click'); - expect(actions.setRenderTreeList).toHaveBeenCalledWith(expect.anything(), true); + expect(store.dispatch).toHaveBeenCalledWith('diffs/setRenderTreeList', true); }); it('sets list button as selected when renderTreeList is false', () => { - createComponent((store) => { - Object.assign(store.state.diffs, { - renderTreeList: false, - }); + setup({ + storeUpdater: (origStore) => + Object.assign(origStore.state.diffs, { renderTreeList: false }), }); expect(wrapper.find('.js-list-view').classes('selected')).toBe(true); @@ -81,10 +68,8 @@ describe('Diff settings dropdown component', () => { }); it('sets tree button as selected when renderTreeList is true', () => { - createComponent((store) => { - Object.assign(store.state.diffs, { - renderTreeList: true, - }); + setup({ + storeUpdater: (origStore) => Object.assign(origStore.state.diffs, { renderTreeList: true }), }); expect(wrapper.find('.js-list-view').classes('selected')).toBe(false); @@ -94,10 +79,9 @@ describe('Diff settings dropdown component', () => { describe('compare changes', () => { it('sets inline button as selected', () => { - createComponent((store) => { - Object.assign(store.state.diffs, { - diffViewType: INLINE_DIFF_VIEW_TYPE, - }); + setup({ + storeUpdater: (origStore) => + Object.assign(origStore.state.diffs, { diffViewType: INLINE_DIFF_VIEW_TYPE }), }); expect(wrapper.find('.js-inline-diff-button').classes('selected')).toBe(true); @@ -105,10 +89,9 @@ describe('Diff settings dropdown component', () => { }); it('sets parallel button as selected', () => { - createComponent((store) => { - Object.assign(store.state.diffs, { - diffViewType: PARALLEL_DIFF_VIEW_TYPE, - }); + setup({ + storeUpdater: (origStore) => + Object.assign(origStore.state.diffs, { diffViewType: PARALLEL_DIFF_VIEW_TYPE }), }); expect(wrapper.find('.js-inline-diff-button').classes('selected')).toBe(false); @@ -116,53 +99,49 @@ describe('Diff settings dropdown component', () => { }); it('calls setInlineDiffViewType when clicking inline button', () => { - createComponent(); - wrapper.find('.js-inline-diff-button').trigger('click'); - expect(actions.setInlineDiffViewType).toHaveBeenCalled(); + expect(store.dispatch).toHaveBeenCalledWith('diffs/setInlineDiffViewType', expect.anything()); }); it('calls setParallelDiffViewType when clicking parallel button', () => { - createComponent(); - wrapper.find('.js-parallel-diff-button').trigger('click'); - expect(actions.setParallelDiffViewType).toHaveBeenCalled(); + expect(store.dispatch).toHaveBeenCalledWith( + 'diffs/setParallelDiffViewType', + expect.anything(), + ); }); }); describe('whitespace toggle', () => { it('does not set as checked when showWhitespace is false', () => { - createComponent((store) => { - Object.assign(store.state.diffs, { - showWhitespace: false, - }); + setup({ + storeUpdater: (origStore) => + Object.assign(origStore.state.diffs, { showWhitespace: false }), }); - expect(wrapper.find('#show-whitespace').element.checked).toBe(false); + expect(wrapper.findByTestId('show-whitespace').element.checked).toBe(false); }); it('sets as checked when showWhitespace is true', () => { - createComponent((store) => { - Object.assign(store.state.diffs, { - showWhitespace: true, - }); + setup({ + storeUpdater: (origStore) => Object.assign(origStore.state.diffs, { showWhitespace: true }), }); - expect(wrapper.find('#show-whitespace').element.checked).toBe(true); + expect(wrapper.findByTestId('show-whitespace').element.checked).toBe(true); }); - it('calls setShowWhitespace on change', () => { - createComponent(); + it('calls setShowWhitespace on change', async () => { + const checkbox = wrapper.findByTestId('show-whitespace'); + const { checked } = checkbox.element; - const checkbox = wrapper.find('#show-whitespace'); + checkbox.trigger('click'); - checkbox.element.checked = true; - checkbox.trigger('change'); + await vm.$nextTick(); - expect(actions.setShowWhitespace).toHaveBeenCalledWith(expect.anything(), { - showWhitespace: true, + expect(store.dispatch).toHaveBeenCalledWith('diffs/setShowWhitespace', { + showWhitespace: !checked, pushState: true, }); }); @@ -179,15 +158,12 @@ describe('Diff settings dropdown component', () => { ${false} | ${false} `( 'sets the checkbox to { checked: $checked } if the fileByFile setting is $fileByFile', - async ({ fileByFile, checked }) => { - createComponent((store) => { - Object.assign(store.state.diffs, { - viewDiffsFileByFile: fileByFile, - }); + ({ fileByFile, checked }) => { + setup({ + storeUpdater: (origStore) => + Object.assign(origStore.state.diffs, { viewDiffsFileByFile: fileByFile }), }); - await vm.$nextTick(); - expect(getFileByFileCheckbox(wrapper).element.checked).toBe(checked); }, ); @@ -199,19 +175,16 @@ describe('Diff settings dropdown component', () => { `( 'when the file by file setting starts as $start, toggling the checkbox should call setFileByFile with $setting', async ({ start, setting }) => { - createComponent((store) => { - Object.assign(store.state.diffs, { - viewDiffsFileByFile: start, - }); + setup({ + storeUpdater: (origStore) => + Object.assign(origStore.state.diffs, { viewDiffsFileByFile: start }), }); - await vm.$nextTick(); - getFileByFileCheckbox(wrapper).trigger('click'); await vm.$nextTick(); - expect(actions.setFileByFile).toHaveBeenLastCalledWith(expect.anything(), { + expect(store.dispatch).toHaveBeenCalledWith('diffs/setFileByFile', { fileByFile: setting, }); }, diff --git a/spec/frontend/issue_spec.js b/spec/frontend/issue_spec.js index fb6caef41e2..19f7e6bd5a9 100644 --- a/spec/frontend/issue_spec.js +++ b/spec/frontend/issue_spec.js @@ -1,91 +1,94 @@ +import { getByText } from '@testing-library/dom'; import MockAdapter from 'axios-mock-adapter'; -import $ from 'jquery'; -import Issue from '~/issue'; +import Issue, { EVENT_ISSUABLE_VUE_APP_CHANGE } from '~/issue'; import axios from '~/lib/utils/axios_utils'; -import '~/lib/utils/text_utility'; describe('Issue', () => { - let $boxClosed; - let $boxOpen; let testContext; + let mock; - beforeEach(() => { - testContext = {}; + beforeAll(() => { + preloadFixtures('issues/closed-issue.html'); + preloadFixtures('issues/open-issue.html'); }); - preloadFixtures('issues/closed-issue.html'); - preloadFixtures('issues/open-issue.html'); - - function expectVisibility($element, shouldBeVisible) { - if (shouldBeVisible) { - expect($element).not.toHaveClass('hidden'); - } else { - expect($element).toHaveClass('hidden'); - } - } - - function expectIssueState(isIssueOpen) { - expectVisibility($boxClosed, !isIssueOpen); - expectVisibility($boxOpen, isIssueOpen); - } + beforeEach(() => { + mock = new MockAdapter(axios); + mock.onGet(/(.*)\/related_branches$/).reply(200, {}); - function findElements() { - $boxClosed = $('div.status-box-issue-closed'); + testContext = {}; + testContext.issue = new Issue(); + }); - expect($boxClosed).toExist(); - expect($boxClosed).toHaveText('Closed'); + afterEach(() => { + mock.restore(); + testContext.issue.dispose(); + }); - $boxOpen = $('div.status-box-open'); + const getIssueCounter = () => document.querySelector('.issue_counter'); + const getOpenStatusBox = () => + getByText(document, (_, el) => el.textContent.match(/Open/), { + selector: '.status-box-open', + }); + const getClosedStatusBox = () => + getByText(document, (_, el) => el.textContent.match(/Closed/), { + selector: '.status-box-issue-closed', + }); - expect($boxOpen).toExist(); - expect($boxOpen).toHaveText('Open'); - } + describe.each` + desc | isIssueInitiallyOpen | expectedCounterText + ${'with an initially open issue'} | ${true} | ${'1,000'} + ${'with an initially closed issue'} | ${false} | ${'1,002'} + `('$desc', ({ isIssueInitiallyOpen, expectedCounterText }) => { + beforeEach(() => { + if (isIssueInitiallyOpen) { + loadFixtures('issues/open-issue.html'); + } else { + loadFixtures('issues/closed-issue.html'); + } - [true, false].forEach((isIssueInitiallyOpen) => { - describe(`with ${isIssueInitiallyOpen ? 'open' : 'closed'} issue`, () => { - const action = isIssueInitiallyOpen ? 'close' : 'reopen'; - let mock; + testContext.issueCounter = getIssueCounter(); + testContext.statusBoxClosed = getClosedStatusBox(); + testContext.statusBoxOpen = getOpenStatusBox(); - function setup() { - testContext.issue = new Issue(); - expectIssueState(isIssueInitiallyOpen); + testContext.issueCounter.textContent = '1,001'; + }); - testContext.$projectIssuesCounter = $('.issue_counter').first(); - testContext.$projectIssuesCounter.text('1,001'); + it(`has the proper visible status box when ${isIssueInitiallyOpen ? 'open' : 'closed'}`, () => { + if (isIssueInitiallyOpen) { + expect(testContext.statusBoxClosed).toHaveClass('hidden'); + expect(testContext.statusBoxOpen).not.toHaveClass('hidden'); + } else { + expect(testContext.statusBoxClosed).not.toHaveClass('hidden'); + expect(testContext.statusBoxOpen).toHaveClass('hidden'); } + }); + describe('when vue app triggers change', () => { beforeEach(() => { - if (isIssueInitiallyOpen) { - loadFixtures('issues/open-issue.html'); - } else { - loadFixtures('issues/closed-issue.html'); - } - - mock = new MockAdapter(axios); - mock.onGet(/(.*)\/related_branches$/).reply(200, {}); - jest.spyOn(axios, 'get'); - - findElements(isIssueInitiallyOpen); - }); - - afterEach(() => { - mock.restore(); - $('div.flash-alert').remove(); - }); - - it(`${action}s the issue on dispatch of issuable_vue_app:change event`, () => { - setup(); - document.dispatchEvent( - new CustomEvent('issuable_vue_app:change', { + new CustomEvent(EVENT_ISSUABLE_VUE_APP_CHANGE, { detail: { data: { id: 1 }, isClosed: isIssueInitiallyOpen, }, }), ); + }); + + it('displays correct status box', () => { + if (isIssueInitiallyOpen) { + expect(testContext.statusBoxClosed).not.toHaveClass('hidden'); + expect(testContext.statusBoxOpen).toHaveClass('hidden'); + } else { + expect(testContext.statusBoxClosed).toHaveClass('hidden'); + expect(testContext.statusBoxOpen).not.toHaveClass('hidden'); + } + }); - expectIssueState(!isIssueInitiallyOpen); + it('updates issueCounter text', () => { + expect(testContext.issueCounter).toBeVisible(); + expect(testContext.issueCounter).toHaveText(expectedCounterText); }); }); }); diff --git a/spec/frontend/pipelines/pipelines_actions_spec.js b/spec/frontend/pipelines/pipelines_actions_spec.js index 1e6c9e50a7e..a481b1cb850 100644 --- a/spec/frontend/pipelines/pipelines_actions_spec.js +++ b/spec/frontend/pipelines/pipelines_actions_spec.js @@ -1,5 +1,5 @@ import { GlDropdown, GlDropdownItem } from '@gitlab/ui'; -import { shallowMount, mount } from '@vue/test-utils'; +import { shallowMount } from '@vue/test-utils'; import MockAdapter from 'axios-mock-adapter'; import waitForPromises from 'helpers/wait_for_promises'; import { TEST_HOST } from 'spec/test_constants'; @@ -63,10 +63,6 @@ describe('Pipelines Actions dropdown', () => { }); describe('on click', () => { - beforeEach(() => { - createComponent({ actions: mockActions }, mount); - }); - it('makes a request and toggles the loading state', async () => { mock.onPost(mockActions.path).reply(200); diff --git a/spec/frontend/pipelines/pipelines_artifacts_spec.js b/spec/frontend/pipelines/pipelines_artifacts_spec.js index f077833ae16..d4a2db08d97 100644 --- a/spec/frontend/pipelines/pipelines_artifacts_spec.js +++ b/spec/frontend/pipelines/pipelines_artifacts_spec.js @@ -1,24 +1,27 @@ -import { GlDropdown, GlDropdownItem } from '@gitlab/ui'; -import { mount } from '@vue/test-utils'; +import { GlDropdown, GlDropdownItem, GlSprintf } from '@gitlab/ui'; +import { shallowMount } from '@vue/test-utils'; import PipelineArtifacts from '~/pipelines/components/pipelines_list/pipelines_artifacts.vue'; describe('Pipelines Artifacts dropdown', () => { let wrapper; const createComponent = () => { - wrapper = mount(PipelineArtifacts, { + wrapper = shallowMount(PipelineArtifacts, { propsData: { artifacts: [ { - name: 'artifact', + name: 'job my-artifact', path: '/download/path', }, { - name: 'artifact two', + name: 'job-2 my-artifact-2', path: '/download/path-two', }, ], }, + stubs: { + GlSprintf, + }, }); }; @@ -39,8 +42,8 @@ describe('Pipelines Artifacts dropdown', () => { }); it('should render a link with the provided path', () => { - expect(findFirstGlDropdownItem().find('a').attributes('href')).toEqual('/download/path'); + expect(findFirstGlDropdownItem().attributes('href')).toBe('/download/path'); - expect(findFirstGlDropdownItem().text()).toContain('artifact'); + expect(findFirstGlDropdownItem().text()).toBe('Download job my-artifact artifact'); }); }); diff --git a/spec/frontend/pipelines/pipelines_table_row_spec.js b/spec/frontend/pipelines/pipelines_table_row_spec.js index ff997912384..56da636bb3d 100644 --- a/spec/frontend/pipelines/pipelines_table_row_spec.js +++ b/spec/frontend/pipelines/pipelines_table_row_spec.js @@ -1,4 +1,5 @@ import { mount } from '@vue/test-utils'; +import waitForPromises from 'helpers/wait_for_promises'; import PipelinesTableRowComponent from '~/pipelines/components/pipelines_list/pipelines_table_row.vue'; import eventHub from '~/pipelines/event_hub'; @@ -181,10 +182,16 @@ describe('Pipelines Table Row', () => { expect(wrapper.find('.js-pipelines-retry-button').attributes('title')).toMatch('Retry'); expect(wrapper.find('.js-pipelines-cancel-button').exists()).toBe(true); expect(wrapper.find('.js-pipelines-cancel-button').attributes('title')).toMatch('Cancel'); + }); + + it('should render the manual actions', async () => { + const manualActions = wrapper.find('[data-testid="pipelines-manual-actions-dropdown"]'); - const actionsMenu = wrapper.find('[data-testid="pipelines-manual-actions-dropdown"]'); + // Click on the dropdown and wait for `lazy` dropdown items + manualActions.find('.dropdown-toggle').trigger('click'); + await waitForPromises(); - expect(actionsMenu.text()).toContain(scheduledJobAction.name); + expect(manualActions.text()).toContain(scheduledJobAction.name); }); it('emits `retryPipeline` event when retry button is clicked and toggles loading', () => { diff --git a/spec/frontend/vue_shared/components/sidebar/labels_select_vue/dropdown_value_spec.js b/spec/frontend/vue_shared/components/sidebar/labels_select_vue/dropdown_value_spec.js index 0d1d6ebcfe5..c90e63313b2 100644 --- a/spec/frontend/vue_shared/components/sidebar/labels_select_vue/dropdown_value_spec.js +++ b/spec/frontend/vue_shared/components/sidebar/labels_select_vue/dropdown_value_spec.js @@ -11,32 +11,31 @@ import { mockConfig, mockRegularLabel, mockScopedLabel } from './mock_data'; const localVue = createLocalVue(); localVue.use(Vuex); -const createComponent = (initialState = mockConfig, slots = {}) => { - const store = new Vuex.Store(labelsSelectModule()); - - store.dispatch('setInitialState', initialState); - - return shallowMount(DropdownValue, { - localVue, - store, - slots, - }); -}; - describe('DropdownValue', () => { let wrapper; - beforeEach(() => { - wrapper = createComponent(); - }); + const createComponent = (initialState = {}, slots = {}) => { + const store = new Vuex.Store(labelsSelectModule()); + + store.dispatch('setInitialState', { ...mockConfig, ...initialState }); + + wrapper = shallowMount(DropdownValue, { + localVue, + store, + slots, + }); + }; afterEach(() => { wrapper.destroy(); + wrapper = null; }); describe('methods', () => { describe('labelFilterUrl', () => { it('returns a label filter URL based on provided label param', () => { + createComponent(); + expect(wrapper.vm.labelFilterUrl(mockRegularLabel)).toBe( '/gitlab-org/my-project/issues?label_name[]=Foo%20Label', ); @@ -44,6 +43,10 @@ describe('DropdownValue', () => { }); describe('scopedLabel', () => { + beforeEach(() => { + createComponent(); + }); + it('returns `true` when provided label param is a scoped label', () => { expect(wrapper.vm.scopedLabel(mockScopedLabel)).toBe(true); }); @@ -56,28 +59,29 @@ describe('DropdownValue', () => { describe('template', () => { it('renders class `has-labels` on component container element when `selectedLabels` is not empty', () => { + createComponent(); + expect(wrapper.attributes('class')).toContain('has-labels'); }); it('renders element containing `None` when `selectedLabels` is empty', () => { - const wrapperNoLabels = createComponent( + createComponent( { - ...mockConfig, selectedLabels: [], }, { default: 'None', }, ); - const noneEl = wrapperNoLabels.find('span.text-secondary'); + const noneEl = wrapper.find('span.text-secondary'); expect(noneEl.exists()).toBe(true); expect(noneEl.text()).toBe('None'); - - wrapperNoLabels.destroy(); }); it('renders labels when `selectedLabels` is not empty', () => { + createComponent(); + expect(wrapper.findAll(GlLabel).length).toBe(2); }); }); diff --git a/spec/frontend/vue_shared/components/sidebar/labels_select_vue/mock_data.js b/spec/frontend/vue_shared/components/sidebar/labels_select_vue/mock_data.js index 85a14226585..f293b8422e7 100644 --- a/spec/frontend/vue_shared/components/sidebar/labels_select_vue/mock_data.js +++ b/spec/frontend/vue_shared/components/sidebar/labels_select_vue/mock_data.js @@ -47,6 +47,7 @@ export const mockConfig = { labelsFetchPath: '/gitlab-org/my-project/-/labels.json', labelsManagePath: '/gitlab-org/my-project/-/labels', labelsFilterBasePath: '/gitlab-org/my-project/issues', + labelsFilterParam: 'label_name', }; export const mockSuggestedColors = { diff --git a/yarn.lock b/yarn.lock index 2d08a5bdbce..24db1e176d4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -889,7 +889,7 @@ resolved "https://registry.yarnpkg.com/@gitlab/favicon-overlay/-/favicon-overlay-2.0.0.tgz#2f32d0b6a4d5b8ac44e2927083d9ab478a78c984" integrity sha512-GNcORxXJ98LVGzOT9dDYKfbheqH6lNgPDD72lyXRnQIH7CjgGyos8i17aSBPq1f4s3zF3PyedFiAR4YEZbva2Q== -"@gitlab/stylelint-config@^2.2.0": +"@gitlab/stylelint-config@2.2.0": version "2.2.0" resolved "https://registry.yarnpkg.com/@gitlab/stylelint-config/-/stylelint-config-2.2.0.tgz#f0139c8bd29525b51ee9f16d26b66283bd2be5bb" integrity sha512-yLBwRu/geN7nGzoOtF6VV2Fbjhcu2w3PwVnJ5/6wX3MILLO7Wh8zzurIjjSnls8124WUoD7n51Catrjl0hyqDw== |