diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-11-02 21:12:04 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-11-02 21:12:04 +0300 |
commit | eed7260f13c0a3139876e3659603f3d803e8fcd7 (patch) | |
tree | f877bb301ff936d73516241ad608271bc5a624fd /app | |
parent | ef211f6aff22891e232a700b61d2d3bf567ed6bf (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'app')
21 files changed, 315 insertions, 161 deletions
diff --git a/app/assets/javascripts/ci/pipelines_page/components/pipeline_status_badge.vue b/app/assets/javascripts/ci/pipelines_page/components/pipeline_status_badge.vue index 3dcbcf260dc..380f8ce172f 100644 --- a/app/assets/javascripts/ci/pipelines_page/components/pipeline_status_badge.vue +++ b/app/assets/javascripts/ci/pipelines_page/components/pipeline_status_badge.vue @@ -32,7 +32,7 @@ export default { <template> <div> <ci-icon - class="gl-mb-3" + class="gl-mb-2" :status="pipelineStatus" show-status-text @ciStatusBadgeClick="trackClick" diff --git a/app/assets/javascripts/diffs/components/tree_list.vue b/app/assets/javascripts/diffs/components/tree_list.vue index f4715c591b2..07984beb709 100644 --- a/app/assets/javascripts/diffs/components/tree_list.vue +++ b/app/assets/javascripts/diffs/components/tree_list.vue @@ -3,22 +3,20 @@ import { GlTooltipDirective, GlIcon } from '@gitlab/ui'; // eslint-disable-next-line no-restricted-imports import { mapActions, mapGetters, mapState } from 'vuex'; import micromatch from 'micromatch'; -import { debounce } from 'lodash'; import { getModifierKey } from '~/constants'; import { s__, sprintf } from '~/locale'; import { RecycleScroller } from 'vendor/vue-virtual-scroller'; -import { contentTop } from '~/lib/utils/common_utils'; import DiffFileRow from './diff_file_row.vue'; +import TreeListHeight from './tree_list_height.vue'; const MODIFIER_KEY = getModifierKey(); -const MAX_ITEMS_ON_NARROW_SCREEN = 8; -const BOTTOM_MARGIN = 16; export default { directives: { GlTooltip: GlTooltipDirective, }, components: { + TreeListHeight, GlIcon, DiffFileRow, RecycleScroller, @@ -32,17 +30,10 @@ export default { data() { return { search: '', - scrollerHeight: 0, - rowHeight: 0, - debouncedHeightCalc: null, - reviewBarHeight: 0, - largeBreakpointSize: 0, }; }, computed: { ...mapState('diffs', ['tree', 'renderTreeList', 'currentDiffFileId', 'viewedDiffFileIds']), - ...mapState('batchComments', ['reviewBarRendered']), - ...mapGetters('batchComments', ['draftsCount']), ...mapGetters('diffs', ['allBlobs']), filteredTreeList() { let search = this.search.toLowerCase().trim(); @@ -95,76 +86,21 @@ export default { return result; }, - reviewBarEnabled() { - return this.draftsCount > 0; - }, - }, - watch: { - reviewBarEnabled() { - this.debouncedHeightCalc(); - }, - calculateReviewBarHeight() { - this.debouncedHeightCalc(); - }, - }, - created() { - this.debouncedHeightCalc = debounce(this.calculateScrollerHeight, 50); - }, - mounted() { - const heightProp = getComputedStyle(this.$refs.wrapper).getPropertyValue('--file-row-height'); - const breakpointProp = getComputedStyle(window.document.body).getPropertyValue( - '--breakpoint-lg', - ); - this.largeBreakpointSize = parseInt(breakpointProp, 10); - this.rowHeight = parseInt(heightProp, 10); - this.calculateScrollerHeight(); - let stop; - // eslint-disable-next-line prefer-const - stop = this.$watch( - () => this.reviewBarRendered, - (enabled) => { - if (!enabled) return; - this.calculateReviewBarHeight(); - stop(); - }, - { immediate: true }, - ); - window.addEventListener('resize', this.debouncedHeightCalc, { passive: true }); - }, - beforeDestroy() { - window.removeEventListener('resize', this.debouncedHeightCalc, { passive: true }); }, methods: { ...mapActions('diffs', ['toggleTreeOpen', 'goToFile']), clearSearch() { this.search = ''; }, - calculateScrollerHeight() { - if (window.matchMedia(`(max-width: ${this.largeBreakpointSize - 1}px)`).matches) { - this.calculateMobileScrollerHeight(); - } else { - let clipping = BOTTOM_MARGIN; - if (this.reviewBarEnabled) clipping += this.reviewBarHeight; - this.scrollerHeight = this.$refs.scrollRoot.clientHeight - clipping; - } - }, - calculateMobileScrollerHeight() { - const maxItems = Math.min(MAX_ITEMS_ON_NARROW_SCREEN, this.flatFilteredTreeList.length); - this.scrollerHeight = Math.min(maxItems * this.rowHeight, window.innerHeight - contentTop()); - }, - calculateReviewBarHeight() { - this.reviewBarHeight = document.querySelector('.js-review-bar')?.offsetHeight || 0; - }, }, searchPlaceholder: sprintf(s__('MergeRequest|Search (e.g. *.vue) (%{MODIFIER_KEY}P)'), { MODIFIER_KEY, }), - DiffFileRow, }; </script> <template> - <div ref="wrapper" class="tree-list-holder d-flex flex-column" data-testid="file-tree-container"> + <div class="tree-list-holder d-flex flex-column" data-testid="file-tree-container"> <div class="gl-pb-3 position-relative tree-list-search d-flex"> <div class="flex-fill d-flex"> <gl-icon name="search" class="gl-absolute gl-top-3 gl-left-3 tree-list-icon" /> @@ -189,41 +125,41 @@ export default { </button> </div> </div> - <div - ref="scrollRoot" - :class="{ 'tree-list-blobs': !renderTreeList || search }" - class="gl-flex-grow-1 mr-tree-list" - > - <recycle-scroller - v-if="flatFilteredTreeList.length" - :style="{ height: `${scrollerHeight}px` }" - :items="flatFilteredTreeList" - :item-size="rowHeight" - :buffer="100" - key-field="key" - > - <template #default="{ item }"> - <diff-file-row - :file="item" - :level="item.level" - :viewed-files="viewedDiffFileIds" - :hide-file-stats="hideFileStats" - :current-diff-file-id="currentDiffFileId" - :style="{ '--level': item.level }" - :class="{ 'tree-list-parent': item.level > 0 }" - class="gl-relative" - @toggleTreeOpen="toggleTreeOpen" - @clickFile="(path) => goToFile({ path })" - /> - </template> - <template #after> - <div class="tree-list-gutter"></div> - </template> - </recycle-scroller> - <p v-else class="prepend-top-20 append-bottom-20 text-center"> - {{ s__('MergeRequest|No files found') }} - </p> - </div> + <tree-list-height class="gl-flex-grow-1 gl-min-h-0" :items-count="flatFilteredTreeList.length"> + <template #default="{ scrollerHeight, rowHeight }"> + <div :class="{ 'tree-list-blobs': !renderTreeList || search }" class="mr-tree-list"> + <recycle-scroller + v-if="flatFilteredTreeList.length" + :style="{ height: `${scrollerHeight}px` }" + :items="flatFilteredTreeList" + :item-size="rowHeight" + :buffer="100" + key-field="key" + > + <template #default="{ item }"> + <diff-file-row + :file="item" + :level="item.level" + :viewed-files="viewedDiffFileIds" + :hide-file-stats="hideFileStats" + :current-diff-file-id="currentDiffFileId" + :style="{ '--level': item.level }" + :class="{ 'tree-list-parent': item.level > 0 }" + class="gl-relative" + @toggleTreeOpen="toggleTreeOpen" + @clickFile="(path) => goToFile({ path })" + /> + </template> + <template #after> + <div class="tree-list-gutter"></div> + </template> + </recycle-scroller> + <p v-else class="prepend-top-20 append-bottom-20 text-center"> + {{ s__('MergeRequest|No files found') }} + </p> + </div> + </template> + </tree-list-height> </div> </template> diff --git a/app/assets/javascripts/diffs/components/tree_list_height.vue b/app/assets/javascripts/diffs/components/tree_list_height.vue new file mode 100644 index 00000000000..4da94cacd75 --- /dev/null +++ b/app/assets/javascripts/diffs/components/tree_list_height.vue @@ -0,0 +1,108 @@ +<script> +import { debounce } from 'lodash'; +// eslint-disable-next-line no-restricted-imports +import { mapState, mapGetters } from 'vuex'; +import { contentTop } from '~/lib/utils/common_utils'; + +const MAX_ITEMS_ON_NARROW_SCREEN = 8; +// Should be enough for the very long titles (10+ lines) on the max smallest screen +const MAX_SCROLL_Y = 600; +const BOTTOM_OFFSET = 16; + +export default { + name: 'TreeListHeight', + props: { + itemsCount: { + type: Number, + required: true, + }, + }, + data() { + return { + scrollerHeight: 0, + rowHeight: 0, + reviewBarHeight: 0, + scrollY: 0, + isNarrowScreen: false, + mediaQueryMatch: null, + }; + }, + computed: { + ...mapState('batchComments', ['reviewBarRendered']), + ...mapGetters('batchComments', ['draftsCount']), + reviewBarEnabled() { + return this.draftsCount > 0; + }, + debouncedHeightCalc() { + return debounce(this.calculateScrollerHeight, 100); + }, + debouncedRecordScroll() { + return debounce(this.recordScroll, 50); + }, + }, + watch: { + reviewBarRendered: { + handler(rendered) { + if (!rendered || this.reviewBarHeight) return; + this.reviewBarHeight = document.querySelector('.js-review-bar').offsetHeight; + this.debouncedHeightCalc(); + }, + immediate: true, + }, + reviewBarEnabled: 'debouncedHeightCalc', + scrollY: 'debouncedHeightCalc', + isNarrowScreen: 'recordScroll', + }, + mounted() { + const computedStyles = getComputedStyle(this.$refs.scrollRoot); + this.rowHeight = parseInt(computedStyles.getPropertyValue('--file-row-height'), 10); + + const largeBreakpointSize = parseInt(computedStyles.getPropertyValue('--breakpoint-lg'), 10); + this.mediaQueryMatch = window.matchMedia(`(max-width: ${largeBreakpointSize - 1}px)`); + this.isNarrowScreen = this.mediaQueryMatch.matches; + this.mediaQueryMatch.addEventListener('change', this.handleMediaMatch); + + window.addEventListener('resize', this.debouncedHeightCalc, { passive: true }); + window.addEventListener('scroll', this.debouncedRecordScroll, { passive: true }); + + this.calculateScrollerHeight(); + }, + beforeDestroy() { + this.mediaQueryMatch.removeEventListener('change', this.handleMediaMatch); + this.mediaQueryMatch = null; + window.removeEventListener('resize', this.debouncedHeightCalc, { passive: true }); + window.removeEventListener('scroll', this.debouncedRecordScroll, { passive: true }); + }, + methods: { + recordScroll() { + const { scrollY } = window; + if (scrollY > MAX_SCROLL_Y || this.isNarrowScreen) { + this.scrollY = MAX_SCROLL_Y; + } else { + this.scrollY = window.scrollY; + } + }, + handleMediaMatch({ matches }) { + this.isNarrowScreen = matches; + }, + calculateScrollerHeight() { + if (this.isNarrowScreen) { + const maxItems = Math.min(MAX_ITEMS_ON_NARROW_SCREEN, this.itemsCount); + const maxHeight = maxItems * this.rowHeight; + this.scrollerHeight = Math.min(maxHeight, window.innerHeight - contentTop()); + } else { + const { y } = this.$refs.scrollRoot.getBoundingClientRect(); + const reviewBarOffset = this.reviewBarEnabled ? this.reviewBarHeight : 0; + // distance from element's top vertical position in the viewport to the bottom of the viewport minus offsets + this.scrollerHeight = window.innerHeight - y - reviewBarOffset - BOTTOM_OFFSET; + } + }, + }, +}; +</script> + +<template> + <div ref="scrollRoot"> + <slot :scroller-height="scrollerHeight" :row-height="rowHeight"></slot> + </div> +</template> diff --git a/app/assets/javascripts/graphql_shared/queries/groups_autocomplete.query.graphql b/app/assets/javascripts/graphql_shared/queries/groups_autocomplete.query.graphql new file mode 100644 index 00000000000..74da46e5a60 --- /dev/null +++ b/app/assets/javascripts/graphql_shared/queries/groups_autocomplete.query.graphql @@ -0,0 +1,10 @@ +query groupsAutocomplete($search: String) { + groups(search: $search) { + nodes { + id + name + fullName + avatarUrl + } + } +} diff --git a/app/assets/javascripts/repository/components/commit_info.vue b/app/assets/javascripts/repository/components/commit_info.vue index 782e2f6ab1b..5c1737eb310 100644 --- a/app/assets/javascripts/repository/components/commit_info.vue +++ b/app/assets/javascripts/repository/components/commit_info.vue @@ -1,6 +1,6 @@ <script> import { GlTooltipDirective, GlLink, GlButton } from '@gitlab/ui'; -import { __ } from '~/locale'; +import { __, sprintf } from '~/locale'; import SafeHtml from '~/vue_shared/directives/safe_html'; import defaultAvatarUrl from 'images/no_avatar.png'; import TimeagoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; @@ -35,6 +35,9 @@ export default { // Strip the newline at the beginning return this.commit?.descriptionHtml?.replace(/^
/, ''); }, + avatarLinkAltText() { + return sprintf(__(`%{username}'s avatar`), { username: this.commit.authorName }); + }, }, methods: { toggleShowDescription() { @@ -58,6 +61,7 @@ export default { v-if="commit.author" :link-href="commit.author.webPath" :img-src="commit.author.avatarUrl" + :img-alt="avatarLinkAltText" :img-size="32" class="gl-my-2 gl-mr-4" /> diff --git a/app/assets/javascripts/token_access/graphql/cache_config.js b/app/assets/javascripts/token_access/graphql/cache_config.js new file mode 100644 index 00000000000..2db534b7eb5 --- /dev/null +++ b/app/assets/javascripts/token_access/graphql/cache_config.js @@ -0,0 +1,14 @@ +export default { + typePolicies: { + Project: { + fields: { + ciCdSettings: { + merge: true, + }, + ciJobTokenScope: { + merge: true, + }, + }, + }, + }, +}; diff --git a/app/assets/javascripts/token_access/index.js b/app/assets/javascripts/token_access/index.js index 9258d5eba45..45bd1921dbd 100644 --- a/app/assets/javascripts/token_access/index.js +++ b/app/assets/javascripts/token_access/index.js @@ -2,11 +2,12 @@ import Vue from 'vue'; import VueApollo from 'vue-apollo'; import createDefaultClient from '~/lib/graphql'; import TokenAccessApp from './components/token_access_app.vue'; +import cacheConfig from './graphql/cache_config'; Vue.use(VueApollo); const apolloProvider = new VueApollo({ - defaultClient: createDefaultClient(), + defaultClient: createDefaultClient({}, { cacheConfig }), }); export const initTokenAccess = (containerId = 'js-ci-token-access-app') => { diff --git a/app/assets/javascripts/vue_shared/components/list_selector/constants.js b/app/assets/javascripts/vue_shared/components/list_selector/constants.js index c9db79581d1..2e58527a2ea 100644 --- a/app/assets/javascripts/vue_shared/components/list_selector/constants.js +++ b/app/assets/javascripts/vue_shared/components/list_selector/constants.js @@ -1,7 +1,6 @@ import { __ } from '~/locale'; -// Note, we can extend this config in future to make the component work in other contexts -// https://gitlab.com/gitlab-org/gitlab/-/issues/428865 export const CONFIG = { users: { title: __('Users'), icon: 'user', filterKey: 'username' }, + groups: { title: __('Groups'), icon: 'group', filterKey: 'name' }, }; diff --git a/app/assets/javascripts/vue_shared/components/list_selector/group_item.vue b/app/assets/javascripts/vue_shared/components/list_selector/group_item.vue new file mode 100644 index 00000000000..2d24cc5553b --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/list_selector/group_item.vue @@ -0,0 +1,55 @@ +<script> +import { GlAvatar, GlButton } from '@gitlab/ui'; +import { sprintf, __ } from '~/locale'; + +export default { + name: 'GroupItem', + components: { + GlAvatar, + GlButton, + }, + props: { + data: { + type: Object, + required: true, + }, + canDelete: { + type: Boolean, + required: false, + default: false, + }, + }, + computed: { + deleteButtonLabel() { + return sprintf(__('Delete %{name}'), { name: this.name }); + }, + fullName() { + return this.data.fullName; + }, + name() { + return this.data.name; + }, + avatarUrl() { + return this.data.avatarUrl; + }, + }, +}; +</script> + +<template> + <span class="gl-display-flex gl-align-items-center gl-gap-3" @click="$emit('select', name)"> + <gl-avatar :alt="fullName" :size="32" :src="avatarUrl" /> + <span class="gl-display-flex gl-flex-direction-column gl-flex-grow-1"> + <span class="gl-font-weight-bold">{{ fullName }}</span> + <span class="gl-text-gray-600">@{{ name }}</span> + </span> + + <gl-button + v-if="canDelete" + icon="remove" + :aria-label="deleteButtonLabel" + category="tertiary" + @click="$emit('delete', name)" + /> + </span> +</template> diff --git a/app/assets/javascripts/vue_shared/components/list_selector/index.vue b/app/assets/javascripts/vue_shared/components/list_selector/index.vue index 237369f5900..6813d9ca077 100644 --- a/app/assets/javascripts/vue_shared/components/list_selector/index.vue +++ b/app/assets/javascripts/vue_shared/components/list_selector/index.vue @@ -1,7 +1,9 @@ <script> import { GlCard, GlIcon, GlCollapsibleListbox, GlSearchBoxByType } from '@gitlab/ui'; import usersAutocompleteQuery from '~/graphql_shared/queries/users_autocomplete.query.graphql'; -import User from './user.vue'; +import groupsAutocompleteQuery from '~/graphql_shared/queries/groups_autocomplete.query.graphql'; +import UserItem from './user_item.vue'; +import GroupItem from './group_item.vue'; import { CONFIG } from './constants'; export default { @@ -11,7 +13,6 @@ export default { GlIcon, GlSearchBoxByType, GlCollapsibleListbox, - User, }, props: { title: { @@ -46,24 +47,23 @@ export default { return CONFIG[this.type]; }, searchItems() { - return ( - this.items?.map((item) => ({ - value: item.username, - text: item.name, - ...item, - })) || [] - ); + return this.items; + }, + isUserVariant() { + return this.type === 'users'; }, component() { - // Note, we can extend this for the component to support other contexts - // https://gitlab.com/gitlab-org/gitlab/-/issues/428865 - return User; + return this.isUserVariant ? UserItem : GroupItem; }, }, methods: { async handleSearchInput(search) { this.$refs.results.open(); - this.items = await this.fetchUsersBySearchTerm(search); + if (this.isUserVariant) { + this.items = await this.fetchUsersBySearchTerm(search); + } else { + this.items = await this.fetchGroupsBySearchTerm(search); + } }, fetchUsersBySearchTerm(search) { const namespace = this.isProject ? 'project' : 'group'; @@ -72,7 +72,27 @@ export default { query: usersAutocompleteQuery, variables: { fullPath: this.projectPath, search, isProject: this.isProject }, }) - .then(({ data }) => data[namespace]?.autocompleteUsers); + .then(({ data }) => + data[namespace]?.autocompleteUsers.map((user) => ({ + text: user.name, + value: user.username, + ...user, + })), + ); + }, + fetchGroupsBySearchTerm(search) { + return this.$apollo + .query({ + query: groupsAutocompleteQuery, + variables: { search }, + }) + .then(({ data }) => + data?.groups.nodes.map((group) => ({ + text: group.fullName, + value: group.name, + ...group, + })), + ); }, getItemByKey(key) { return this.searchItems.find((item) => item[this.config.filterKey] === key); @@ -90,9 +110,9 @@ export default { <template> <gl-card header-class="gl-new-card-header gl-border-none" body-class="gl-card-footer"> <template #header - ><strong + ><strong data-testid="list-selector-title" >{{ title }} - <span class="gl-text-gray-500" + <span class="gl-text-gray-700 gl-ml-3" ><gl-icon :name="config.icon" /> {{ selectedItems.length }}</span ></strong ></template diff --git a/app/assets/javascripts/vue_shared/components/list_selector/user.vue b/app/assets/javascripts/vue_shared/components/list_selector/user_item.vue index fdbc767db81..fdbc767db81 100644 --- a/app/assets/javascripts/vue_shared/components/list_selector/user.vue +++ b/app/assets/javascripts/vue_shared/components/list_selector/user_item.vue diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 7149fe6db4a..ab8547c3fef 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -487,7 +487,7 @@ $performance-bar-height: 2.5rem; $system-header-height: 16px; $system-footer-height: $system-header-height; $mr-sticky-header-height: 72px; -$mr-review-bar-height: calc(2rem + 13px); +$mr-review-bar-height: calc(2rem + 16px); $flash-height: 52px; $context-header-height: 60px; $top-bar-height: 48px; @@ -717,10 +717,10 @@ $blame-blue: #254e77; */ $builds-log-bg: #111; $job-log-highlight-height: 18px; -$job-log-line-padding: 55px; +$job-log-line-padding: 63px; $job-line-number-width: 50px; -$job-line-number-margin: 43px; -$job-arrow-margin: 55px; +$job-line-number-margin: 51px; +$job-arrow-margin: 63px; /* * Calendar diff --git a/app/assets/stylesheets/page_bundles/merge_requests.scss b/app/assets/stylesheets/page_bundles/merge_requests.scss index f353e678429..847cd3f2ff4 100644 --- a/app/assets/stylesheets/page_bundles/merge_requests.scss +++ b/app/assets/stylesheets/page_bundles/merge_requests.scss @@ -258,15 +258,15 @@ $tabs-holder-z-index: 250; position: sticky; top: calc(#{$calc-application-header-height} + #{$mr-tabs-height} + #{$diff-file-header-top}); + // height calc is fully delegated to the tree_list_height.vue component + height: 0; min-height: 300px; - height: calc(#{$calc-application-viewport-height} - (#{$mr-tabs-height} + #{$diff-file-header-top})); .drag-handle { bottom: 16px; } &.is-sidebar-moved { - height: calc(#{$calc-application-viewport-height} - (#{$mr-sticky-header-height} + #{$diff-file-header-top})); top: calc(#{$calc-application-header-height} + #{$mr-sticky-header-height} + #{$diff-file-header-top}); } } @@ -1108,7 +1108,7 @@ $tabs-holder-z-index: 250; display: flex; align-items: center; width: 100%; - height: $toggle-sidebar-height; + height: var(--mr-review-bar-height); padding-left: $contextual-sidebar-width; padding-right: $right-sidebar-collapsed-width; background: var(--white, $white); diff --git a/app/assets/stylesheets/page_bundles/pipelines.scss b/app/assets/stylesheets/page_bundles/pipelines.scss index f9c49b0e6ca..bcc0ad112ac 100644 --- a/app/assets/stylesheets/page_bundles/pipelines.scss +++ b/app/assets/stylesheets/page_bundles/pipelines.scss @@ -14,10 +14,6 @@ // - app/assets/javascripts/commit/pipelines/pipelines_bundle.js .pipelines { - .badge { - margin-bottom: 3px; - } - .pipeline-actions { min-width: 170px; //Guarantees buttons don't break in several lines. diff --git a/app/controllers/admin/dashboard_controller.rb b/app/controllers/admin/dashboard_controller.rb index dab0f3e870a..a03e0c0807f 100644 --- a/app/controllers/admin/dashboard_controller.rb +++ b/app/controllers/admin/dashboard_controller.rb @@ -13,8 +13,7 @@ class Admin::DashboardController < Admin::ApplicationController @projects = Project.order_id_desc.without_deleted.with_route.limit(10) @users = User.order_id_desc.limit(10) @groups = Group.order_id_desc.with_route.limit(10) - @notices = Gitlab::ConfigChecker::PumaRuggedChecker.check - @notices += Gitlab::ConfigChecker::ExternalDatabaseChecker.check + @notices = Gitlab::ConfigChecker::ExternalDatabaseChecker.check @redis_versions = Gitlab::Redis::ALL_CLASSES.map(&:version).uniq end diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 72a5c4d3e69..5b9b3b7de11 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -279,6 +279,7 @@ class GroupsController < Groups::ApplicationController :avatar, :description, :emails_disabled, + :emails_enabled, :show_diff_preview_in_email, :mentions_disabled, :lfs_enabled, diff --git a/app/finders/user_group_notification_settings_finder.rb b/app/finders/user_group_notification_settings_finder.rb index c6a1a6b36d1..8d06d3d18ca 100644 --- a/app/finders/user_group_notification_settings_finder.rb +++ b/app/finders/user_group_notification_settings_finder.rb @@ -11,11 +11,16 @@ class UserGroupNotificationSettingsFinder @loaded_groups_with_ancestors = groups_with_ancestors.index_by(&:id) @loaded_notification_settings = user.notification_settings_for_groups(groups_with_ancestors).preload_source_route.index_by(&:source_id) - preload_emails_disabled + preload_emails_enabled - groups.map do |group| + group_notifications = groups.map do |group| find_notification_setting_for(group) end + + group_sources = group_notifications.map(&:source) + ActiveRecord::Associations::Preloader.new(records: group_sources, associations: :namespace_settings).call + + group_notifications end private @@ -45,18 +50,18 @@ class UserGroupNotificationSettingsFinder parent_setting.level != NotificationSetting.levels[:global] || parent_setting.notification_email.present? end - # This method preloads the `emails_disabled` strong memoized method for the given groups. + # This method preloads the `emails_enabled` strong memoized method for the given groups. # - # For each group, look up the ancestor hierarchy and look for any group where emails_disabled is true. + # For each group, look up the ancestor hierarchy and look for any group where emails_enabled is false. # The lookup is implemented with an EXISTS subquery, so we can look up the ancestor chain for each group individually. # The query will return groups where at least one ancestor has the `emails_disabled` set to true. # # After the query, we set the instance variable. - def preload_emails_disabled + def preload_emails_enabled group_ids_with_disabled_email = Group.ids_with_disabled_email(groups.to_a) groups.each do |group| - group.emails_disabled_memoized = group_ids_with_disabled_email.include?(group.id) if group.parent_id + group.emails_enabled_memoized = group_ids_with_disabled_email.exclude?(group.id) if group.parent_id end end end diff --git a/app/models/commit.rb b/app/models/commit.rb index 39e12b53f21..886e6e9fbd7 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -372,9 +372,7 @@ class Commit strong_memoize(:raw_signature_type) do next unless @raw.instance_of?(Gitlab::Git::Commit) - if raw_commit_from_rugged? && gpg_commit.signature_text.present? - :PGP - elsif defined? @raw.raw_commit.signature_type + if defined? @raw.raw_commit.signature_type @raw.raw_commit.signature_type end end @@ -397,10 +395,6 @@ class Commit end end - def raw_commit_from_rugged? - @raw.raw_commit.is_a?(Rugged::Commit) - end - def gpg_commit @gpg_commit ||= Gitlab::Gpg::Commit.new(self) end diff --git a/app/models/group.rb b/app/models/group.rb index 919b80ccffb..492f4195931 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -300,14 +300,15 @@ class Group < Namespace groups.drop(1).each { |group| group.root_ancestor = root } end - # Returns the ids of the passed group models where the `emails_disabled` - # column is set to true anywhere in the ancestor hierarchy. + # Returns the ids of the passed group models where the `emails_enabled` + # column is set to false anywhere in the ancestor hierarchy. def ids_with_disabled_email(groups) inner_groups = Group.where('id = namespaces_with_emails_disabled.id') inner_query = inner_groups .self_and_ancestors - .where(emails_disabled: true) + .joins(:namespace_settings) + .where(namespace_settings: { emails_enabled: false }) .select('1') .limit(1) diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 733b89fcaf2..5abda6196c1 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -138,6 +138,8 @@ class Namespace < ApplicationRecord to: :namespace_settings delegate :runner_registration_enabled, :runner_registration_enabled?, :runner_registration_enabled=, to: :namespace_settings + delegate :emails_enabled, :emails_enabled=, + to: :namespace_settings, allow_nil: true delegate :allow_runner_registration_token, :allow_runner_registration_token=, to: :namespace_settings @@ -204,7 +206,7 @@ class Namespace < ApplicationRecord # Make sure that the name is same as strong_memoize name in root_ancestor # method - attr_writer :root_ancestor, :emails_disabled_memoized + attr_writer :root_ancestor, :emails_enabled_memoized class << self def sti_class_for(type_name) @@ -382,17 +384,16 @@ class Namespace < ApplicationRecord # any ancestor can disable emails for all descendants def emails_disabled? - strong_memoize(:emails_disabled_memoized) do - if parent_id - self_and_ancestors.where(emails_disabled: true).exists? - else - !!emails_disabled - end - end + !emails_enabled? end def emails_enabled? - !emails_disabled? + # If no namespace_settings, we can assume it has not changed from enabled + return true unless namespace_settings + + strong_memoize(:emails_enabled_memoized) do + namespace_settings.emails_enabled? + end end def lfs_enabled? diff --git a/app/models/namespace_setting.rb b/app/models/namespace_setting.rb index 3befcdeaec5..13d2c5a62e2 100644 --- a/app/models/namespace_setting.rb +++ b/app/models/namespace_setting.rb @@ -63,6 +63,12 @@ class NamespaceSetting < ApplicationRecord namespace.root_ancestor.prevent_sharing_groups_outside_hierarchy end + def emails_enabled? + return emails_enabled unless namespace.has_parent? + + all_ancestors_have_emails_enabled? + end + def show_diff_preview_in_email? return show_diff_preview_in_email unless namespace.has_parent? @@ -89,6 +95,10 @@ class NamespaceSetting < ApplicationRecord private + def all_ancestors_have_emails_enabled? + self.class.where(namespace_id: namespace.self_and_ancestors, emails_enabled: false).none? + end + def all_ancestors_allow_diff_preview_in_email? !self.class.where(namespace_id: namespace.self_and_ancestors, show_diff_preview_in_email: false).exists? end |