diff options
36 files changed, 188 insertions, 105 deletions
diff --git a/.gitlab/CODEOWNERS b/.gitlab/CODEOWNERS index ae971c2f987..a545196b597 100644 --- a/.gitlab/CODEOWNERS +++ b/.gitlab/CODEOWNERS @@ -172,6 +172,11 @@ Dangerfile @gl-quality/eng-prod /scripts/review_apps/seed-dast-test-data.sh @dappelt @ngeorge1 @gl-quality/eng-prod .editorconfig @gl-quality/eng-prod +[Backend Static Code Analysis] +.rubocop*.yml @dstull @splattael @gl-quality/eng-prod +/rubocop/ @dstull @splattael @gl-quality/eng-prod +/spec/rubocop/ @dstull @splattael @gl-quality/eng-prod + [End-to-end] /qa/ @gl-quality @@ -196,7 +196,7 @@ gem 'acts-as-taggable-on', '~> 7.0' gem 'sidekiq', '~> 5.2.7' gem 'sidekiq-cron', '~> 1.0' gem 'redis-namespace', '~> 1.7.0' -gem 'gitlab-sidekiq-fetcher', '0.5.2', require: 'sidekiq-reliable-fetch' +gem 'gitlab-sidekiq-fetcher', '0.5.3', require: 'sidekiq-reliable-fetch' # Cron Parser gem 'fugit', '~> 1.2.1' diff --git a/Gemfile.lock b/Gemfile.lock index de9836ecf67..9369da2ed0d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -451,7 +451,7 @@ GEM gitlab-pry-byebug (3.9.0) byebug (~> 11.0) pry (~> 0.13.0) - gitlab-sidekiq-fetcher (0.5.2) + gitlab-sidekiq-fetcher (0.5.3) sidekiq (~> 5) gitlab-styles (6.0.0) rubocop (~> 0.91.1) @@ -1381,7 +1381,7 @@ DEPENDENCIES gitlab-markup (~> 1.7.1) gitlab-net-dns (~> 0.9.1) gitlab-pry-byebug - gitlab-sidekiq-fetcher (= 0.5.2) + gitlab-sidekiq-fetcher (= 0.5.3) gitlab-styles (~> 6.0.0) gitlab_chronic_duration (~> 0.10.6.2) gitlab_omniauth-ldap (~> 2.1.1) diff --git a/app/assets/javascripts/design_management/components/design_notes/design_discussion.vue b/app/assets/javascripts/design_management/components/design_notes/design_discussion.vue index 33f0aa00cad..41b123c7cb1 100644 --- a/app/assets/javascripts/design_management/components/design_notes/design_discussion.vue +++ b/app/assets/javascripts/design_management/components/design_notes/design_discussion.vue @@ -257,8 +257,8 @@ export default { <reply-placeholder v-if="!isFormVisible" class="qa-discussion-reply" - :button-text="__('Reply...')" - @onClick="showForm" + :placeholder-text="__('Reply…')" + @focus="showForm" /> <apollo-mutation v-else diff --git a/app/assets/javascripts/diffs/components/diff_discussion_reply.vue b/app/assets/javascripts/diffs/components/diff_discussion_reply.vue index 9027d0c8aa4..3766c125325 100644 --- a/app/assets/javascripts/diffs/components/diff_discussion_reply.vue +++ b/app/assets/javascripts/diffs/components/diff_discussion_reply.vue @@ -35,8 +35,9 @@ export default { <slot v-if="hasForm" name="form"></slot> <template v-else-if="renderReplyPlaceholder"> <reply-placeholder - :button-text="__('Start a new discussion...')" - @onClick="$emit('showNewDiscussionForm')" + :placeholder-text="__('Start a new discussion…')" + :label-text="__('New discussion')" + @focus="$emit('showNewDiscussionForm')" /> </template> </template> diff --git a/app/assets/javascripts/notes/components/discussion_actions.vue b/app/assets/javascripts/notes/components/discussion_actions.vue index 27408bc3354..6f0745d4fb0 100644 --- a/app/assets/javascripts/notes/components/discussion_actions.vue +++ b/app/assets/javascripts/notes/components/discussion_actions.vue @@ -50,8 +50,8 @@ export default { <div class="discussion-with-resolve-btn clearfix"> <reply-placeholder data-qa-selector="discussion_reply_tab" - :button-text="s__('MergeRequests|Reply...')" - @onClick="$emit('showReplyForm')" + :placeholder-text="__('Reply…')" + @focus="$emit('showReplyForm')" /> <div v-if="userCanResolveDiscussion" class="btn-group discussion-actions" role="group"> diff --git a/app/assets/javascripts/notes/components/discussion_reply_placeholder.vue b/app/assets/javascripts/notes/components/discussion_reply_placeholder.vue index 0204169214b..1165a869d2b 100644 --- a/app/assets/javascripts/notes/components/discussion_reply_placeholder.vue +++ b/app/assets/javascripts/notes/components/discussion_reply_placeholder.vue @@ -1,23 +1,30 @@ <script> +import { __ } from '~/locale'; + export default { name: 'ReplyPlaceholder', props: { - buttonText: { + placeholderText: { + type: String, + required: false, + default: __('Reply…'), + }, + labelText: { type: String, - required: true, + required: false, + default: __('Reply to comment'), }, }, }; </script> <template> - <button - ref="button" - type="button" - class="js-vue-discussion-reply btn btn-text-field" - :title="s__('MergeRequests|Add a reply')" - @click="$emit('onClick')" - > - {{ buttonText }} - </button> + <textarea + ref="textarea" + rows="1" + class="reply-placeholder-text-field js-vue-discussion-reply" + :placeholder="placeholderText" + :aria-label="labelText" + @focus="$emit('focus')" + ></textarea> </template> diff --git a/app/assets/javascripts/notes/components/note_form.vue b/app/assets/javascripts/notes/components/note_form.vue index 653bc450d0b..a28c467117a 100644 --- a/app/assets/javascripts/notes/components/note_form.vue +++ b/app/assets/javascripts/notes/components/note_form.vue @@ -345,7 +345,7 @@ export default { class="note-textarea js-gfm-input js-note-text js-autosize markdown-area js-vue-issue-note-form" data-qa-selector="reply_field" dir="auto" - :aria-label="__('Description')" + :aria-label="__('Reply to comment')" :placeholder="__('Write a comment or drag your files here…')" @keydown.meta.enter="handleKeySubmit()" @keydown.ctrl.enter="handleKeySubmit()" diff --git a/app/assets/stylesheets/framework/buttons.scss b/app/assets/stylesheets/framework/buttons.scss index b51fec925cb..91af41c68dd 100644 --- a/app/assets/stylesheets/framework/buttons.scss +++ b/app/assets/stylesheets/framework/buttons.scss @@ -216,6 +216,7 @@ color: $gray-700; } + // deprecated class &.btn-text-field { width: 100%; text-align: left; diff --git a/app/assets/stylesheets/framework/filters.scss b/app/assets/stylesheets/framework/filters.scss index 07d59847829..5803b274d9b 100644 --- a/app/assets/stylesheets/framework/filters.scss +++ b/app/assets/stylesheets/framework/filters.scss @@ -453,7 +453,7 @@ .search-token-target-branch { .value { font-family: $monospace-font; - font-size: 13px; + font-size: $gl-font-size-monospace; } } diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index 496e2aba421..00a111286dd 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -604,7 +604,7 @@ pre { display: block; padding: $gl-padding-8 $input-horizontal-padding; margin: 0 0 $gl-padding-8; - font-size: 13px; + font-size: $gl-font-size-monospace; word-break: break-all; word-wrap: break-word; color: $gl-text-color; @@ -646,7 +646,7 @@ code { */ textarea.js-gfm-input { font-family: $monospace-font; - font-size: 13px; + font-size: $gl-font-size-monospace; } .strikethrough { diff --git a/app/assets/stylesheets/pages/note_form.scss b/app/assets/stylesheets/pages/note_form.scss index ffbfa47f9bd..9c9ce745630 100644 --- a/app/assets/stylesheets/pages/note_form.scss +++ b/app/assets/stylesheets/pages/note_form.scss @@ -302,8 +302,20 @@ table { width: 100%; } } +} + +.discussion-reply-holder { + .reply-placeholder-text-field { + font-family: $monospace-font; + font-size: $gl-font-size-monospace; + border-radius: $gl-border-radius-base; + width: 100%; + resize: none; + padding: $gl-padding-8 $gl-padding-12; + line-height: 1; + border-color: $border-color; + background-color: $white; - .btn-text-field { @include media-breakpoint-down(xs) { margin-bottom: $gl-padding-8; } diff --git a/app/views/projects/settings/ci_cd/_form.html.haml b/app/views/projects/settings/ci_cd/_form.html.haml index e0c4a3d624e..3b0073848a6 100644 --- a/app/views/projects/settings/ci_cd/_form.html.haml +++ b/app/views/projects/settings/ci_cd/_form.html.haml @@ -96,7 +96,7 @@ = html_escape(_('The regular expression used to find test coverage output in the job log. For example, use %{regex} for Simplecov (Ruby). Leave blank to disable.')) % { regex: '<code>\(\d+.\d+%\)</code>'.html_safe } = link_to sprite_icon('question-o'), help_page_path('ci/pipelines/settings', anchor: 'test-coverage-parsing'), target: '_blank' - = f.submit _('Save changes'), class: "btn btn-success", data: { qa_selector: 'save_general_pipelines_changes_button' } + = f.submit _('Save changes'), class: "btn gl-button btn-success", data: { qa_selector: 'save_general_pipelines_changes_button' } %hr diff --git a/changelogs/unreleased/241058-mg-update-reply-placeholder.yml b/changelogs/unreleased/241058-mg-update-reply-placeholder.yml new file mode 100644 index 00000000000..04a1368b46b --- /dev/null +++ b/changelogs/unreleased/241058-mg-update-reply-placeholder.yml @@ -0,0 +1,5 @@ +--- +title: Update accessibility of the "Reply to discussion" UX +merge_request: 54380 +author: +type: other diff --git a/doc/development/testing_guide/best_practices.md b/doc/development/testing_guide/best_practices.md index 79ff46ae352..da8db98e244 100644 --- a/doc/development/testing_guide/best_practices.md +++ b/doc/development/testing_guide/best_practices.md @@ -168,7 +168,7 @@ can be used: ```ruby RSpec.describe API::Search, factory_default: :keep do - let_it_be(:namespace) { create_default(:namespace) } + let_it_be(:namespace) { create_default(:namespace).freeze } ``` Then every project we create uses this `namespace`, without us having to pass @@ -176,13 +176,17 @@ it as `namespace: namespace`. In order to make it work along with `let_it_be`, ` must be explicitly specified. That keeps the default factory for every example in a suite instead of recreating it for each example. +Objects created inside a `factory_default: :keep`, and using +`create_default` inside a `let_it_be` should be frozen to prevent accidental reliance +between test examples. + Maybe we don't need to create 208 different projects - we can create one and reuse it. In addition, we can see that only about 1/3 of the projects we create are ones we ask for (76/208). There is benefit in setting a default value for projects as well: ```ruby - let_it_be(:project) { create_default(:project) } + let_it_be(:project) { create_default(:project).freeze } ``` In this case, the `total time` and `top-level time` numbers match more closely: diff --git a/doc/user/project/issue_board.md b/doc/user/project/issue_board.md index e4f42b97b84..28b463774f4 100644 --- a/doc/user/project/issue_board.md +++ b/doc/user/project/issue_board.md @@ -277,6 +277,32 @@ group and its descendant subgroups. Similarly, you can only filter by group labe boards. When updating milestones and labels for an issue through the sidebar update mechanism, again only group-level objects are available. +#### GraphQL-based sidebar for group issue boards **(PREMIUM)** + +<!-- When the feature flag is removed, integrate this section into the above ("Group issue boards"). --> + +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/285074) in GitLab 13.9. +> - It's [deployed behind a feature flag](../feature_flags.md), disabled by default. +> - It's disabled on GitLab.com. +> - It's not recommended for production use. +> - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enable-or-disable-graphql-based-sidebar-for-group-issue-boards). **(PREMIUM SELF)** + +WARNING: +This feature might not be available to you. Check the **version history** note above for details. + +The work-in-progress GraphQL-based sidebar for group issue boards brings better performance and the +ability to edit issue titles in the issue sidebar. + +To **edit an issue's title** in the issue sidebar: + +1. In a group issue board, select the issue card. The issue sidebar opens on the right. +1. Next to the issue's title, select **Edit**. + +This is work in progress as of GitLab 13.9. Learn more about the known issues in +[MR 51480](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/51480). + +<!-- Add this at the end of the file --> + ### Assignee lists **(PREMIUM)** > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/5784) in [GitLab Premium](https://about.gitlab.com/pricing/) 11.0. @@ -317,6 +343,7 @@ As in other list types, click the trash icon to remove a list. > - Grouping by epic [introduced](https://gitlab.com/groups/gitlab-org/-/epics/3352) in [GitLab Premium](https://about.gitlab.com/pricing/) 13.6. > - Editing issue titles in the issue sidebar [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/232745) in GitLab 13.8. +> - Editing iteration in the issue sidebar [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/290232) in GitLab 13.9. With swimlanes you can visualize issues grouped by epic. Your issue board keeps all the other features, but with a different visual organization of issues. @@ -456,24 +483,6 @@ the list by filtering by the following: - Release - Weight -#### Enable or disable adding issues to the list **(FREE SELF)** - -Adding issues to the list is deployed behind a feature flag that is **disabled by default**. -[GitLab administrators with access to the GitLab Rails console](../../administration/feature_flags.md) -can enable it. - -To enable it: - -```ruby -Feature.enable(:add_issues_button) -``` - -To disable it: - -```ruby -Feature.disable(:add_issues_button) -``` - ### Remove an issue from a list Removing an issue from a list can be done by clicking the issue card and then @@ -593,3 +602,40 @@ A few things to remember: - For performance and visibility reasons, each list shows the first 20 issues by default. If you have more than 20 issues, start scrolling down and the next 20 appear. + +## Enable or disable GraphQL-based sidebar for group issue boards **(PREMIUM SELF)** + +GraphQL-based sidebar for group issue boards is under development and not ready for production use. +It is deployed behind a feature flag that is **disabled by default**. +[GitLab administrators with access to the GitLab Rails console](../../administration/feature_flags.md) +can enable it. + +To enable it: + +```ruby +Feature.enable(:graphql_board_lists) +``` + +To disable it: + +```ruby +Feature.disable(:graphql_board_lists) +``` + +## Enable or disable adding issues to the list **(FREE SELF)** + +Adding issues to the list is deployed behind a feature flag that is **disabled by default**. +[GitLab administrators with access to the GitLab Rails console](../../administration/feature_flags.md) +can enable it. + +To enable it: + +```ruby +Feature.enable(:add_issues_button) +``` + +To disable it: + +```ruby +Feature.disable(:add_issues_button) +``` diff --git a/locale/gitlab.pot b/locale/gitlab.pot index c78da6956a9..90f5c85e823 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -18644,9 +18644,6 @@ msgstr "" msgid "MergeRequestDiffs|Select comment starting line" msgstr "" -msgid "MergeRequests|Add a reply" -msgstr "" - msgid "MergeRequests|An error occurred while checking whether another squash is in progress." msgstr "" @@ -18656,9 +18653,6 @@ msgstr "" msgid "MergeRequests|Failed to squash. Should be done manually." msgstr "" -msgid "MergeRequests|Reply..." -msgstr "" - msgid "MergeRequests|Resolve this thread in a new issue" msgstr "" @@ -19935,6 +19929,9 @@ msgstr "" msgid "New directory" msgstr "" +msgid "New discussion" +msgstr "" + msgid "New environment" msgstr "" @@ -24867,7 +24864,7 @@ msgstr "" msgid "Reply to this email directly or %{view_it_on_gitlab}." msgstr "" -msgid "Reply..." +msgid "Reply…" msgstr "" msgid "Repo by URL" @@ -27974,7 +27971,7 @@ msgstr "" msgid "Start a Free Ultimate Trial" msgstr "" -msgid "Start a new discussion..." +msgid "Start a new discussion…" msgstr "" msgid "Start a new merge request" diff --git a/scripts/validate_migration_schema b/scripts/validate_migration_schema index 95a9ae9f93f..0b909f7ac77 100755 --- a/scripts/validate_migration_schema +++ b/scripts/validate_migration_schema @@ -27,7 +27,7 @@ class MigrationSchemaValidator private def validate_schema_on_rollback! - committed_migrations.each do |filename| + committed_migrations.reverse_each do |filename| version = find_migration_version(filename) run("bin/rails db:migrate:down VERSION=#{version}") diff --git a/spec/factories_spec.rb b/spec/factories_spec.rb index 38ade20de28..cc84a5de368 100644 --- a/spec/factories_spec.rb +++ b/spec/factories_spec.rb @@ -29,9 +29,16 @@ RSpec.describe 'factories' do # and reuse them in other factories. # # However, for some factories we cannot use FactoryDefault because the - # associations must be unique and cannot be reused. + # associations must be unique and cannot be reused, or the factory default + # is being mutated. skip_factory_defaults = %i[ fork_network_member + group_member + import_state + namespace + project_broken_repo + users_star_project + wiki_page ].to_set.freeze # Some factories and their corresponding models are based on @@ -46,9 +53,9 @@ RSpec.describe 'factories' do .partition { |factory| skip_factory_defaults.include?(factory.name) } context 'with factory defaults', factory_default: :keep do - let_it_be(:namespace) { create_default(:namespace) } - let_it_be(:project) { create_default(:project, :repository) } - let_it_be(:user) { create_default(:user) } + let_it_be(:namespace) { create_default(:namespace).freeze } + let_it_be(:project) { create_default(:project, :repository).freeze } + let_it_be(:user) { create_default(:user).freeze } before do factories_based_on_view.each do |factory| diff --git a/spec/features/issues/gfm_autocomplete_spec.rb b/spec/features/issues/gfm_autocomplete_spec.rb index e2087868035..eb3d0f223db 100644 --- a/spec/features/issues/gfm_autocomplete_spec.rb +++ b/spec/features/issues/gfm_autocomplete_spec.rb @@ -994,7 +994,7 @@ RSpec.describe 'GFM autocomplete', :js do end def start_and_cancel_discussion - click_button('Reply...') + find_field('Reply…').click fill_in('note_note', with: 'Whoops!') diff --git a/spec/features/merge_request/batch_comments_spec.rb b/spec/features/merge_request/batch_comments_spec.rb index c8fc23bebf9..25f2707146d 100644 --- a/spec/features/merge_request/batch_comments_spec.rb +++ b/spec/features/merge_request/batch_comments_spec.rb @@ -223,7 +223,7 @@ end def write_reply_to_discussion(button_text: 'Start a review', text: 'Line is wrong', resolve: false, unresolve: false) page.within(first('.diff-files-holder .discussion-reply-holder')) do - click_button('Reply...') + find_field('Reply…', match: :first).click fill_in('note_note', with: text) diff --git a/spec/features/merge_request/user_posts_diff_notes_spec.rb b/spec/features/merge_request/user_posts_diff_notes_spec.rb index 794dfd7c8da..163ce10132e 100644 --- a/spec/features/merge_request/user_posts_diff_notes_spec.rb +++ b/spec/features/merge_request/user_posts_diff_notes_spec.rb @@ -192,7 +192,7 @@ RSpec.describe 'Merge request > User posts diff notes', :js do it 'adds as discussion' do should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]'), asset_form_reset: false) expect(page).to have_css('.notes_holder .note.note-discussion', count: 1) - expect(page).to have_button('Reply...') + expect(page).to have_field('Reply…') end end end diff --git a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb index b86586d53e2..caa04059469 100644 --- a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb +++ b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb @@ -149,7 +149,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do it 'allows user to comment' do page.within '.diff-content' do - click_button 'Reply...' + find_field('Reply…').click find(".js-unresolve-checkbox").set false find('.js-note-text').set 'testing' @@ -179,7 +179,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do it 'allows user to comment & unresolve thread' do page.within '.diff-content' do - click_button 'Reply...' + find_field('Reply…').click find('.js-note-text').set 'testing' @@ -208,7 +208,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do it 'allows user to comment & resolve thread' do page.within '.diff-content' do - click_button 'Reply...' + find_field('Reply…').click find('.js-note-text').set 'testing' @@ -442,7 +442,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do it 'allows user to comment & resolve thread' do page.within '.diff-content' do - click_button 'Reply...' + find_field('Reply…').click find('.js-note-text').set 'testing' @@ -461,7 +461,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do page.within '.diff-content' do click_button 'Resolve thread' - click_button 'Reply...' + find_field('Reply…').click find('.js-note-text').set 'testing' diff --git a/spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb b/spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb index d15d5b3bc73..90cdc28d1bd 100644 --- a/spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb +++ b/spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb @@ -37,7 +37,7 @@ RSpec.describe 'Merge request > User sees avatars on diff notes', :js do end it 'does not render avatars after commenting on discussion tab' do - click_button 'Reply...' + find_field('Reply…').click page.within('.js-discussion-note-form') do find('.note-textarea').native.send_keys('Test comment') @@ -132,7 +132,7 @@ RSpec.describe 'Merge request > User sees avatars on diff notes', :js do end it 'adds avatar when commenting' do - click_button 'Reply...' + find_field('Reply…', match: :first).click page.within '.js-discussion-note-form' do find('.js-note-text').native.send_keys('Test') @@ -151,7 +151,7 @@ RSpec.describe 'Merge request > User sees avatars on diff notes', :js do it 'adds multiple comments' do 3.times do - click_button 'Reply...' + find_field('Reply…', match: :first).click page.within '.js-discussion-note-form' do find('.js-note-text').native.send_keys('Test') diff --git a/spec/features/merge_request/user_sees_discussions_spec.rb b/spec/features/merge_request/user_sees_discussions_spec.rb index 289c861739f..d79763ba5e0 100644 --- a/spec/features/merge_request/user_sees_discussions_spec.rb +++ b/spec/features/merge_request/user_sees_discussions_spec.rb @@ -60,7 +60,7 @@ RSpec.describe 'Merge request > User sees threads', :js do it 'can be replied to' do within(".discussion[data-discussion-id='#{discussion_id}']") do - click_button 'Reply...' + find_field('Reply…').click fill_in 'note[note]', with: 'Test!' click_button 'Comment' diff --git a/spec/features/merge_request/user_sees_notes_from_forked_project_spec.rb b/spec/features/merge_request/user_sees_notes_from_forked_project_spec.rb index 20c45a1d652..ea46ae06329 100644 --- a/spec/features/merge_request/user_sees_notes_from_forked_project_spec.rb +++ b/spec/features/merge_request/user_sees_notes_from_forked_project_spec.rb @@ -27,7 +27,7 @@ RSpec.describe 'Merge request > User sees notes from forked project', :js do expect(page).to have_content('A commit comment') page.within('.discussion-notes') do - find('.btn-text-field').click + find_field('Reply…').click scroll_to(page.find('#note_note', visible: false)) find('#note_note').send_keys('A reply comment') find('.js-comment-button').click diff --git a/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb b/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb index bbeb91bbd19..dbc88d0cce2 100644 --- a/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb +++ b/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb @@ -83,7 +83,7 @@ RSpec.describe 'User comments on a diff', :js do wait_for_requests - click_button 'Reply...' + find_field('Reply…', match: :first).click find('.js-suggestion-btn').click diff --git a/spec/frontend/design_management/components/design_notes/design_discussion_spec.js b/spec/frontend/design_management/components/design_notes/design_discussion_spec.js index 92e188f4bcc..efadb9b717d 100644 --- a/spec/frontend/design_management/components/design_notes/design_discussion_spec.js +++ b/spec/frontend/design_management/components/design_notes/design_discussion_spec.js @@ -93,7 +93,7 @@ describe('Design discussions component', () => { }); it('does not render a checkbox in reply form', () => { - findReplyPlaceholder().vm.$emit('onClick'); + findReplyPlaceholder().vm.$emit('focus'); return wrapper.vm.$nextTick().then(() => { expect(findResolveCheckbox().exists()).toBe(false); @@ -124,7 +124,7 @@ describe('Design discussions component', () => { }); it('renders a checkbox with Resolve thread text in reply form', () => { - findReplyPlaceholder().vm.$emit('onClick'); + findReplyPlaceholder().vm.$emit('focus'); wrapper.setProps({ discussionWithOpenForm: defaultMockDiscussion.id }); return wrapper.vm.$nextTick().then(() => { @@ -193,7 +193,7 @@ describe('Design discussions component', () => { }); it('renders a checkbox with Unresolve thread text in reply form', () => { - findReplyPlaceholder().vm.$emit('onClick'); + findReplyPlaceholder().vm.$emit('focus'); wrapper.setProps({ discussionWithOpenForm: defaultMockDiscussion.id }); return wrapper.vm.$nextTick().then(() => { @@ -205,7 +205,7 @@ describe('Design discussions component', () => { it('hides reply placeholder and opens form on placeholder click', () => { createComponent(); - findReplyPlaceholder().vm.$emit('onClick'); + findReplyPlaceholder().vm.$emit('focus'); wrapper.setProps({ discussionWithOpenForm: defaultMockDiscussion.id }); return wrapper.vm.$nextTick().then(() => { @@ -307,7 +307,7 @@ describe('Design discussions component', () => { it('emits openForm event on opening the form', () => { createComponent(); - findReplyPlaceholder().vm.$emit('onClick'); + findReplyPlaceholder().vm.$emit('focus'); expect(wrapper.emitted('open-form')).toBeTruthy(); }); diff --git a/spec/frontend/notes/components/discussion_actions_spec.js b/spec/frontend/notes/components/discussion_actions_spec.js index 03e5842bb0f..c6a7d7ead98 100644 --- a/spec/frontend/notes/components/discussion_actions_spec.js +++ b/spec/frontend/notes/components/discussion_actions_spec.js @@ -96,7 +96,7 @@ describe('DiscussionActions', () => { it('emits showReplyForm event when clicking on reply placeholder', () => { jest.spyOn(wrapper.vm, '$emit'); - wrapper.find(ReplyPlaceholder).find('button').trigger('click'); + wrapper.find(ReplyPlaceholder).find('textarea').trigger('focus'); expect(wrapper.vm.$emit).toHaveBeenCalledWith('showReplyForm'); }); diff --git a/spec/frontend/notes/components/discussion_reply_placeholder_spec.js b/spec/frontend/notes/components/discussion_reply_placeholder_spec.js index b7b7ec08867..2a4cd0df0c7 100644 --- a/spec/frontend/notes/components/discussion_reply_placeholder_spec.js +++ b/spec/frontend/notes/components/discussion_reply_placeholder_spec.js @@ -1,17 +1,17 @@ import { shallowMount } from '@vue/test-utils'; import ReplyPlaceholder from '~/notes/components/discussion_reply_placeholder.vue'; -const buttonText = 'Test Button Text'; +const placeholderText = 'Test Button Text'; describe('ReplyPlaceholder', () => { let wrapper; - const findButton = () => wrapper.find({ ref: 'button' }); + const findTextarea = () => wrapper.find({ ref: 'textarea' }); beforeEach(() => { wrapper = shallowMount(ReplyPlaceholder, { propsData: { - buttonText, + placeholderText, }, }); }); @@ -20,17 +20,17 @@ describe('ReplyPlaceholder', () => { wrapper.destroy(); }); - it('emits onClick event on button click', () => { - findButton().trigger('click'); + it('emits focus event on button click', () => { + findTextarea().trigger('focus'); return wrapper.vm.$nextTick().then(() => { expect(wrapper.emitted()).toEqual({ - onClick: [[]], + focus: [[]], }); }); }); it('should render reply button', () => { - expect(findButton().text()).toEqual(buttonText); + expect(findTextarea().attributes('placeholder')).toEqual(placeholderText); }); }); diff --git a/spec/frontend/notes/components/noteable_discussion_spec.js b/spec/frontend/notes/components/noteable_discussion_spec.js index 87538279c3d..34df39bf1c7 100644 --- a/spec/frontend/notes/components/noteable_discussion_spec.js +++ b/spec/frontend/notes/components/noteable_discussion_spec.js @@ -65,7 +65,7 @@ describe('noteable_discussion component', () => { expect(wrapper.vm.isReplying).toEqual(false); const replyPlaceholder = wrapper.find(ReplyPlaceholder); - replyPlaceholder.vm.$emit('onClick'); + replyPlaceholder.vm.$emit('focus'); await nextTick(); expect(wrapper.vm.isReplying).toEqual(true); diff --git a/spec/lib/gitlab/ci/trace_spec.rb b/spec/lib/gitlab/ci/trace_spec.rb index 92bf2519588..597e4ca9b03 100644 --- a/spec/lib/gitlab/ci/trace_spec.rb +++ b/spec/lib/gitlab/ci/trace_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Gitlab::Ci::Trace, :clean_gitlab_redis_shared_state, factory_default: :keep do - let_it_be(:project) { create_default(:project) } + let_it_be(:project) { create_default(:project).freeze } let_it_be_with_reload(:build) { create(:ci_build) } let(:trace) { described_class.new(build) } diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 94943fb3644..351b9f0554c 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -8,8 +8,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do include Ci::SourcePipelineHelpers let_it_be(:user) { create(:user) } - let_it_be(:namespace) { create_default(:namespace) } - let_it_be(:project) { create_default(:project, :repository) } + let_it_be(:namespace) { create_default(:namespace).freeze } + let_it_be(:project) { create_default(:project, :repository).freeze } let(:pipeline) do create(:ci_empty_pipeline, status: :created, project: project) @@ -3785,17 +3785,11 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#default_branch?' do - let(:default_branch) { 'master'} - subject { pipeline.default_branch? } - before do - allow(project).to receive(:default_branch).and_return(default_branch) - end - context 'when pipeline ref is the default branch of the project' do let(:pipeline) do - build(:ci_empty_pipeline, status: :created, project: project, ref: default_branch) + build(:ci_empty_pipeline, status: :created, project: project, ref: project.default_branch) end it "returns true" do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index ebe2cd2ac03..cbb20de75b0 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -9,8 +9,8 @@ RSpec.describe MergeRequest, factory_default: :keep do using RSpec::Parameterized::TableSyntax - let_it_be(:namespace) { create_default(:namespace) } - let_it_be(:project, refind: true) { create_default(:project, :repository) } + let_it_be(:namespace) { create_default(:namespace).freeze } + let_it_be(:project, refind: true) { create_default(:project, :repository).freeze } subject { create(:merge_request) } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index fd7975bf65d..c33c6c05c24 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -8,7 +8,7 @@ RSpec.describe Project, factory_default: :keep do include ExternalAuthorizationServiceHelpers using RSpec::Parameterized::TableSyntax - let_it_be(:namespace) { create_default(:namespace) } + let_it_be(:namespace) { create_default(:namespace).freeze } it_behaves_like 'having unique enum values' @@ -1799,7 +1799,8 @@ RSpec.describe Project, factory_default: :keep do describe '#default_branch_protected?' do using RSpec::Parameterized::TableSyntax - let_it_be(:project) { create(:project) } + let_it_be(:namespace) { create(:namespace) } + let_it_be(:project) { create(:project, namespace: namespace) } subject { project.default_branch_protected? } @@ -2802,7 +2803,8 @@ RSpec.describe Project, factory_default: :keep do end describe '#emails_disabled?' do - let(:project) { build(:project, emails_disabled: false) } + let_it_be(:namespace) { create(:namespace) } + let(:project) { build(:project, namespace: namespace, emails_disabled: false) } context 'emails disabled in group' do it 'returns true' do @@ -2830,7 +2832,8 @@ RSpec.describe Project, factory_default: :keep do end describe '#lfs_enabled?' do - let(:project) { build(:project) } + let(:namespace) { create(:namespace) } + let(:project) { build(:project, namespace: namespace) } shared_examples 'project overrides group' do it 'returns true when enabled in project' do @@ -4877,7 +4880,8 @@ RSpec.describe Project, factory_default: :keep do end context 'branch protection' do - let(:project) { create(:project, :repository) } + let_it_be(:namespace) { create(:namespace) } + let(:project) { create(:project, :repository, namespace: namespace) } before do create(:import_state, :started, project: project) diff --git a/spec/workers/jira_connect/sync_project_worker_spec.rb b/spec/workers/jira_connect/sync_project_worker_spec.rb index f7fa565d534..04cc3bec3af 100644 --- a/spec/workers/jira_connect/sync_project_worker_spec.rb +++ b/spec/workers/jira_connect/sync_project_worker_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe JiraConnect::SyncProjectWorker, factory_default: :keep do describe '#perform' do - let_it_be(:project) { create_default(:project) } + let_it_be(:project) { create_default(:project).freeze } let!(:mr_with_jira_title) { create(:merge_request, :unique_branches, title: 'TEST-123') } let!(:mr_with_jira_description) { create(:merge_request, :unique_branches, description: 'TEST-323') } let!(:mr_with_other_title) { create(:merge_request, :unique_branches) } |