Automatic merge of gitlab-org/gitlab master

.haml-lint.yml

@@ -137,7 +137,6 @@ linters:
       - Style/ParenthesesAroundCondition
       - Style/RedundantParentheses
       - Style/SelfAssignment
-      - Style/Semicolon
       - Style/TernaryParentheses
       - Style/TrailingCommaInHashLiteral
       - Style/UnlessElse

app/assets/javascripts/design_management/components/design_notes/design_note.vue

 <script>
 import { GlTooltipDirective, GlIcon, GlLink, GlSafeHtmlDirective } from '@gitlab/ui';
 import { ApolloMutation } from 'vue-apollo';
+import { __ } from '~/locale';
 import TimelineEntryItem from '~/vue_shared/components/notes/timeline_entry_item.vue';
 import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
 import UserAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue';
@@ -10,6 +11,9 @@ import { findNoteId, extractDesignNoteId } from '../../utils/design_management_u
 import DesignReplyForm from './design_reply_form.vue';
 
 export default {
+  i18n: {
+    editCommentLabel: __('Edit comment'),
+  },
   components: {
     UserAvatarLink,
     TimelineEntryItem,
@@ -113,7 +117,8 @@ export default {
           v-if="isEditButtonVisible"
           v-gl-tooltip
           type="button"
-          :title="__('Edit comment')"
+          :title="$options.i18n.editCommentLabel"
+          :aria-label="$options.i18n.editCommentLabel"
           class="note-action-button js-note-edit btn btn-transparent qa-note-edit-button"
           @click="isEditing = true"
         >

app/assets/javascripts/diffs/components/diff_discussions.vue

@@ -68,6 +68,7 @@ export default {
             }"
             type="button"
             class="js-diff-notes-toggle"
+            :aria-label="__('Show comments')"
             @click="toggleDiscussion({ discussionId: discussion.id })"
           >
             <gl-icon v-if="discussion.expanded" name="collapse" class="collapse-icon" />

app/assets/javascripts/diffs/components/image_diff_overlay.vue

@@ -122,6 +122,7 @@ export default {
       :disabled="!shouldToggleDiscussion"
       class="js-image-badge"
       type="button"
+      :aria-label="__('Show comments')"
       @click="clickedToggle(discussion)"
     >
       <gl-icon v-if="showCommentIcon" name="image-comment-dark" :size="24" />

app/assets/javascripts/diffs/components/inline_diff_table_row.vue

@@ -146,6 +146,7 @@ export default {
           type="button"
           class="add-diff-note note-button js-add-diff-note-button"
           :disabled="line.commentsDisabled"
+          :aria-label="addCommentTooltip"
           @click="handleCommentButton"
         >
           <gl-icon :size="12" name="comment" />

app/assets/javascripts/diffs/components/parallel_diff_table_row.vue

@@ -195,6 +195,7 @@ export default {
             type="button"
             class="add-diff-note note-button js-add-diff-note-button qa-diff-comment"
             :disabled="line.left.commentsDisabled"
+            :aria-label="addCommentTooltipLeft"
             @click="handleCommentButton(line.left)"
           >
             <gl-icon :size="12" name="comment" />
@@ -252,6 +253,7 @@ export default {
             type="button"
             class="add-diff-note note-button js-add-diff-note-button qa-diff-comment"
             :disabled="line.right.commentsDisabled"
+            :aria-label="addCommentTooltipRight"
             @click="handleCommentButton(line.right)"
           >
             <gl-icon :size="12" name="comment" />

app/assets/javascripts/emoji/components/picker.vue

@@ -105,6 +105,7 @@ export default {
           }"
           type="button"
           class="gl-border-0 gl-border-b-2 gl-border-b-solid gl-flex-fill-1 gl-text-gray-300 gl-pt-3 gl-pb-3 gl-bg-transparent emoji-picker-category-tab"
+          :aria-label="category.name"
           @click="scrollToCategory(category.name)"
         >
           <gl-icon :name="category.icon" :size="12" />

app/assets/javascripts/error_tracking/components/error_tracking_actions.vue

@@ -51,6 +51,7 @@ export default {
         v-gl-tooltip.hover
         class="gl-display-block gl-mb-4 mb-md-0 gl-w-full"
         :title="ignoreBtn.title"
+        :aria-label="ignoreBtn.title"
         @click="$emit('update-issue-status', { errorId: error.id, status: ignoreBtn.status })"
       >
         <gl-icon class="gl-display-none d-md-inline gl-m-0" :name="ignoreBtn.icon" :size="12" />
@@ -62,6 +63,7 @@ export default {
         v-gl-tooltip.hover
         class="gl-display-block gl-mb-4 mb-md-0 gl-w-full"
         :title="resolveBtn.title"
+        :aria-label="resolveBtn.title"
         @click="$emit('update-issue-status', { errorId: error.id, status: resolveBtn.status })"
       >
         <gl-icon class="gl-display-none d-md-inline gl-m-0" :name="resolveBtn.icon" :size="12" />

app/assets/javascripts/ide/components/jobs/detail/scroll_button.vue

@@ -55,6 +55,7 @@ export default {
       :disabled="disabled"
       class="btn-scroll btn-transparent btn-blank"
       type="button"
+      :aria-label="tooltipTitle"
       @click="clickedScroll"
     >
       <gl-icon :name="iconName" />

app/assets/javascripts/search/topbar/components/searchable_dropdown.vue

@@ -9,10 +9,13 @@ import {
   GlSkeletonLoader,
   GlTooltipDirective,
 } from '@gitlab/ui';
-
+import { __ } from '~/locale';
 import { ANY_OPTION } from '../constants';
 
 export default {
+  i18n: {
+    clearLabel: __('Clear'),
+  },
   name: 'SearchableDropdown',
   components: {
     GlDropdown,
@@ -96,7 +99,8 @@ export default {
         v-gl-tooltip
         name="clear"
         category="tertiary"
-        :title="__('Clear')"
+        :title="$options.i18n.clearLabel"
+        :aria-label="$options.i18n.clearLabel"
         class="gl-p-0! gl-mr-2"
         @keydown.enter.stop="resetDropdown"
         @click.stop="resetDropdown"

app/assets/javascripts/vue_merge_request_widget/components/deployment/deployment_action_button.vue

@@ -68,6 +68,7 @@ export default {
       category="primary"
       size="small"
       :title="buttonTitle"
+      :aria-label="buttonTitle"
       :loading="isLoading"
       :disabled="isActionInProgress"
       :class="`inline gl-ml-2 ${containerClasses}`"

app/assets/javascripts/vue_shared/components/help_popover.vue

@@ -25,7 +25,7 @@ export default {
 </script>
 <template>
   <span>
-    <gl-button ref="popoverTrigger" variant="link" icon="question" tabindex="0" />
+    <gl-button ref="popoverTrigger" variant="link" icon="question" :aria-label="__('Help')" />
     <gl-popover :target="() => $refs.popoverTrigger.$el" v-bind="options">
       <template v-if="options.title" #title>
         <span v-safe-html="options.title"></span>

app/assets/javascripts/vue_shared/components/rich_content_editor/toolbar_item.vue

@@ -21,7 +21,11 @@ export default {
 };
 </script>
 <template>
-  <button v-gl-tooltip="{ title: tooltip }" class="p-0 gl-display-flex toolbar-button">
+  <button
+    v-gl-tooltip="{ title: tooltip }"
+    :aria-label="tooltip"
+    class="p-0 gl-display-flex toolbar-button"
+  >
     <gl-icon class="gl-mx-auto gl-align-self-center" :name="icon" />
   </button>
 </template>

app/models/concerns/issuable.rb

@@ -137,14 +137,6 @@ module Issuable
     scope :references_project, -> { references(:project) }
     scope :non_archived, -> { join_project.where(projects: { archived: false }) }
 
-    scope :includes_for_bulk_update, -> do
-      associations = %i[author assignees epic group labels metrics project source_project target_project].select do |association|
-        reflect_on_association(association)
-      end
-
-      includes(*associations)
-    end
-
     attr_mentionable :title, pipeline: :single_line
     attr_mentionable :description
 

app/models/concerns/milestoneable.rb

@@ -39,13 +39,11 @@ module Milestoneable
     private
 
     def milestone_is_valid
-      errors.add(:milestone_id, 'is invalid') if respond_to?(:milestone_id) && !milestone_available?
+      errors.add(:milestone_id, 'is invalid') if respond_to?(:milestone_id) && milestone_id.present? && !milestone_available?
     end
   end
 
   def milestone_available?
-    return true if milestone_id.blank?
-
     project_id == milestone&.project_id || project.ancestors_upto.compact.include?(milestone&.group)
   end
 

app/services/issuable/bulk_update_service.rb

@@ -15,7 +15,7 @@ module Issuable
       set_update_params(type)
       items = update_issuables(type, ids)
 
-      response_success(payload: { count: items.size })
+      response_success(payload: { count: items.count })
     rescue ArgumentError => e
       response_error(e.message, 422)
     end
@@ -59,17 +59,10 @@ module Issuable
 
     def find_issuables(parent, model_class, ids)
       if parent.is_a?(Project)
-        projects = parent
+        model_class.id_in(ids).of_projects(parent)
       elsif parent.is_a?(Group)
-        projects = parent.all_projects
-      else
-        return
+        model_class.id_in(ids).of_projects(parent.all_projects)
       end
-
-      model_class
-        .id_in(ids)
-        .of_projects(projects)
-        .includes_for_bulk_update
     end
 
     def response_success(message: nil, payload: nil)

app/views/layouts/_flash.html.haml

 -# We currently only support `alert`, `notice`, `success`, 'toast'
-- icons = {'alert' => 'error', 'notice' => 'information-o', 'success' => 'check-circle'};
+- icons = {'alert' => 'error', 'notice' => 'information-o', 'success' => 'check-circle'}
 .flash-container.flash-container-page.sticky{ data: { qa_selector: 'flash_container' } }
   - flash.each do |key, value|
     - if key == 'toast' && value

app/views/shared/blob/_markdown_buttons.html.haml

-- modifier_key = client_js_flags[:isMac] ? '⌘' : s_('KeyboardKey|Ctrl+');
+- modifier_key = client_js_flags[:isMac] ? '⌘' : s_('KeyboardKey|Ctrl+')
 
 .md-header-toolbar.active
   = markdown_toolbar_button({ icon: "bold",

changelogs/unreleased/21068-optimize-issueable-updates.yml

----
-title: Optimize issuable updates
-merge_request: 58114
-author:
-type: performance

changelogs/unreleased/cngo-add-aria-labels-to-icon-only-buttons-3.yml

+---
+title: Add aria labels to icon-only buttons
+merge_request: 59037
+author:
+type: fixed

doc/update/index.md

@@ -177,7 +177,7 @@ Find where your version sits in the upgrade path below, and upgrade GitLab
 accordingly, while also consulting the
 [version-specific upgrade instructions](#version-specific-upgrading-instructions):
 
-`8.11.Z` -> `8.12.0` -> `8.17.7` -> `9.5.10` -> `10.8.7` -> `11.11.8` -> `12.0.12` -> `12.1.17` -> `12.10.14` -> `13.0.14` -> `13.1.11`  -> `13.5.4` - > [latest `13.Y.Z`](https://about.gitlab.com/releases/categories/releases/)
+`8.11.Z` -> `8.12.0` -> `8.17.7` -> `9.5.10` -> `10.8.7` -> `11.11.8` -> `12.0.12` -> `12.1.17` -> `12.10.14` -> `13.0.14` -> `13.1.11`  -> [latest `13.Y.Z`](https://about.gitlab.com/releases/categories/releases/)
 
 The following table, while not exhaustive, shows some examples of the supported
 upgrade paths.

ee/app/assets/javascripts/analytics/productivity_analytics/components/app.vue

@@ -326,7 +326,12 @@ export default {
                     </span>
                   </gl-dropdown-item>
                 </gl-dropdown>
-                <gl-button v-gl-tooltip.hover :title="sortTooltipTitle" @click="toggleSortOrder">
+                <gl-button
+                  v-gl-tooltip.hover
+                  :title="sortTooltipTitle"
+                  :aria-label="sortTooltipTitle"
+                  @click="toggleSortOrder"
+                >
                   <gl-icon :name="sortIcon" />
                 </gl-button>
               </div>

ee/app/assets/javascripts/dependencies/components/dependencies_actions.vue

 <script>
 import { GlButton, GlDropdown, GlDropdownItem, GlIcon, GlTooltipDirective } from '@gitlab/ui';
 import { mapActions, mapState } from 'vuex';
+import { __ } from '~/locale';
 import { DEPENDENCY_LIST_TYPES } from '../store/constants';
 import { SORT_FIELDS, SORT_ORDER } from '../store/modules/list/constants';
 
 export default {
+  i18n: {
+    sortDirectionLabel: __('Sort direction'),
+  },
   name: 'DependenciesActions',
   components: {
     GlButton,
@@ -80,7 +84,8 @@ export default {
       </gl-dropdown>
       <gl-button
         v-gl-tooltip
-        :title="__('Sort direction')"
+        :title="$options.i18n.sortDirectionLabel"
+        :aria-label="$options.i18n.sortDirectionLabel"
         class="flex-grow-0 js-sort-order"
         @click="toggleSortOrder"
       >

ee/app/assets/javascripts/related_items_tree/components/tree_item.vue

@@ -87,6 +87,7 @@ export default {
         v-if="!childrenFetchInProgress && hasChildren"
         v-gl-tooltip.viewport.hover
         :title="chevronTooltip"
+        :aria-label="chevronTooltip"
         :class="chevronType"
         variant="link"
         class="btn-svg btn-tree-item-chevron align-self-start"

ee/app/services/ee/issuable/bulk_update_service.rb

@@ -9,12 +9,9 @@ module EE
 
       override :find_issuables
       def find_issuables(parent, model_class, ids)
-        return super unless model_class == ::Epic
+        return model_class.id_in(ids).in_selected_groups(parent.self_and_descendants) if model_class == ::Epic
 
-        model_class
-          .id_in(ids)
-          .in_selected_groups(parent.self_and_descendants)
-          .includes_for_bulk_update
+        super
       end
 
       override :permitted_attrs

ee/spec/frontend/dependencies/components/__snapshots__/dependencies_actions_spec.js.snap

@@ -83,6 +83,7 @@ exports[`DependenciesActions component matches the snapshot 1`] = `
     </gl-dropdown-stub>
      
     <gl-button-stub
+      aria-label="Sort direction"
       buttontextclasses=""
       category="primary"
       class="flex-grow-0 js-sort-order"

ee/spec/lib/gitlab/analytics/cycle_analytics/request_params_spec.rb

@@ -204,11 +204,13 @@ RSpec.describe Gitlab::Analytics::CycleAnalytics::RequestParams do
 
     subject { described_class.new(params).to_data_attributes }
 
-    it { expect(subject[:milestone]).to eq('title') }
-    it { expect(subject[:assignees]).to eq('["username1"]') }
-    it { expect(subject[:labels]).to eq('["label1","label2"]') }
-    it { expect(subject[:author]).to eq('author') }
-    it { expect(subject[:stage]).to eq('{"id":1,"title":"Stage #1"}') }
+    it "has the correct attributes" do
+      expect(subject[:milestone]).to eq('title')
+      expect(subject[:assignees]).to eq('["username1"]')
+      expect(subject[:labels]).to eq('["label1","label2"]')
+      expect(subject[:author]).to eq('author')
+      expect(subject[:stage]).to eq(%Q|{"id":#{stage.id},"title":"#{stage.name}"}|)
+    end
   end
 
   describe 'sorting params' do

ee/spec/models/concerns/ee/milestoneable_spec.rb

@@ -4,11 +4,11 @@ require 'spec_helper'
 
 RSpec.describe EE::Milestoneable do
   describe '#milestone_available?' do
-    context 'for epics' do
-      let(:epic) { build(:epic) }
+    context 'no Epic' do
+      let(:issue) { create(:issue) }
 
-      it 'returns true' do
-        expect(epic.milestone_available?).to be(true)
+      it 'returns false' do
+        expect(issue.milestone_available?).to be_falsy
       end
     end
   end
@@ -21,7 +21,7 @@ RSpec.describe EE::Milestoneable do
       let(:epic) { build(:epic) }
 
       it 'returns false' do
-        expect(epic.supports_milestone?).to be(false)
+        expect(epic.supports_milestone?).to be_falsy
       end
     end
   end

spec/frontend/vue_shared/components/help_popover_spec.js

@@ -34,7 +34,6 @@ describe('HelpPopover', () => {
       icon: 'question',
       variant: 'link',
     });
-    expect(findQuestionButton().attributes().tabindex).toBe('0');
   });
 
   it('renders popover that uses the question button as target', () => {

spec/models/concerns/issuable_spec.rb

@@ -65,23 +65,6 @@ RSpec.describe Issuable do
     it { expect(issuable_class).to respond_to(:opened) }
     it { expect(issuable_class).to respond_to(:closed) }
     it { expect(issuable_class).to respond_to(:assigned) }
-
-    describe '.includes_for_bulk_update' do
-      before do
-        stub_const('Example', Class.new(ActiveRecord::Base))
-
-        Example.class_eval do
-          include Issuable # adds :labels and :metrics, among others
-
-          belongs_to :author
-          has_many :assignees
-        end
-      end
-
-      it 'includes available associations' do
-        expect(Example.includes_for_bulk_update.includes_values).to eq([:author, :assignees, :labels, :metrics])
-      end
-    end
   end
 
   describe 'author_name' do

spec/models/concerns/milestoneable_spec.rb

@@ -50,13 +50,13 @@ RSpec.describe Milestoneable do
       it 'returns true with a milestone from the issue project' do
         milestone = create(:milestone, project: project)
 
-        expect(build_milestoneable(milestone.id).milestone_available?).to be(true)
+        expect(build_milestoneable(milestone.id).milestone_available?).to be_truthy
       end
 
       it 'returns true with a milestone from the issue project group' do
         milestone = create(:milestone, group: group)
 
-        expect(build_milestoneable(milestone.id).milestone_available?).to be(true)
+        expect(build_milestoneable(milestone.id).milestone_available?).to be_truthy
       end
 
       it 'returns true with a milestone from the the parent of the issue project group' do
@@ -64,23 +64,19 @@ RSpec.describe Milestoneable do
         group.update!(parent: parent)
         milestone = create(:milestone, group: parent)
 
-        expect(build_milestoneable(milestone.id).milestone_available?).to be(true)
-      end
-
-      it 'returns true with a blank milestone' do
-        expect(build_milestoneable('').milestone_available?).to be(true)
+        expect(build_milestoneable(milestone.id).milestone_available?).to be_truthy
       end
 
       it 'returns false with a milestone from another project' do
         milestone = create(:milestone)
 
-        expect(build_milestoneable(milestone.id).milestone_available?).to be(false)
+        expect(build_milestoneable(milestone.id).milestone_available?).to be_falsey
       end
 
       it 'returns false with a milestone from another group' do
         milestone = create(:milestone, group: create(:group))
 
-        expect(build_milestoneable(milestone.id).milestone_available?).to be(false)
+        expect(build_milestoneable(milestone.id).milestone_available?).to be_falsey
       end
     end
   end