diff options
34 files changed, 363 insertions, 331 deletions
diff --git a/GITLAB_PAGES_VERSION b/GITLAB_PAGES_VERSION index 39fc130ef85..bf50e910e62 100644 --- a/GITLAB_PAGES_VERSION +++ b/GITLAB_PAGES_VERSION @@ -1 +1 @@ -1.36.0 +1.37.0 diff --git a/app/assets/javascripts/diffs/components/commit_item.vue b/app/assets/javascripts/diffs/components/commit_item.vue index 92b317eb3f0..bc0f2fb0b69 100644 --- a/app/assets/javascripts/diffs/components/commit_item.vue +++ b/app/assets/javascripts/diffs/components/commit_item.vue @@ -1,7 +1,6 @@ <script> /* eslint-disable vue/no-v-html */ -import { GlButtonGroup, GlButton, GlTooltipDirective, GlIcon } from '@gitlab/ui'; -import { mapActions } from 'vuex'; +import { GlButtonGroup, GlButton, GlTooltipDirective } from '@gitlab/ui'; import CommitPipelineStatus from '~/projects/tree/components/commit_pipeline_status_component.vue'; import ModalCopyButton from '~/vue_shared/components/modal_copy_button.vue'; @@ -9,7 +8,6 @@ import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; import UserAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; -import { setUrlParams } from '../../lib/utils/url_utility'; import initUserPopovers from '../../user_popovers'; /** @@ -24,14 +22,6 @@ import initUserPopovers from '../../user_popovers'; * coexist, but there is an issue to remove the duplication. * https://gitlab.com/gitlab-org/gitlab-foss/issues/51613 * - * EXCEPTION WARNING - * 1. The commit navigation buttons (next neighbor, previous neighbor) - * are not duplicated because: - * - We don't have the same data available on the Rails side (yet, - * without backend work) - * - This Vue component should always be what's used when in the - * context of an MR diff, so the HAML should never have any idea - * about navigating among commits. */ export default { @@ -42,7 +32,6 @@ export default { CommitPipelineStatus, GlButtonGroup, GlButton, - GlIcon, }, directives: { GlTooltip: GlTooltipDirective, @@ -94,28 +83,12 @@ export default { // Strip the newline at the beginning return this.commit.description_html.replace(/^
/, ''); }, - nextCommitUrl() { - return this.commit.next_commit_id - ? setUrlParams({ commit_id: this.commit.next_commit_id }) - : ''; - }, - previousCommitUrl() { - return this.commit.prev_commit_id - ? setUrlParams({ commit_id: this.commit.prev_commit_id }) - : ''; - }, - hasNeighborCommits() { - return this.commit.next_commit_id || this.commit.prev_commit_id; - }, }, created() { this.$nextTick(() => { initUserPopovers(this.$el.querySelectorAll('.js-user-link')); }); }, - methods: { - ...mapActions('diffs', ['moveToNeighboringCommit']), - }, }; </script> @@ -146,38 +119,6 @@ export default { class="input-group-text" /> </gl-button-group> - <div v-if="hasNeighborCommits" class="commit-nav-buttons ml-3"> - <gl-button-group> - <gl-button - :href="previousCommitUrl" - :disabled="!commit.prev_commit_id" - @click.prevent="moveToNeighboringCommit({ direction: 'previous' })" - > - <span - v-if="!commit.prev_commit_id" - v-gl-tooltip - class="h-100 w-100 position-absolute" - :title="__('You\'re at the first commit')" - ></span> - <gl-icon name="chevron-left" /> - {{ __('Prev') }} - </gl-button> - <gl-button - :href="nextCommitUrl" - :disabled="!commit.next_commit_id" - @click.prevent="moveToNeighboringCommit({ direction: 'next' })" - > - <span - v-if="!commit.next_commit_id" - v-gl-tooltip - class="h-100 w-100 position-absolute" - :title="__('You\'re at the last commit')" - ></span> - {{ __('Next') }} - <gl-icon name="chevron-right" /> - </gl-button> - </gl-button-group> - </div> </div> <div> <div class="d-flex float-left align-items-center align-self-start"> diff --git a/app/assets/javascripts/diffs/components/compare_versions.vue b/app/assets/javascripts/diffs/components/compare_versions.vue index 3fb9787ac30..7526c5347f7 100644 --- a/app/assets/javascripts/diffs/components/compare_versions.vue +++ b/app/assets/javascripts/diffs/components/compare_versions.vue @@ -1,7 +1,8 @@ <script> -import { GlTooltipDirective, GlLink, GlButton, GlSprintf } from '@gitlab/ui'; +import { GlTooltipDirective, GlIcon, GlLink, GlButtonGroup, GlButton, GlSprintf } from '@gitlab/ui'; import { mapActions, mapGetters, mapState } from 'vuex'; import { __ } from '~/locale'; +import { setUrlParams } from '../../lib/utils/url_utility'; import { CENTERED_LIMITED_CONTAINER_CLASSES, EVT_EXPAND_ALL_FILES } from '../constants'; import eventHub from '../event_hub'; import CompareDropdownLayout from './compare_dropdown_layout.vue'; @@ -11,7 +12,9 @@ import SettingsDropdown from './settings_dropdown.vue'; export default { components: { CompareDropdownLayout, + GlIcon, GlLink, + GlButtonGroup, GlButton, GlSprintf, SettingsDropdown, @@ -56,6 +59,19 @@ export default { hasSourceVersions() { return this.diffCompareDropdownSourceVersions.length > 0; }, + nextCommitUrl() { + return this.commit.next_commit_id + ? setUrlParams({ commit_id: this.commit.next_commit_id }) + : ''; + }, + previousCommitUrl() { + return this.commit.prev_commit_id + ? setUrlParams({ commit_id: this.commit.prev_commit_id }) + : ''; + }, + hasNeighborCommits() { + return this.commit && (this.commit.next_commit_id || this.commit.prev_commit_id); + }, }, created() { this.CENTERED_LIMITED_CONTAINER_CLASSES = CENTERED_LIMITED_CONTAINER_CLASSES; @@ -65,6 +81,7 @@ export default { expandAllFiles() { eventHub.$emit(EVT_EXPAND_ALL_FILES); }, + ...mapActions('diffs', ['moveToNeighboringCommit']), }, }; </script> @@ -92,6 +109,38 @@ export default { {{ __('Viewing commit') }} <gl-link :href="commit.commit_url" class="monospace">{{ commit.short_id }}</gl-link> </div> + <div v-if="hasNeighborCommits" class="commit-nav-buttons ml-3"> + <gl-button-group> + <gl-button + :href="previousCommitUrl" + :disabled="!commit.prev_commit_id" + @click.prevent="moveToNeighboringCommit({ direction: 'previous' })" + > + <span + v-if="!commit.prev_commit_id" + v-gl-tooltip + class="h-100 w-100 position-absolute position-top-0 position-left-0" + :title="__('You\'re at the first commit')" + ></span> + <gl-icon name="chevron-left" /> + {{ __('Prev') }} + </gl-button> + <gl-button + :href="nextCommitUrl" + :disabled="!commit.next_commit_id" + @click.prevent="moveToNeighboringCommit({ direction: 'next' })" + > + <span + v-if="!commit.next_commit_id" + v-gl-tooltip + class="h-100 w-100 position-absolute position-top-0 position-left-0" + :title="__('You\'re at the last commit')" + ></span> + {{ __('Next') }} + <gl-icon name="chevron-right" /> + </gl-button> + </gl-button-group> + </div> <gl-sprintf v-else-if="hasSourceVersions" class="d-flex align-items-center compare-versions-container" diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index e51cc0b4479..bcb950a2b87 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -115,11 +115,6 @@ font-size: $gl-font-size; line-height: $gl-font-size-large; } - - .home-panel-topic-list, - .home-panel-metadata { - font-size: $gl-font-size-small; - } } } diff --git a/app/controllers/projects/merge_requests/creations_controller.rb b/app/controllers/projects/merge_requests/creations_controller.rb index 43ecb7f8492..c339f06c227 100644 --- a/app/controllers/projects/merge_requests/creations_controller.rb +++ b/app/controllers/projects/merge_requests/creations_controller.rb @@ -6,7 +6,6 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap include RendersCommits skip_before_action :merge_request - before_action :disable_query_limiting, only: [:create] before_action :authorize_create_merge_request_from! before_action :apply_diff_view_cookie!, only: [:diffs, :diff_for_path] before_action :build_merge_request, except: [:create] @@ -133,13 +132,11 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap end # rubocop: enable CodeReuse/ActiveRecord - def disable_query_limiting - Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/20801') - end - def incr_count_webide_merge_request return if params[:nav_source] != 'webide' Gitlab::UsageDataCounters::WebIdeCounter.increment_merge_requests_count end end + +Projects::MergeRequests::CreationsController.prepend_ee_mod diff --git a/app/services/merge_requests/after_create_service.rb b/app/services/merge_requests/after_create_service.rb index b22afe8a20d..92525ff92ef 100644 --- a/app/services/merge_requests/after_create_service.rb +++ b/app/services/merge_requests/after_create_service.rb @@ -24,6 +24,16 @@ module MergeRequests merge_request.create_cross_references!(current_user) OnboardingProgressService.new(merge_request.target_project.namespace).execute(action: :merge_request_created) + + todo_service.new_merge_request(merge_request, current_user) + merge_request.cache_merge_request_closes_issues!(current_user) + + Gitlab::UsageDataCounters::MergeRequestCounter.count(:create) + link_lfs_objects(merge_request) + end + + def link_lfs_objects(merge_request) + LinkLfsObjectsService.new(merge_request.target_project).execute(merge_request) end end end diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb index ac84a13f437..f685cc03819 100644 --- a/app/services/merge_requests/create_service.rb +++ b/app/services/merge_requests/create_service.rb @@ -18,12 +18,6 @@ module MergeRequests # be performed in Sidekiq NewMergeRequestWorker.perform_async(issuable.id, current_user.id) - todo_service.new_merge_request(issuable, current_user) - issuable.cache_merge_request_closes_issues!(current_user) - - Gitlab::UsageDataCounters::MergeRequestCounter.count(:create) - link_lfs_objects(issuable) - super end @@ -54,10 +48,6 @@ module MergeRequests raise Gitlab::Access::AccessDeniedError end end - - def link_lfs_objects(issuable) - LinkLfsObjectsService.new(issuable.target_project).execute(issuable) - end end end diff --git a/app/views/layouts/nav/sidebar/_group.html.haml b/app/views/layouts/nav/sidebar/_group.html.haml index 48187f661a7..c835bbc6055 100644 --- a/app/views/layouts/nav/sidebar/_group.html.haml +++ b/app/views/layouts/nav/sidebar/_group.html.haml @@ -91,13 +91,13 @@ .nav-icon-container = sprite_icon('git-merge') %span.nav-item-name - = _('Merge Requests') + = _('Merge requests') %span.badge.badge-pill.count= merge_requests_count %ul.sidebar-sub-level-items.is-fly-out-only = nav_link(path: 'groups#merge_requests', html_options: { class: "fly-out-top-item" } ) do = link_to merge_requests_group_path(@group) do %strong.fly-out-top-item-name - = _('Merge Requests') + = _('Merge requests') %span.badge.badge-pill.count.merge_counter.js-merge-counter.fly-out-badge= merge_requests_count = render_if_exists "layouts/nav/ee/security_link" # EE-specific diff --git a/app/views/layouts/nav/sidebar/_project.html.haml b/app/views/layouts/nav/sidebar/_project.html.haml index 6b9cf644044..022bbd39723 100644 --- a/app/views/layouts/nav/sidebar/_project.html.haml +++ b/app/views/layouts/nav/sidebar/_project.html.haml @@ -160,14 +160,14 @@ .nav-icon-container = sprite_icon('git-merge') %span.nav-item-name#js-onboarding-mr-link - = _('Merge Requests') + = _('Merge requests') %span.badge.badge-pill.count.merge_counter.js-merge-counter = number_with_delimiter(@project.open_merge_requests_count) %ul.sidebar-sub-level-items.is-fly-out-only = nav_link(controller: :merge_requests, html_options: { class: "fly-out-top-item" } ) do = link_to project_merge_requests_path(@project) do %strong.fly-out-top-item-name - = _('Merge Requests') + = _('Merge requests') %span.badge.badge-pill.count.merge_counter.js-merge-counter.fly-out-badge = number_with_delimiter(@project.open_merge_requests_count) diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml index a2d24feaeec..b2380a3ba57 100644 --- a/app/views/projects/_home_panel.html.haml +++ b/app/views/projects/_home_panel.html.haml @@ -6,18 +6,18 @@ .project-home-panel.js-show-on-project-root.gl-my-5{ class: [("empty-project" if empty_repo)] } .gl-display-flex.gl-justify-content-space-between.gl-flex-wrap.gl-sm-flex-direction-column.gl-mb-3 .home-panel-title-row.gl-display-flex - .avatar-container.rect-avatar.s64.home-panel-avatar.gl-flex-shrink-0.gl-w-11.gl-h-11.gl-mr-3.float-none + %div{ class: 'avatar-container rect-avatar s64 home-panel-avatar gl-flex-shrink-0 gl-w-11 gl-h-11 gl-mr-3! float-none' } = project_icon(@project, alt: @project.name, class: 'avatar avatar-tile s64', width: 64, height: 64, itemprop: 'image') .d-flex.flex-column.flex-wrap.align-items-baseline .d-inline-flex.align-items-baseline - %h1.home-panel-title.gl-mt-3.gl-mb-2.gl-font-size-h1.gl-line-height-24.gl-font-weight-bold{ data: { qa_selector: 'project_name_content' }, itemprop: 'name' } + %h1.home-panel-title.gl-mt-3.gl-mb-2.gl-font-size-h1.gl-line-height-24.gl-font-weight-bold.gl-ml-3{ data: { qa_selector: 'project_name_content' }, itemprop: 'name' } = @project.name %span.visibility-icon.text-secondary.gl-ml-2.has-tooltip{ data: { container: 'body' }, title: visibility_icon_description(@project) } = visibility_level_icon(@project.visibility_level, options: { class: 'icon' }) = render_if_exists 'compliance_management/compliance_framework/compliance_framework_badge', project: @project .home-panel-metadata.d-flex.flex-wrap.text-secondary.gl-font-base.gl-font-weight-normal.gl-line-height-normal - if can?(current_user, :read_project, @project) - - button_class = "btn-clipboard btn-transparent btn-no-padding gl-font-base gl-font-weight-normal gl-line-height-normal home-panel-metadata" + - button_class = "btn gl-button btn-sm btn-tertiary btn-default-tertiary home-panel-metadata" - button_text = s_('ProjectPage|Project ID: %{project_id}') % { project_id: @project.id } = clipboard_button(title: s_('ProjectPage|Copy project ID'), text: @project.id, hide_button_icon: true, button_text: button_text, class: button_class, qa_selector: 'project_id_content', itemprop: 'identifier') - if current_user diff --git a/changelogs/unreleased/21121-fj-fix-n-1-projects-endpoint-forked-projects.yml b/changelogs/unreleased/21121-fj-fix-n-1-projects-endpoint-forked-projects.yml new file mode 100644 index 00000000000..3b202fe4bfc --- /dev/null +++ b/changelogs/unreleased/21121-fj-fix-n-1-projects-endpoint-forked-projects.yml @@ -0,0 +1,5 @@ +--- +title: Fix N+1 in projects REST endpoint with forked projects +merge_request: 57798 +author: +type: performance diff --git a/changelogs/unreleased/mk-speed-up-merge-request-creation-action.yml b/changelogs/unreleased/mk-speed-up-merge-request-creation-action.yml new file mode 100644 index 00000000000..80cca000a3e --- /dev/null +++ b/changelogs/unreleased/mk-speed-up-merge-request-creation-action.yml @@ -0,0 +1,6 @@ +--- +title: Perform more merge request creation tasks asynchronously to improve response + times +merge_request: 57453 +author: +type: performance diff --git a/changelogs/unreleased/project-overview-copy-id.yml b/changelogs/unreleased/project-overview-copy-id.yml new file mode 100644 index 00000000000..0b23d39e8ff --- /dev/null +++ b/changelogs/unreleased/project-overview-copy-id.yml @@ -0,0 +1,5 @@ +--- +title: Utilize btn-tertiary for copy project id on project overview +merge_request: 57766 +author: +type: changed diff --git a/changelogs/unreleased/release-pages-1-37-0.yml b/changelogs/unreleased/release-pages-1-37-0.yml new file mode 100644 index 00000000000..3cec0336d66 --- /dev/null +++ b/changelogs/unreleased/release-pages-1-37-0.yml @@ -0,0 +1,5 @@ +--- +title: Upgrade GitLab Pages to v1.37.0 +merge_request: 57946 +author: +type: added diff --git a/changelogs/unreleased/tc-move-commit-buttons.yml b/changelogs/unreleased/tc-move-commit-buttons.yml new file mode 100644 index 00000000000..52f13e85e83 --- /dev/null +++ b/changelogs/unreleased/tc-move-commit-buttons.yml @@ -0,0 +1,5 @@ +--- +title: Move commit neighbor buttons to sticky MR controls +merge_request: 57743 +author: +type: changed diff --git a/doc/user/project/merge_requests/img/commit_nav_v13_11.png b/doc/user/project/merge_requests/img/commit_nav_v13_11.png Binary files differnew file mode 100644 index 00000000000..3d85cb87df4 --- /dev/null +++ b/doc/user/project/merge_requests/img/commit_nav_v13_11.png diff --git a/doc/user/project/merge_requests/img/commit_nav_v13_4.png b/doc/user/project/merge_requests/img/commit_nav_v13_4.png Binary files differdeleted file mode 100644 index 0ae6ce32693..00000000000 --- a/doc/user/project/merge_requests/img/commit_nav_v13_4.png +++ /dev/null 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 89b4c999b06..674660d221f 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 @@ -145,10 +145,10 @@ To seamlessly navigate among commits in a merge request: 1. Select a commit to open it in the single-commit view. 1. Navigate through the commits by either: - - Selecting **Prev** and **Next** buttons on the top-right of the page. + - Selecting **Prev** and **Next** buttons right beneath the tab buttons. - Using the <kbd>X</kbd> and <kbd>C</kbd> keyboard shortcuts. - + ### Incrementally expand merge request diffs diff --git a/lib/api/entities/basic_project_details.rb b/lib/api/entities/basic_project_details.rb index cf0b32bed26..2de49d6ed40 100644 --- a/lib/api/entities/basic_project_details.rb +++ b/lib/api/entities/basic_project_details.rb @@ -8,11 +8,10 @@ module API expose :default_branch, if: -> (project, options) { Ability.allowed?(options[:current_user], :download_code, project) } # Avoids an N+1 query: https://github.com/mbleigh/acts-as-taggable-on/issues/91#issuecomment-168273770 expose :tag_list do |project| - # project.tags.order(:name).pluck(:name) is the most suitable option - # to avoid loading all the ActiveRecord objects but, if we use it here - # it override the preloaded associations and makes a query - # (fixed in https://github.com/rails/rails/pull/25976). - project.tags.map(&:name).sort + # Tags is a preloaded association. If we perform then sorting + # through the database, it will trigger a new query, ending up + # in an N+1 if we have several projects + project.tags.pluck(:name).sort # rubocop:disable CodeReuse/ActiveRecord end expose :ssh_url_to_repo, :http_url_to_repo, :web_url, :readme_url diff --git a/lib/api/entities/project.rb b/lib/api/entities/project.rb index 161cdb68170..b159effdde7 100644 --- a/lib/api/entities/project.rb +++ b/lib/api/entities/project.rb @@ -135,7 +135,7 @@ module API .preload(project_group_links: { group: :route }, fork_network: :root_project, fork_network_member: :forked_from_project, - forked_from_project: [:route, :forks, :tags, namespace: :route]) + forked_from_project: [:route, :forks, :tags, :group, :project_feature, namespace: [:route, :owner]]) end # rubocop: enable CodeReuse/ActiveRecord diff --git a/spec/features/groups/group_page_with_external_authorization_service_spec.rb b/spec/features/groups/group_page_with_external_authorization_service_spec.rb index 8ef1b60d8ca..187d878472e 100644 --- a/spec/features/groups/group_page_with_external_authorization_service_spec.rb +++ b/spec/features/groups/group_page_with_external_authorization_service_spec.rb @@ -19,7 +19,7 @@ RSpec.describe 'The group page' do expect(page).to have_link('Details') expect(page).to have_link('Activity') expect(page).to have_link('Issues') - expect(page).to have_link('Merge Requests') + expect(page).to have_link('Merge requests') expect(page).to have_link('Members') end end @@ -50,7 +50,7 @@ RSpec.describe 'The group page' do expect(page).not_to have_link('Contribution') expect(page).not_to have_link('Issues') - expect(page).not_to have_link('Merge Requests') + expect(page).not_to have_link('Merge requests') expect(page).to have_link('Members') end end diff --git a/spec/features/groups/merge_requests_spec.rb b/spec/features/groups/merge_requests_spec.rb index 43d4b6b23e0..f79c93157dc 100644 --- a/spec/features/groups/merge_requests_spec.rb +++ b/spec/features/groups/merge_requests_spec.rb @@ -27,7 +27,7 @@ RSpec.describe 'Group merge requests page' do end it 'ignores archived merge request count badges in navbar' do - expect(first(:link, text: 'Merge Requests').find('.badge').text).to eq("1") + expect(first(:link, text: 'Merge requests').find('.badge').text).to eq("1") end it 'ignores archived merge request count badges in state-filters' do diff --git a/spec/features/groups/navbar_spec.rb b/spec/features/groups/navbar_spec.rb index 7025874a4ff..710755ce306 100644 --- a/spec/features/groups/navbar_spec.rb +++ b/spec/features/groups/navbar_spec.rb @@ -30,7 +30,7 @@ RSpec.describe 'Group navbar' do ] }, { - nav_item: _('Merge Requests'), + nav_item: _('Merge requests'), nav_sub_items: [] }, (security_and_compliance_nav_item if Gitlab.ee?), diff --git a/spec/features/projects/active_tabs_spec.rb b/spec/features/projects/active_tabs_spec.rb index 86fe59f003f..9de43e7d18c 100644 --- a/spec/features/projects/active_tabs_spec.rb +++ b/spec/features/projects/active_tabs_spec.rb @@ -79,7 +79,7 @@ RSpec.describe 'Project active tab' do visit project_merge_requests_path(project) end - it_behaves_like 'page has active tab', 'Merge Requests' + it_behaves_like 'page has active tab', 'Merge requests' end context 'on project Wiki' do diff --git a/spec/features/projects/fork_spec.rb b/spec/features/projects/fork_spec.rb index 81751d2123a..f82dddbe38d 100644 --- a/spec/features/projects/fork_spec.rb +++ b/spec/features/projects/fork_spec.rb @@ -208,7 +208,7 @@ RSpec.describe 'Project fork' do expect(page).to have_content(/new merge request/i) page.within '.nav-sidebar' do - first(:link, 'Merge Requests').click + first(:link, 'Merge requests').click end expect(page).to have_content(/new merge request/i) diff --git a/spec/features/projects/merge_request_button_spec.rb b/spec/features/projects/merge_request_button_spec.rb index 9547ba8a390..93bbabcc3f8 100644 --- a/spec/features/projects/merge_request_button_spec.rb +++ b/spec/features/projects/merge_request_button_spec.rb @@ -14,7 +14,9 @@ RSpec.describe 'Merge Request button' do it 'does not show Create merge request button' do visit url - expect(page).not_to have_link(label) + within '.content-wrapper' do + expect(page).not_to have_link(label) + end end end diff --git a/spec/features/projects/user_uses_shortcuts_spec.rb b/spec/features/projects/user_uses_shortcuts_spec.rb index f97c8d820e3..b6fde19e0d4 100644 --- a/spec/features/projects/user_uses_shortcuts_spec.rb +++ b/spec/features/projects/user_uses_shortcuts_spec.rb @@ -151,7 +151,7 @@ RSpec.describe 'User uses shortcuts', :js do find('body').native.send_key('g') find('body').native.send_key('m') - expect(page).to have_active_navigation('Merge Requests') + expect(page).to have_active_navigation('Merge requests') end end diff --git a/spec/frontend/diffs/components/commit_item_spec.js b/spec/frontend/diffs/components/commit_item_spec.js index 8cb4fd20063..0191822d97a 100644 --- a/spec/frontend/diffs/components/commit_item_spec.js +++ b/spec/frontend/diffs/components/commit_item_spec.js @@ -13,8 +13,6 @@ const TEST_AUTHOR_EMAIL = 'test+test@gitlab.com'; const TEST_AUTHOR_GRAVATAR = `${TEST_HOST}/avatar/test?s=40`; const TEST_SIGNATURE_HTML = '<a>Legit commit</a>'; const TEST_PIPELINE_STATUS_PATH = `${TEST_HOST}/pipeline/status`; -const NEXT_COMMIT_URL = `${TEST_HOST}/?commit_id=next`; -const PREV_COMMIT_URL = `${TEST_HOST}/?commit_id=prev`; describe('diffs/components/commit_item', () => { let wrapper; @@ -31,12 +29,6 @@ describe('diffs/components/commit_item', () => { const getCommitActionsElement = () => wrapper.find('.commit-actions'); const getCommitPipelineStatus = () => wrapper.find(CommitPipelineStatus); - const getCommitNavButtonsElement = () => wrapper.find('.commit-nav-buttons'); - const getNextCommitNavElement = () => - getCommitNavButtonsElement().find('.btn-group > *:last-child'); - const getPrevCommitNavElement = () => - getCommitNavButtonsElement().find('.btn-group > *:first-child'); - const mountComponent = (propsData) => { wrapper = mount(Component, { propsData: { @@ -180,126 +172,4 @@ describe('diffs/components/commit_item', () => { expect(getCommitPipelineStatus().exists()).toBe(true); }); }); - - describe('without neighbor commits', () => { - beforeEach(() => { - mountComponent({ commit: { ...commit, prev_commit_id: null, next_commit_id: null } }); - }); - - it('does not render any navigation buttons', () => { - expect(getCommitNavButtonsElement().exists()).toEqual(false); - }); - }); - - describe('with neighbor commits', () => { - let mrCommit; - - beforeEach(() => { - mrCommit = { - ...commit, - next_commit_id: 'next', - prev_commit_id: 'prev', - }; - - mountComponent({ commit: mrCommit }); - }); - - it('renders the commit navigation buttons', () => { - expect(getCommitNavButtonsElement().exists()).toEqual(true); - - mountComponent({ - commit: { ...mrCommit, next_commit_id: null }, - }); - expect(getCommitNavButtonsElement().exists()).toEqual(true); - - mountComponent({ - commit: { ...mrCommit, prev_commit_id: null }, - }); - expect(getCommitNavButtonsElement().exists()).toEqual(true); - }); - - describe('prev commit', () => { - const { location } = window; - - beforeAll(() => { - delete window.location; - window.location = { href: `${TEST_HOST}?commit_id=${mrCommit.id}` }; - }); - - beforeEach(() => { - jest.spyOn(wrapper.vm, 'moveToNeighboringCommit').mockImplementation(() => {}); - }); - - afterAll(() => { - window.location = location; - }); - - it('uses the correct href', () => { - const link = getPrevCommitNavElement(); - - expect(link.element.getAttribute('href')).toEqual(PREV_COMMIT_URL); - }); - - it('triggers the correct Vuex action on click', () => { - const link = getPrevCommitNavElement(); - - link.trigger('click'); - return wrapper.vm.$nextTick().then(() => { - expect(wrapper.vm.moveToNeighboringCommit).toHaveBeenCalledWith({ - direction: 'previous', - }); - }); - }); - - it('renders a disabled button when there is no prev commit', () => { - mountComponent({ commit: { ...mrCommit, prev_commit_id: null } }); - - const button = getPrevCommitNavElement(); - - expect(button.element.tagName).toEqual('BUTTON'); - expect(button.element.hasAttribute('disabled')).toEqual(true); - }); - }); - - describe('next commit', () => { - const { location } = window; - - beforeAll(() => { - delete window.location; - window.location = { href: `${TEST_HOST}?commit_id=${mrCommit.id}` }; - }); - - beforeEach(() => { - jest.spyOn(wrapper.vm, 'moveToNeighboringCommit').mockImplementation(() => {}); - }); - - afterAll(() => { - window.location = location; - }); - - it('uses the correct href', () => { - const link = getNextCommitNavElement(); - - expect(link.element.getAttribute('href')).toEqual(NEXT_COMMIT_URL); - }); - - it('triggers the correct Vuex action on click', () => { - const link = getNextCommitNavElement(); - - link.trigger('click'); - return wrapper.vm.$nextTick().then(() => { - expect(wrapper.vm.moveToNeighboringCommit).toHaveBeenCalledWith({ direction: 'next' }); - }); - }); - - it('renders a disabled button when there is no next commit', () => { - mountComponent({ commit: { ...mrCommit, next_commit_id: null } }); - - const button = getNextCommitNavElement(); - - expect(button.element.tagName).toEqual('BUTTON'); - expect(button.element.hasAttribute('disabled')).toEqual(true); - }); - }); - }); }); diff --git a/spec/frontend/diffs/components/compare_versions_spec.js b/spec/frontend/diffs/components/compare_versions_spec.js index c93a3771ec0..4feddb939c6 100644 --- a/spec/frontend/diffs/components/compare_versions_spec.js +++ b/spec/frontend/diffs/components/compare_versions_spec.js @@ -1,5 +1,6 @@ import { mount, createLocalVue } from '@vue/test-utils'; import Vuex from 'vuex'; +import { TEST_HOST } from 'helpers/test_constants'; import { trimText } from 'helpers/text_helper'; import CompareVersionsComponent from '~/diffs/components/compare_versions.vue'; import { createStore } from '~/mr_notes/stores'; @@ -9,12 +10,17 @@ import diffsMockData from '../mock_data/merge_request_diffs'; const localVue = createLocalVue(); localVue.use(Vuex); +const NEXT_COMMIT_URL = `${TEST_HOST}/?commit_id=next`; +const PREV_COMMIT_URL = `${TEST_HOST}/?commit_id=prev`; + describe('CompareVersions', () => { let wrapper; let store; const targetBranchName = 'tmp-wine-dev'; + const { commit } = getDiffWithCommit(); - const createWrapper = (props) => { + const createWrapper = (props = {}, commitArgs = {}) => { + store.state.diffs.commit = { ...store.state.diffs.commit, ...commitArgs }; wrapper = mount(CompareVersionsComponent, { localVue, store, @@ -28,6 +34,11 @@ describe('CompareVersions', () => { const findLimitedContainer = () => wrapper.find('.container-limited.limit-container-width'); const findCompareSourceDropdown = () => wrapper.find('.mr-version-dropdown'); const findCompareTargetDropdown = () => wrapper.find('.mr-version-compare-dropdown'); + const getCommitNavButtonsElement = () => wrapper.find('.commit-nav-buttons'); + const getNextCommitNavElement = () => + getCommitNavButtonsElement().find('.btn-group > *:last-child'); + const getPrevCommitNavElement = () => + getCommitNavButtonsElement().find('.btn-group > *:first-child'); beforeEach(() => { store = createStore(); @@ -161,4 +172,124 @@ describe('CompareVersions', () => { expect(findCompareTargetDropdown().exists()).toBe(false); }); }); + + describe('without neighbor commits', () => { + beforeEach(() => { + createWrapper({ commit: { ...commit, prev_commit_id: null, next_commit_id: null } }); + }); + + it('does not render any navigation buttons', () => { + expect(getCommitNavButtonsElement().exists()).toEqual(false); + }); + }); + + describe('with neighbor commits', () => { + let mrCommit; + + beforeEach(() => { + mrCommit = { + ...commit, + next_commit_id: 'next', + prev_commit_id: 'prev', + }; + + createWrapper({}, mrCommit); + }); + + it('renders the commit navigation buttons', () => { + expect(getCommitNavButtonsElement().exists()).toEqual(true); + + createWrapper({ + commit: { ...mrCommit, next_commit_id: null }, + }); + expect(getCommitNavButtonsElement().exists()).toEqual(true); + + createWrapper({ + commit: { ...mrCommit, prev_commit_id: null }, + }); + expect(getCommitNavButtonsElement().exists()).toEqual(true); + }); + + describe('prev commit', () => { + const { location } = window; + + beforeAll(() => { + delete window.location; + window.location = { href: `${TEST_HOST}?commit_id=${mrCommit.id}` }; + }); + + beforeEach(() => { + jest.spyOn(wrapper.vm, 'moveToNeighboringCommit').mockImplementation(() => {}); + }); + + afterAll(() => { + window.location = location; + }); + + it('uses the correct href', () => { + const link = getPrevCommitNavElement(); + + expect(link.element.getAttribute('href')).toEqual(PREV_COMMIT_URL); + }); + + it('triggers the correct Vuex action on click', () => { + const link = getPrevCommitNavElement(); + + link.trigger('click'); + return wrapper.vm.$nextTick().then(() => { + expect(wrapper.vm.moveToNeighboringCommit).toHaveBeenCalledWith({ + direction: 'previous', + }); + }); + }); + + it('renders a disabled button when there is no prev commit', () => { + createWrapper({}, { ...mrCommit, prev_commit_id: null }); + + const button = getPrevCommitNavElement(); + + expect(button.element.hasAttribute('disabled')).toEqual(true); + }); + }); + + describe('next commit', () => { + const { location } = window; + + beforeAll(() => { + delete window.location; + window.location = { href: `${TEST_HOST}?commit_id=${mrCommit.id}` }; + }); + + beforeEach(() => { + jest.spyOn(wrapper.vm, 'moveToNeighboringCommit').mockImplementation(() => {}); + }); + + afterAll(() => { + window.location = location; + }); + + it('uses the correct href', () => { + const link = getNextCommitNavElement(); + + expect(link.element.getAttribute('href')).toEqual(NEXT_COMMIT_URL); + }); + + it('triggers the correct Vuex action on click', () => { + const link = getNextCommitNavElement(); + + link.trigger('click'); + return wrapper.vm.$nextTick().then(() => { + expect(wrapper.vm.moveToNeighboringCommit).toHaveBeenCalledWith({ direction: 'next' }); + }); + }); + + it('renders a disabled button when there is no next commit', () => { + createWrapper({}, { ...mrCommit, next_commit_id: null }); + + const button = getNextCommitNavElement(); + + expect(button.element.hasAttribute('disabled')).toEqual(true); + }); + }); + }); }); diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 1f0eed96503..1850363bb72 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -810,6 +810,31 @@ RSpec.describe API::Projects do end end end + + context 'with forked projects', :use_clean_rails_memory_store_caching do + include ProjectForksHelper + + let_it_be(:admin) { create(:admin) } + + it 'avoids N+1 queries' do + get api('/projects', admin) + + base_project = create(:project, :public, namespace: admin.namespace) + + fork_project1 = fork_project(base_project, admin, namespace: create(:user).namespace) + fork_project2 = fork_project(fork_project1, admin, namespace: create(:user).namespace) + + control = ActiveRecord::QueryRecorder.new do + get api('/projects', admin) + end + + fork_project(fork_project2, admin, namespace: create(:user).namespace) + + expect do + get api('/projects', admin) + end.not_to exceed_query_limit(control.count) + end + end end describe 'POST /projects' do diff --git a/spec/services/merge_requests/after_create_service_spec.rb b/spec/services/merge_requests/after_create_service_spec.rb index dce351d8a31..4830221649f 100644 --- a/spec/services/merge_requests/after_create_service_spec.rb +++ b/spec/services/merge_requests/after_create_service_spec.rb @@ -93,5 +93,93 @@ RSpec.describe MergeRequests::AfterCreateService do expect(merge_request.reload).to be_unchecked end end + + it 'increments the usage data counter of create event' do + counter = Gitlab::UsageDataCounters::MergeRequestCounter + + expect { execute_service }.to change { counter.read(:create) }.by(1) + end + + context 'todos' do + it 'does not creates todos' do + attributes = { + project: merge_request.target_project, + target_id: merge_request.id, + target_type: merge_request.class.name + } + + expect { execute_service }.not_to change { Todo.where(attributes).count } + end + + context 'when merge request is assigned to someone' do + let_it_be(:assignee) { create(:user) } + let_it_be(:merge_request) { create(:merge_request, assignees: [assignee]) } + + it 'creates a todo for new assignee' do + attributes = { + project: merge_request.target_project, + author: merge_request.author, + user: assignee, + target_id: merge_request.id, + target_type: merge_request.class.name, + action: Todo::ASSIGNED, + state: :pending + } + + expect { execute_service }.to change { Todo.where(attributes).count }.by(1) + end + end + + context 'when reviewer is assigned' do + let_it_be(:reviewer) { create(:user) } + let_it_be(:merge_request) { create(:merge_request, reviewers: [reviewer]) } + + it 'creates a todo for new reviewer' do + attributes = { + project: merge_request.target_project, + author: merge_request.author, + user: reviewer, + target_id: merge_request.id, + target_type: merge_request.class.name, + action: Todo::REVIEW_REQUESTED, + state: :pending + } + + expect { execute_service }.to change { Todo.where(attributes).count }.by(1) + end + end + end + + context 'when saving references to issues that the created merge request closes' do + let_it_be(:first_issue) { create(:issue, project: merge_request.target_project) } + let_it_be(:second_issue) { create(:issue, project: merge_request.target_project) } + + it 'creates a `MergeRequestsClosingIssues` record for each issue' do + merge_request.description = "Closes #{first_issue.to_reference} and #{second_issue.to_reference}" + merge_request.source_branch = "feature" + merge_request.target_branch = merge_request.target_project.default_branch + merge_request.save! + + execute_service + + issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id) + expect(issue_ids).to match_array([first_issue.id, second_issue.id]) + end + end + + it 'tracks merge request creation in usage data' do + expect(Gitlab::UsageDataCounters::MergeRequestCounter).to receive(:count).with(:create) + + execute_service + end + + it 'calls MergeRequests::LinkLfsObjectsService#execute' do + service = instance_spy(MergeRequests::LinkLfsObjectsService) + allow(MergeRequests::LinkLfsObjectsService).to receive(:new).with(merge_request.target_project).and_return(service) + + execute_service + + expect(service).to have_received(:execute).with(merge_request) + end end end diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index 4f47a22b07c..4646ed866a0 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -47,16 +47,6 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do .to change { project.open_merge_requests_count }.from(0).to(1) end - it 'does not creates todos' do - attributes = { - project: project, - target_id: merge_request.id, - target_type: merge_request.class.name - } - - expect(Todo.where(attributes).count).to be_zero - end - it 'creates exactly 1 create MR event', :sidekiq_might_not_need_inline do attributes = { action: :created, @@ -113,20 +103,6 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do end it { expect(merge_request.assignees).to eq([user2]) } - - it 'creates a todo for new assignee' do - attributes = { - project: project, - author: user, - user: user2, - target_id: merge_request.id, - target_type: merge_request.class.name, - action: Todo::ASSIGNED, - state: :pending - } - - expect(Todo.where(attributes).count).to eq 1 - end end context 'when reviewer is assigned' do @@ -142,20 +118,6 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do it { expect(merge_request.reviewers).to eq([user2]) } - it 'creates a todo for new reviewer' do - attributes = { - project: project, - author: user, - user: user2, - target_id: merge_request.id, - target_type: merge_request.class.name, - action: Todo::REVIEW_REQUESTED, - state: :pending - } - - expect(Todo.where(attributes).count).to eq 1 - end - it 'invalidates counter cache for reviewers', :use_clean_rails_memory_store_caching do expect { merge_request } .to change { user2.review_requested_open_merge_requests_count } @@ -328,12 +290,6 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do end end - it 'increments the usage data counter of create event' do - counter = Gitlab::UsageDataCounters::MergeRequestCounter - - expect { service.execute }.to change { counter.read(:create) }.by(1) - end - context 'after_save callback to store_mentions' do let(:labels) { create_pair(:label, project: project) } let(:milestone) { create(:milestone, project: project) } @@ -494,35 +450,6 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do end end - context 'while saving references to issues that the created merge request closes' do - let(:first_issue) { create(:issue, project: project) } - let(:second_issue) { create(:issue, project: project) } - - let(:opts) do - { - title: 'Awesome merge_request', - source_branch: 'feature', - target_branch: 'master', - force_remove_source_branch: '1' - } - end - - before do - project.add_maintainer(user) - project.add_developer(user2) - end - - it 'creates a `MergeRequestsClosingIssues` record for each issue' do - issue_closing_opts = opts.merge(description: "Closes #{first_issue.to_reference} and #{second_issue.to_reference}") - service = described_class.new(project, user, issue_closing_opts) - allow(service).to receive(:execute_hooks) - merge_request = service.execute - - issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id) - expect(issue_ids).to match_array([first_issue.id, second_issue.id]) - end - end - context 'when source and target projects are different' do let(:target_project) { fork_project(project, nil, repository: true) } @@ -571,14 +498,6 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do expect(merge_request).to be_persisted end - it 'calls MergeRequests::LinkLfsObjectsService#execute', :sidekiq_might_not_need_inline do - expect_next_instance_of(MergeRequests::LinkLfsObjectsService) do |service| - expect(service).to receive(:execute).with(instance_of(MergeRequest)) - end - - described_class.new(project, user, opts).execute - end - it 'does not create the merge request when the target project is archived' do target_project.update!(archived: true) diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 7a7f684c6d0..d27a35420aa 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -948,18 +948,8 @@ RSpec.describe MergeRequests::UpdateService, :mailer do end it 'removes `MergeRequestsClosingIssues` records when issues are not closed anymore' do - opts = { - title: 'Awesome merge_request', - description: "Closes #{first_issue.to_reference} and #{second_issue.to_reference}", - source_branch: 'feature', - target_branch: 'master', - force_remove_source_branch: '1' - } - - merge_request = MergeRequests::CreateService.new(project, user, opts).execute - - issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id) - expect(issue_ids).to match_array([first_issue.id, second_issue.id]) + create(:merge_requests_closing_issues, issue: first_issue, merge_request: merge_request) + create(:merge_requests_closing_issues, issue: second_issue, merge_request: merge_request) service = described_class.new(project, user, description: "not closing any issues") allow(service).to receive(:execute_hooks) diff --git a/spec/support/shared_contexts/navbar_structure_context.rb b/spec/support/shared_contexts/navbar_structure_context.rb index 460c7febc30..78d14ecb880 100644 --- a/spec/support/shared_contexts/navbar_structure_context.rb +++ b/spec/support/shared_contexts/navbar_structure_context.rb @@ -59,7 +59,7 @@ RSpec.shared_context 'project navbar structure' do ] }, { - nav_item: _('Merge Requests'), + nav_item: _('Merge requests'), nav_sub_items: [] }, { @@ -190,7 +190,7 @@ RSpec.shared_context 'group navbar structure' do ] }, { - nav_item: _('Merge Requests'), + nav_item: _('Merge requests'), nav_sub_items: [] }, security_and_compliance_nav_item, |
