diff options
52 files changed, 910 insertions, 75 deletions
diff --git a/app/assets/javascripts/monitoring/components/dashboard.vue b/app/assets/javascripts/monitoring/components/dashboard.vue index dbfb3e97c20..531d23bb6e5 100644 --- a/app/assets/javascripts/monitoring/components/dashboard.vue +++ b/app/assets/javascripts/monitoring/components/dashboard.vue @@ -19,7 +19,12 @@ import PanelType from 'ee_else_ce/monitoring/components/panel_type.vue'; import { s__ } from '~/locale'; import createFlash from '~/flash'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; -import { mergeUrlParams, redirectTo, refreshCurrentPage } from '~/lib/utils/url_utility'; +import { + mergeUrlParams, + redirectTo, + refreshCurrentPage, + updateHistory, +} from '~/lib/utils/url_utility'; import invalidUrl from '~/lib/utils/invalid_url'; import Icon from '~/vue_shared/components/icon.vue'; import DateTimePicker from '~/vue_shared/components/date_time_picker/date_time_picker.vue'; @@ -356,6 +361,14 @@ export default { refreshDashboard() { refreshCurrentPage(); }, + + onTimeRangeZoom({ start, end }) { + updateHistory({ + url: mergeUrlParams({ start, end }, window.location.href), + title: document.title, + }); + this.selectedTimeRange = { start, end }; + }, }, addMetric: { title: s__('Metrics|Add metric'), @@ -577,6 +590,7 @@ export default { :alerts-endpoint="alertsEndpoint" :prometheus-alerts-available="prometheusAlertsAvailable" :index="`${index}-${graphIndex}`" + @timerangezoom="onTimeRangeZoom" /> </div> </div> diff --git a/app/assets/javascripts/monitoring/components/panel_type.vue b/app/assets/javascripts/monitoring/components/panel_type.vue index ba92b72a71d..79f2c8ad41f 100644 --- a/app/assets/javascripts/monitoring/components/panel_type.vue +++ b/app/assets/javascripts/monitoring/components/panel_type.vue @@ -23,6 +23,10 @@ import MonitorEmptyChart from './charts/empty_chart.vue'; import TrackEventDirective from '~/vue_shared/directives/track_event'; import { timeRangeToUrl, downloadCSVOptions, generateLinkToChartOptions } from '../utils'; +const events = { + timeRangeZoom: 'timerangezoom', +}; + export default { components: { MonitorSingleStatChart, @@ -159,6 +163,7 @@ export default { }, onDatazoom({ start, end }) { this.zoomedTimeRange = { start, end }; + this.$emit(events.timeRangeZoom, { start, end }); }, }, }; diff --git a/app/assets/javascripts/mr_notes/index.js b/app/assets/javascripts/mr_notes/index.js index 8eccba07c38..c1edf7be870 100644 --- a/app/assets/javascripts/mr_notes/index.js +++ b/app/assets/javascripts/mr_notes/index.js @@ -4,6 +4,7 @@ import initNotesApp from './init_notes'; import initDiffsApp from '../diffs'; import discussionCounter from '../notes/components/discussion_counter.vue'; import initDiscussionFilters from '../notes/discussion_filters'; +import initSortDiscussions from '../notes/sort_discussions'; import MergeRequest from '../merge_request'; import { resetServiceWorkersPublicPath } from '../lib/utils/webpack'; @@ -32,5 +33,6 @@ export default function initMrNotes() { }); initDiscussionFilters(store); + initSortDiscussions(store); initDiffsApp(store); } diff --git a/app/assets/javascripts/notes/components/notes_app.vue b/app/assets/javascripts/notes/components/notes_app.vue index 762228dd138..c1dd56aedf2 100644 --- a/app/assets/javascripts/notes/components/notes_app.vue +++ b/app/assets/javascripts/notes/components/notes_app.vue @@ -12,6 +12,7 @@ import commentForm from './comment_form.vue'; import placeholderNote from '../../vue_shared/components/notes/placeholder_note.vue'; import placeholderSystemNote from '../../vue_shared/components/notes/placeholder_system_note.vue'; import skeletonLoadingContainer from '../../vue_shared/components/notes/skeleton_note.vue'; +import OrderedLayout from '~/vue_shared/components/ordered_layout.vue'; import highlightCurrentUser from '~/behaviors/markdown/highlight_current_user'; import { __ } from '~/locale'; import initUserPopovers from '~/user_popovers'; @@ -27,6 +28,7 @@ export default { placeholderSystemNote, skeletonLoadingContainer, discussionFilterNote, + OrderedLayout, }, props: { noteableData: { @@ -70,7 +72,11 @@ export default { 'getNoteableData', 'userCanReply', 'discussionTabCounter', + 'sortDirection', ]), + sortDirDesc() { + return this.sortDirection === constants.DESC; + }, discussionTabCounterText() { return this.isLoading ? '' : this.discussionTabCounter; }, @@ -91,6 +97,9 @@ export default { canReply() { return this.userCanReply && !this.commentsDisabled; }, + slotKeys() { + return this.sortDirDesc ? ['form', 'comments'] : ['comments', 'form']; + }, }, watch: { shouldShow() { @@ -156,6 +165,9 @@ export default { 'convertToDiscussion', 'stopPolling', ]), + discussionIsIndividualNoteAndNotConverted(discussion) { + return discussion.individual_note && !this.convertedDisscussionIds.includes(discussion.id); + }, handleHashChanged() { const noteId = this.checkLocationHash(); @@ -232,44 +244,51 @@ export default { <template> <div v-show="shouldShow" id="notes"> - <ul id="notes-list" class="notes main-notes-list timeline"> - <template v-for="discussion in allDiscussions"> - <skeleton-loading-container v-if="discussion.isSkeletonNote" :key="discussion.id" /> - <template v-else-if="discussion.isPlaceholderNote"> - <placeholder-system-note - v-if="discussion.placeholderType === $options.systemNote" - :key="discussion.id" - :note="discussion.notes[0]" - /> - <placeholder-note v-else :key="discussion.id" :note="discussion.notes[0]" /> - </template> - <template - v-else-if="discussion.individual_note && !convertedDisscussionIds.includes(discussion.id)" - > - <system-note - v-if="discussion.notes[0].system" - :key="discussion.id" - :note="discussion.notes[0]" - /> - <noteable-note - v-else - :key="discussion.id" - :note="discussion.notes[0]" - :show-reply-button="canReply" - @startReplying="startReplying(discussion.id)" - /> - </template> - <noteable-discussion - v-else - :key="discussion.id" - :discussion="discussion" - :render-diff-file="true" - :help-page-path="helpPagePath" + <ordered-layout :slot-keys="slotKeys"> + <template #form> + <comment-form + v-if="!commentsDisabled" + class="js-comment-form" + :noteable-type="noteableType" /> </template> - <discussion-filter-note v-show="commentsDisabled" /> - </ul> - - <comment-form v-if="!commentsDisabled" :noteable-type="noteableType" /> + <template #comments> + <ul id="notes-list" class="notes main-notes-list timeline"> + <template v-for="discussion in allDiscussions"> + <skeleton-loading-container v-if="discussion.isSkeletonNote" :key="discussion.id" /> + <template v-else-if="discussion.isPlaceholderNote"> + <placeholder-system-note + v-if="discussion.placeholderType === $options.systemNote" + :key="discussion.id" + :note="discussion.notes[0]" + /> + <placeholder-note v-else :key="discussion.id" :note="discussion.notes[0]" /> + </template> + <template v-else-if="discussionIsIndividualNoteAndNotConverted(discussion)"> + <system-note + v-if="discussion.notes[0].system" + :key="discussion.id" + :note="discussion.notes[0]" + /> + <noteable-note + v-else + :key="discussion.id" + :note="discussion.notes[0]" + :show-reply-button="canReply" + @startReplying="startReplying(discussion.id)" + /> + </template> + <noteable-discussion + v-else + :key="discussion.id" + :discussion="discussion" + :render-diff-file="true" + :help-page-path="helpPagePath" + /> + </template> + <discussion-filter-note v-show="commentsDisabled" /> + </ul> + </template> + </ordered-layout> </div> </template> diff --git a/app/assets/javascripts/notes/components/sort_discussion.vue b/app/assets/javascripts/notes/components/sort_discussion.vue new file mode 100644 index 00000000000..16eded52763 --- /dev/null +++ b/app/assets/javascripts/notes/components/sort_discussion.vue @@ -0,0 +1,64 @@ +<script> +import { GlIcon } from '@gitlab/ui'; +import { mapActions, mapGetters } from 'vuex'; +import { __ } from '~/locale'; +import { ASC, DESC } from '../constants'; + +const SORT_OPTIONS = [ + { key: DESC, text: __('Newest first'), cls: 'js-newest-first' }, + { key: ASC, text: __('Oldest first'), cls: 'js-oldest-first' }, +]; + +export default { + SORT_OPTIONS, + components: { + GlIcon, + }, + computed: { + ...mapGetters(['sortDirection']), + selectedOption() { + return SORT_OPTIONS.find(({ key }) => this.sortDirection === key); + }, + dropdownText() { + return this.selectedOption.text; + }, + }, + methods: { + ...mapActions(['setDiscussionSortDirection']), + fetchSortedDiscussions(direction) { + if (this.isDropdownItemActive(direction)) { + return; + } + + this.setDiscussionSortDirection(direction); + }, + isDropdownItemActive(sortDir) { + return sortDir === this.sortDirection; + }, + }, +}; +</script> + +<template> + <div class="mr-2 d-inline-block align-bottom full-width-mobile"> + <button class="btn btn-sm js-dropdown-text" data-toggle="dropdown" aria-expanded="false"> + {{ dropdownText }} + <gl-icon name="chevron-down" /> + </button> + <div ref="dropdownMenu" class="dropdown-menu dropdown-menu-selectable dropdown-menu-right"> + <div class="dropdown-content"> + <ul> + <li v-for="{ text, key, cls } in $options.SORT_OPTIONS" :key="key"> + <button + :class="[cls, { 'is-active': isDropdownItemActive(key) }]" + type="button" + @click="fetchSortedDiscussions(key)" + > + {{ text }} + </button> + </li> + </ul> + </div> + </div> + </div> +</template> diff --git a/app/assets/javascripts/notes/constants.js b/app/assets/javascripts/notes/constants.js index e9a81bc9553..c1449237f8a 100644 --- a/app/assets/javascripts/notes/constants.js +++ b/app/assets/javascripts/notes/constants.js @@ -19,6 +19,8 @@ export const DISCUSSION_FILTERS_DEFAULT_VALUE = 0; export const DISCUSSION_TAB_LABEL = 'show'; export const NOTE_UNDERSCORE = 'note_'; export const TIME_DIFFERENCE_VALUE = 10; +export const ASC = 'asc'; +export const DESC = 'desc'; export const NOTEABLE_TYPE_MAPPING = { Issue: ISSUE_NOTEABLE_TYPE, diff --git a/app/assets/javascripts/notes/index.js b/app/assets/javascripts/notes/index.js index 30372103590..8f9e2359e0d 100644 --- a/app/assets/javascripts/notes/index.js +++ b/app/assets/javascripts/notes/index.js @@ -1,6 +1,7 @@ import Vue from 'vue'; import notesApp from './components/notes_app.vue'; import initDiscussionFilters from './discussion_filters'; +import initSortDiscussions from './sort_discussions'; import createStore from './stores'; document.addEventListener('DOMContentLoaded', () => { @@ -50,4 +51,5 @@ document.addEventListener('DOMContentLoaded', () => { }); initDiscussionFilters(store); + initSortDiscussions(store); }); diff --git a/app/assets/javascripts/notes/sort_discussions.js b/app/assets/javascripts/notes/sort_discussions.js new file mode 100644 index 00000000000..a06c23f5f76 --- /dev/null +++ b/app/assets/javascripts/notes/sort_discussions.js @@ -0,0 +1,16 @@ +import Vue from 'vue'; +import SortDiscussion from './components/sort_discussion.vue'; + +export default store => { + const el = document.getElementById('js-vue-sort-issue-discussions'); + + if (!el) return null; + + return new Vue({ + el, + store, + render(createElement) { + return createElement(SortDiscussion); + }, + }); +}; diff --git a/app/assets/javascripts/notes/stores/actions.js b/app/assets/javascripts/notes/stores/actions.js index accc37121d0..1b80b59621a 100644 --- a/app/assets/javascripts/notes/stores/actions.js +++ b/app/assets/javascripts/notes/stores/actions.js @@ -69,6 +69,10 @@ export const updateDiscussion = ({ commit, state }, discussion) => { return utils.findNoteObjectById(state.discussions, discussion.id); }; +export const setDiscussionSortDirection = ({ commit }, direction) => { + commit(types.SET_DISCUSSIONS_SORT, direction); +}; + export const removeNote = ({ commit, dispatch, state }, note) => { const discussion = state.discussions.find(({ id }) => id === note.discussion_id); diff --git a/app/assets/javascripts/notes/stores/getters.js b/app/assets/javascripts/notes/stores/getters.js index 28cc9cdd7e9..eb877083bca 100644 --- a/app/assets/javascripts/notes/stores/getters.js +++ b/app/assets/javascripts/notes/stores/getters.js @@ -1,8 +1,16 @@ -import { flattenDeep } from 'lodash'; +import { flattenDeep, clone } from 'lodash'; import * as constants from '../constants'; import { collapseSystemNotes } from './collapse_utils'; -export const discussions = state => collapseSystemNotes(state.discussions); +export const discussions = state => { + let discussionsInState = clone(state.discussions); + // NOTE: not testing bc will be removed when backend is finished. + if (state.discussionSortOrder === constants.DESC) { + discussionsInState = discussionsInState.reverse(); + } + + return collapseSystemNotes(discussionsInState); +}; export const convertedDisscussionIds = state => state.convertedDisscussionIds; @@ -12,6 +20,13 @@ export const getNotesData = state => state.notesData; export const isNotesFetched = state => state.isNotesFetched; +/* + * WARNING: This is an example of an "unnecessary" getter + * more info found here: https://gitlab.com/groups/gitlab-org/-/epics/2913. + */ + +export const sortDirection = state => state.discussionSortOrder; + export const isLoading = state => state.isLoading; export const getNotesDataByProp = state => prop => state.notesData[prop]; diff --git a/app/assets/javascripts/notes/stores/modules/index.js b/app/assets/javascripts/notes/stores/modules/index.js index 2d317dcd7da..81844ad6e98 100644 --- a/app/assets/javascripts/notes/stores/modules/index.js +++ b/app/assets/javascripts/notes/stores/modules/index.js @@ -1,10 +1,12 @@ import * as actions from '../actions'; import * as getters from '../getters'; import mutations from '../mutations'; +import { ASC } from '../../constants'; export default () => ({ state: { discussions: [], + discussionSortOrder: ASC, convertedDisscussionIds: [], targetNoteHash: null, lastFetchedAt: null, diff --git a/app/assets/javascripts/notes/stores/mutation_types.js b/app/assets/javascripts/notes/stores/mutation_types.js index 0cc59f9150c..5b7225bb3d2 100644 --- a/app/assets/javascripts/notes/stores/mutation_types.js +++ b/app/assets/javascripts/notes/stores/mutation_types.js @@ -27,6 +27,7 @@ export const TOGGLE_DISCUSSION = 'TOGGLE_DISCUSSION'; export const SET_EXPAND_DISCUSSIONS = 'SET_EXPAND_DISCUSSIONS'; export const UPDATE_RESOLVABLE_DISCUSSIONS_COUNTS = 'UPDATE_RESOLVABLE_DISCUSSIONS_COUNTS'; export const SET_CURRENT_DISCUSSION_ID = 'SET_CURRENT_DISCUSSION_ID'; +export const SET_DISCUSSIONS_SORT = 'SET_DISCUSSIONS_SORT'; // Issue export const CLOSE_ISSUE = 'CLOSE_ISSUE'; diff --git a/app/assets/javascripts/notes/stores/mutations.js b/app/assets/javascripts/notes/stores/mutations.js index 68bf8394508..028fc87198c 100644 --- a/app/assets/javascripts/notes/stores/mutations.js +++ b/app/assets/javascripts/notes/stores/mutations.js @@ -263,6 +263,10 @@ export default { discussion.truncated_diff_lines = utils.prepareDiffLines(diffLines); }, + [types.SET_DISCUSSIONS_SORT](state, sort) { + state.discussionSortOrder = sort; + }, + [types.DISABLE_COMMENTS](state, value) { state.commentsDisabled = value; }, diff --git a/app/assets/javascripts/vue_shared/components/ordered_layout.vue b/app/assets/javascripts/vue_shared/components/ordered_layout.vue new file mode 100644 index 00000000000..117e79ca39f --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/ordered_layout.vue @@ -0,0 +1,12 @@ +<script> +export default { + functional: true, + render(h, context) { + const { slotKeys } = context.props; + const slots = context.slots(); + const children = slotKeys.map(key => slots[key]).filter(x => x); + + return children; + }, +}; +</script> diff --git a/app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/dropdown_contents_create_view.vue b/app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/dropdown_contents_create_view.vue index 285a0fe9ffb..842b2fdbc43 100644 --- a/app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/dropdown_contents_create_view.vue +++ b/app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/dropdown_contents_create_view.vue @@ -58,13 +58,13 @@ export default { </script> <template> - <div class="labels-select-contents-create"> + <div class="labels-select-contents-create js-labels-create"> <div class="dropdown-title d-flex align-items-center pt-0 pb-2"> <gl-button :aria-label="__('Go back')" variant="link" size="sm" - class="dropdown-header-button p-0" + class="js-btn-back dropdown-header-button p-0" @click="toggleDropdownContentsCreateView" > <gl-icon name="arrow-left" /> @@ -116,7 +116,7 @@ export default { <gl-loading-icon v-show="labelCreateInProgress" :inline="true" class="mr-1" /> {{ __('Create') }} </gl-button> - <gl-button class="pull-right" @click="toggleDropdownContentsCreateView"> + <gl-button class="pull-right js-btn-cancel-create" @click="toggleDropdownContentsCreateView"> {{ __('Cancel') }} </gl-button> </div> diff --git a/app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/dropdown_contents_labels_view.vue b/app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/dropdown_contents_labels_view.vue index 7ec420fa908..a3494a9e38f 100644 --- a/app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/dropdown_contents_labels_view.vue +++ b/app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/dropdown_contents_labels_view.vue @@ -117,7 +117,7 @@ export default { </script> <template> - <div class="labels-select-contents-list" @keydown="handleKeyDown"> + <div class="labels-select-contents-list js-labels-list" @keydown="handleKeyDown"> <gl-loading-icon v-if="labelsFetchInProgress" class="labels-fetch-loading position-absolute d-flex align-items-center w-100 h-100" diff --git a/app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/labels_select_root.vue b/app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/labels_select_root.vue index 5e41a155ef6..78102caacf5 100644 --- a/app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/labels_select_root.vue +++ b/app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/labels_select_root.vue @@ -1,4 +1,5 @@ <script> +import $ from 'jquery'; import Vue from 'vue'; import Vuex, { mapState, mapActions } from 'vuex'; import { __ } from '~/locale'; @@ -149,9 +150,16 @@ export default { * the dropdown while dropdown is visible. */ handleDocumentClick({ target }) { + // This approach of element detection is needed + // as the dropdown wrapper is not using `GlDropdown` as + // it will also require us to use `BDropdownForm` + // which is yet to be implemented in GitLab UI. if ( this.showDropdownButton && this.showDropdownContents && + !$(target).parents('.js-btn-back').length && + !$(target).parents('.js-labels-list').length && + !target?.classList.contains('js-btn-cancel-create') && !target?.classList.contains('js-sidebar-dropdown-toggle') && !this.$refs.dropdownButtonCollapsed?.$el.contains(target) && !this.$refs.dropdownContents?.$el.contains(target) diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 5ddc60707d5..d813d7b8225 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -47,6 +47,10 @@ class Projects::IssuesController < Projects::ApplicationController push_frontend_feature_flag(:save_issuable_health_status, project.group) end + before_action only: :show do + push_frontend_feature_flag(:sort_discussions, @project) + end + around_action :allow_gitaly_ref_name_caching, only: [:discussions] respond_to :html diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index e87f1728cbb..a81fd0f7dc9 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -29,6 +29,10 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo push_frontend_feature_flag(:vue_issuable_sidebar, @project.group) end + before_action only: :show do + push_frontend_feature_flag(:sort_discussions, @project) + end + around_action :allow_gitaly_ref_name_caching, only: [:index, :show, :discussions] def index diff --git a/app/models/member.rb b/app/models/member.rb index 089efcb81dd..a87efdf63ff 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -100,6 +100,7 @@ class Member < ApplicationRecord after_destroy :destroy_notification_setting after_destroy :post_destroy_hook, unless: :pending? after_commit :refresh_member_authorized_projects + after_commit :update_highest_role default_value_for :notification_level, NotificationSetting.levels[:global] @@ -459,6 +460,22 @@ class Member < ApplicationRecord errors.add(:access_level, s_("should be greater than or equal to %{access} inherited membership from group %{group_name}") % error_parameters) end end + + # Triggers the service to schedule a Sidekiq job to update the highest role + # for a User + # + # The job will be called outside of a transaction in order to ensure the changes + # for a Member to be commited before attempting to update the highest role. + # rubocop: disable CodeReuse/ServiceClass + def update_highest_role + return unless user_id.present? + return unless previous_changes[:access_level].present? + + run_after_commit_or_now do + Members::UpdateHighestRoleService.new(user_id).execute + end + end + # rubocop: enable CodeReuse/ServiceClass end Member.prepend_if_ee('EE::Member') diff --git a/app/models/user.rb b/app/models/user.rb index 090d033f80c..9fb3c47e143 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1694,6 +1694,11 @@ class User < ApplicationRecord end end + # Load the current highest access by looking directly at the user's memberships + def current_highest_access_level + members.non_request.maximum(:access_level) + end + protected # override, from Devise::Validatable diff --git a/app/services/members/update_highest_role_service.rb b/app/services/members/update_highest_role_service.rb new file mode 100644 index 00000000000..ce1707c13ec --- /dev/null +++ b/app/services/members/update_highest_role_service.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module Members + class UpdateHighestRoleService < ::BaseService + include ExclusiveLeaseGuard + + LEASE_TIMEOUT = 30.minutes.to_i + + attr_reader :user_id + + def initialize(user_id) + @user_id = user_id + end + + def execute + try_obtain_lease do + UpdateHighestRoleWorker.perform_async(user_id) + end + end + + private + + def lease_key + "update_highest_role:#{user_id}" + end + + def lease_timeout + LEASE_TIMEOUT + end + + # Do not release the lease before the timeout to + # prevent multiple jobs being executed during the + # defined timeout + def lease_release? + false + end + end +end diff --git a/app/services/users/update_highest_member_role_service.rb b/app/services/users/update_highest_member_role_service.rb new file mode 100644 index 00000000000..00f0af89e8d --- /dev/null +++ b/app/services/users/update_highest_member_role_service.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module Users + class UpdateHighestMemberRoleService < BaseService + attr_reader :user, :identity_params + + def initialize(user) + @user = user + end + + def execute + return true if user_highest_role.persisted? && highest_access_level == user_highest_role.highest_access_level + + user_highest_role.update!(highest_access_level: highest_access_level) + end + + private + + def user_highest_role + @user_highest_role ||= begin + @user.user_highest_role || @user.build_user_highest_role + end + end + + def highest_access_level + @highest_access_level ||= @user.current_highest_access_level + end + end +end diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index 9062f2097b8..f7702499970 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -87,6 +87,8 @@ .col-md-12.col-lg-6.js-noteable-awards = render 'award_emoji/awards_block', awardable: @issue, inline: true .col-md-12.col-lg-6.new-branch-col + - if Feature.enabled?(:sort_discussions, @project) + #js-vue-sort-issue-discussions #js-vue-discussion-filter{ data: { default_filter: current_user&.notes_filter_for(@issue), notes_filters: UserPreference.notes_filters.to_json } } = render 'new_branch' if show_new_branch_button? diff --git a/app/views/projects/merge_requests/_awards_block.html.haml b/app/views/projects/merge_requests/_awards_block.html.haml index 1eab28a2ff3..c1e92e22590 100644 --- a/app/views/projects/merge_requests/_awards_block.html.haml +++ b/app/views/projects/merge_requests/_awards_block.html.haml @@ -2,4 +2,6 @@ = render 'award_emoji/awards_block', awardable: @merge_request, inline: true do - if mr_tabs_position_enabled? .ml-auto.mt-auto.mb-auto + - if Feature.enabled?(:sort_discussions, @merge_request.target_project) + #js-vue-sort-issue-discussions = render "projects/merge_requests/discussion_filter" diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 72a5a2b653e..16520519a0b 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -1333,6 +1333,13 @@ :resource_boundary: :unknown :weight: 3 :idempotent: +- :name: update_highest_role + :feature_category: :authentication_and_authorization + :has_external_dependencies: + :urgency: :high + :resource_boundary: :unknown + :weight: 2 + :idempotent: true - :name: update_merge_requests :feature_category: :source_code_management :has_external_dependencies: diff --git a/app/workers/update_highest_role_worker.rb b/app/workers/update_highest_role_worker.rb new file mode 100644 index 00000000000..e62131a77d0 --- /dev/null +++ b/app/workers/update_highest_role_worker.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class UpdateHighestRoleWorker + include ApplicationWorker + + feature_category :authentication_and_authorization + urgency :high + weight 2 + + idempotent! + + # rubocop: disable CodeReuse/ActiveRecord + def perform(user_id) + user = User.active.find_by(id: user_id) + + Users::UpdateHighestMemberRoleService.new(user).execute if user.present? + end + # rubocop: enable CodeReuse/ActiveRecord +end diff --git a/changelogs/unreleased/jivanvl-change-url-time-slider.yml b/changelogs/unreleased/jivanvl-change-url-time-slider.yml new file mode 100644 index 00000000000..605ab02640b --- /dev/null +++ b/changelogs/unreleased/jivanvl-change-url-time-slider.yml @@ -0,0 +1,5 @@ +--- +title: Change the url when the timeslider changes +merge_request: 27726 +author: +type: changed diff --git a/changelogs/unreleased/update_user_highest_roles_table.yml b/changelogs/unreleased/update_user_highest_roles_table.yml new file mode 100644 index 00000000000..49d1fec65aa --- /dev/null +++ b/changelogs/unreleased/update_user_highest_roles_table.yml @@ -0,0 +1,5 @@ +--- +title: Update user's highest role to keep the users statistics up to date +merge_request: 27231 +author: +type: added diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index d4f391e75bb..40e09d6196e 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -244,6 +244,8 @@ - 1 - - update_external_pull_requests - 3 +- - update_highest_role + - 2 - - update_merge_requests - 3 - - update_namespace_statistics diff --git a/db/migrate/20200312125121_add_index_on_active_and_template_and_type_and_id_to_services.rb b/db/migrate/20200312125121_add_index_on_active_and_template_and_type_and_id_to_services.rb index 3a2390bb6a1..76e5860c2f3 100644 --- a/db/migrate/20200312125121_add_index_on_active_and_template_and_type_and_id_to_services.rb +++ b/db/migrate/20200312125121_add_index_on_active_and_template_and_type_and_id_to_services.rb @@ -13,6 +13,6 @@ class AddIndexOnActiveAndTemplateAndTypeAndIdToServices < ActiveRecord::Migratio end def down - remove_concurrent_index :services, INDEX_NAME + remove_concurrent_index_by_name :services, INDEX_NAME end end diff --git a/db/migrate/20200313123934_add_index_on_user_id_type_source_type_ldap_and_created_at_to_members.rb b/db/migrate/20200313123934_add_index_on_user_id_type_source_type_ldap_and_created_at_to_members.rb index 0215d102529..c4dde7086c2 100644 --- a/db/migrate/20200313123934_add_index_on_user_id_type_source_type_ldap_and_created_at_to_members.rb +++ b/db/migrate/20200313123934_add_index_on_user_id_type_source_type_ldap_and_created_at_to_members.rb @@ -13,6 +13,6 @@ class AddIndexOnUserIdTypeSourceTypeLdapAndCreatedAtToMembers < ActiveRecord::Mi end def down - remove_concurrent_index :members, INDEX_NAME + remove_concurrent_index_by_name :members, INDEX_NAME end end diff --git a/db/migrate/20200323122201_add_index_on_user_and_created_at_to_ci_builds.rb b/db/migrate/20200323122201_add_index_on_user_and_created_at_to_ci_builds.rb index 4f41fc4f478..d0f63034502 100644 --- a/db/migrate/20200323122201_add_index_on_user_and_created_at_to_ci_builds.rb +++ b/db/migrate/20200323122201_add_index_on_user_and_created_at_to_ci_builds.rb @@ -14,6 +14,6 @@ class AddIndexOnUserAndCreatedAtToCiBuilds < ActiveRecord::Migration[6.0] end def down - remove_concurrent_index :ci_builds, INDEX_NAME + remove_concurrent_index_by_name :ci_builds, INDEX_NAME end end diff --git a/doc/user/project/deploy_boards.md b/doc/user/project/deploy_boards.md index fe2c8bbf4ed..c2d7fe89833 100644 --- a/doc/user/project/deploy_boards.md +++ b/doc/user/project/deploy_boards.md @@ -102,6 +102,34 @@ navigate to the environments page under **Operations > Environments**. Deploy Boards are visible by default. You can explicitly click the triangle next to their respective environment name in order to hide them. +### Example manifest file + +The following example is an extract of a Kubernetes manifest deployment file, using the two annotations `app.gitlab.com/env` and `app.gitlab.com/app` to enable the **Deploy Boards**: + +```yaml +apiVersion: apps/v1 +kind: Deployment +metadata: + name: "APPLICATION_NAME" + annotations: + app.gitlab.com/app: ${CI_PROJECT_PATH_SLUG} + app.gitlab.com/env: ${CI_ENVIRONMENT_SLUG} +spec: + replicas: 1 + selector: + matchLabels: + app: "APPLICATION_NAME" + template: + metadata: + labels: + app: "APPLICATION_NAME" + annotations: + app.gitlab.com/app: ${CI_PROJECT_PATH_SLUG} + app.gitlab.com/env: ${CI_ENVIRONMENT_SLUG} +``` + +The annotations will be applied to the deployments, replica sets, and pods. By changing the number of replicas, like `kubectl scale --replicas=3 deploy APPLICATION_NAME -n ${KUBE_NAMESPACE}`, you can follow the instances' pods from the board. + ## Canary Deployments A popular CI strategy, where a small portion of the fleet is updated to the new diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d4f1cc75a49..19fe4f77c85 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -13213,6 +13213,9 @@ msgstr "" msgid "New..." msgstr "" +msgid "Newest first" +msgstr "" + msgid "Newly registered users will by default be external" msgstr "" @@ -13693,6 +13696,9 @@ msgstr "" msgid "Ok let's go" msgstr "" +msgid "Oldest first" +msgstr "" + msgid "OmniAuth" msgstr "" @@ -20026,6 +20032,9 @@ msgstr "" msgid "The vulnerability is no longer detected. Verify the vulnerability has been fixed or removed before changing its status." msgstr "" +msgid "The vulnerability is no longer detected. Verify the vulnerability has been remediated before changing its status." +msgstr "" + msgid "There are no GPG keys associated with this account." msgstr "" @@ -22435,6 +22444,12 @@ msgstr "" msgid "Vulnerability remediated. Review before resolving." msgstr "" +msgid "Vulnerability resolved in %{branch}" +msgstr "" + +msgid "Vulnerability resolved in the default branch" +msgstr "" + msgid "Vulnerability-Check" msgstr "" diff --git a/spec/factories/user_highest_roles.rb b/spec/factories/user_highest_roles.rb index d04db507e44..761a8b6c583 100644 --- a/spec/factories/user_highest_roles.rb +++ b/spec/factories/user_highest_roles.rb @@ -2,14 +2,13 @@ FactoryBot.define do factory :user_highest_role do + highest_access_level { nil } user - trait :maintainer do - highest_access_level { Gitlab::Access::MAINTAINER } - end - - trait :developer do - highest_access_level { Gitlab::Access::DEVELOPER } - end + trait(:guest) { highest_access_level { GroupMember::GUEST } } + trait(:reporter) { highest_access_level { GroupMember::REPORTER } } + trait(:developer) { highest_access_level { GroupMember::DEVELOPER } } + trait(:maintainer) { highest_access_level { GroupMember::MAINTAINER } } + trait(:owner) { highest_access_level { GroupMember::OWNER } } end end diff --git a/spec/finders/protected_branches_finder_spec.rb b/spec/finders/protected_branches_finder_spec.rb index e6a2cf4577c..c6b9964b6c5 100644 --- a/spec/finders/protected_branches_finder_spec.rb +++ b/spec/finders/protected_branches_finder_spec.rb @@ -30,7 +30,7 @@ describe ProtectedBranchesFinder do end it 'returns limited protected branches of project' do - expect(subject).to eq([another_protected_branch]) + expect(subject.count).to eq(1) end end end diff --git a/spec/frontend/monitoring/components/dashboard_url_time_spec.js b/spec/frontend/monitoring/components/dashboard_url_time_spec.js index bf5a11a536e..ebfa09874fa 100644 --- a/spec/frontend/monitoring/components/dashboard_url_time_spec.js +++ b/spec/frontend/monitoring/components/dashboard_url_time_spec.js @@ -1,7 +1,13 @@ import { mount } from '@vue/test-utils'; import MockAdapter from 'axios-mock-adapter'; import createFlash from '~/flash'; -import { queryToObject, redirectTo, removeParams, mergeUrlParams } from '~/lib/utils/url_utility'; +import { + queryToObject, + redirectTo, + removeParams, + mergeUrlParams, + updateHistory, +} from '~/lib/utils/url_utility'; import axios from '~/lib/utils/axios_utils'; import { mockProjectDir } from '../mock_data'; @@ -137,4 +143,23 @@ describe('dashboard invalid url parameters', () => { expect(redirectTo).toHaveBeenCalledTimes(1); }); }); + + it('changes the url when a panel moves the time slider', () => { + const timeRange = { + start: '2020-01-01T00:00:00.000Z', + end: '2020-01-01T01:00:00.000Z', + }; + + queryToObject.mockReturnValue(timeRange); + + createMountedWrapper(); + + return wrapper.vm.$nextTick().then(() => { + wrapper.vm.onTimeRangeZoom(timeRange); + + expect(updateHistory).toHaveBeenCalled(); + expect(wrapper.vm.selectedTimeRange.start.toString()).toBe(timeRange.start); + expect(wrapper.vm.selectedTimeRange.end.toString()).toBe(timeRange.end); + }); + }); }); diff --git a/spec/frontend/monitoring/components/panel_type_spec.js b/spec/frontend/monitoring/components/panel_type_spec.js index 058c201d325..927d93ab697 100644 --- a/spec/frontend/monitoring/components/panel_type_spec.js +++ b/spec/frontend/monitoring/components/panel_type_spec.js @@ -99,6 +99,8 @@ describe('Panel Type component', () => { }); describe('when graph data is available', () => { + const findTimeChart = () => wrapper.find({ ref: 'timeChart' }); + beforeEach(() => { createWrapper({ graphData: graphDataPrometheusQueryRange, @@ -122,6 +124,21 @@ describe('Panel Type component', () => { expect(findCopyLink().exists()).toBe(false); }); + it('should emit `timerange` event when a zooming in/out in a chart occcurs', () => { + const timeRange = { + start: '2020-01-01T00:00:00.000Z', + end: '2020-01-01T01:00:00.000Z', + }; + + jest.spyOn(wrapper.vm, '$emit'); + + findTimeChart().vm.$emit('datazoom', timeRange); + + return wrapper.vm.$nextTick(() => { + expect(wrapper.vm.$emit).toHaveBeenCalledWith('timerangezoom', timeRange); + }); + }); + describe('Time Series Chart panel type', () => { it('is rendered', () => { expect(wrapper.find(TimeSeriesChart).isVueInstance()).toBe(true); diff --git a/spec/frontend/notes/components/notes_app_spec.js b/spec/frontend/notes/components/notes_app_spec.js index 60e866542a6..e22dd85f221 100644 --- a/spec/frontend/notes/components/notes_app_spec.js +++ b/spec/frontend/notes/components/notes_app_spec.js @@ -1,26 +1,44 @@ import $ from 'jquery'; import AxiosMockAdapter from 'axios-mock-adapter'; import Vue from 'vue'; -import { mount } from '@vue/test-utils'; +import { mount, shallowMount } from '@vue/test-utils'; import { setTestTimeout } from 'helpers/timeout'; import axios from '~/lib/utils/axios_utils'; import NotesApp from '~/notes/components/notes_app.vue'; +import CommentForm from '~/notes/components/comment_form.vue'; import createStore from '~/notes/stores'; +import * as constants from '~/notes/constants'; import '~/behaviors/markdown/render_gfm'; // TODO: use generated fixture (https://gitlab.com/gitlab-org/gitlab-foss/issues/62491) import * as mockData from '../../notes/mock_data'; import * as urlUtility from '~/lib/utils/url_utility'; +import OrderedLayout from '~/vue_shared/components/ordered_layout.vue'; jest.mock('~/user_popovers', () => jest.fn()); setTestTimeout(1000); +const TYPE_COMMENT_FORM = 'comment-form'; +const TYPE_NOTES_LIST = 'notes-list'; + +const propsData = { + noteableData: mockData.noteableDataMock, + notesData: mockData.notesDataMock, + userData: mockData.userDataMock, +}; + describe('note_app', () => { let axiosMock; let mountComponent; let wrapper; let store; + const getComponentOrder = () => { + return wrapper + .findAll('#notes-list,.js-comment-form') + .wrappers.map(node => (node.is(CommentForm) ? TYPE_COMMENT_FORM : TYPE_NOTES_LIST)); + }; + /** * waits for fetchNotes() to complete */ @@ -43,13 +61,7 @@ describe('note_app', () => { axiosMock = new AxiosMockAdapter(axios); store = createStore(); - mountComponent = data => { - const propsData = data || { - noteableData: mockData.noteableDataMock, - notesData: mockData.notesDataMock, - userData: mockData.userDataMock, - }; - + mountComponent = () => { return mount( { components: { @@ -346,4 +358,39 @@ describe('note_app', () => { expect(setTargetNoteHash).toHaveBeenCalled(); }); }); + + describe('when sort direction is desc', () => { + beforeEach(() => { + store = createStore(); + store.state.discussionSortOrder = constants.DESC; + wrapper = shallowMount(NotesApp, { + propsData, + store, + stubs: { + 'ordered-layout': OrderedLayout, + }, + }); + }); + + it('finds CommentForm before notes list', () => { + expect(getComponentOrder()).toStrictEqual([TYPE_COMMENT_FORM, TYPE_NOTES_LIST]); + }); + }); + + describe('when sort direction is asc', () => { + beforeEach(() => { + store = createStore(); + wrapper = shallowMount(NotesApp, { + propsData, + store, + stubs: { + 'ordered-layout': OrderedLayout, + }, + }); + }); + + it('finds CommentForm after notes list', () => { + expect(getComponentOrder()).toStrictEqual([TYPE_NOTES_LIST, TYPE_COMMENT_FORM]); + }); + }); }); diff --git a/spec/frontend/notes/components/sort_discussion_spec.js b/spec/frontend/notes/components/sort_discussion_spec.js new file mode 100644 index 00000000000..785e8c75233 --- /dev/null +++ b/spec/frontend/notes/components/sort_discussion_spec.js @@ -0,0 +1,72 @@ +import { shallowMount, createLocalVue } from '@vue/test-utils'; +import Vuex from 'vuex'; +import SortDiscussion from '~/notes/components/sort_discussion.vue'; +import createStore from '~/notes/stores'; +import { ASC, DESC } from '~/notes/constants'; + +const localVue = createLocalVue(); +localVue.use(Vuex); + +describe('Sort Discussion component', () => { + let wrapper; + let store; + + const createComponent = () => { + jest.spyOn(store, 'dispatch').mockImplementation(); + + wrapper = shallowMount(SortDiscussion, { + localVue, + store, + }); + }; + + beforeEach(() => { + store = createStore(); + }); + + afterEach(() => { + wrapper.destroy(); + wrapper = null; + }); + + describe('when asc', () => { + describe('when the dropdown is clicked', () => { + it('calls the right actions', () => { + createComponent(); + + wrapper.find('.js-newest-first').trigger('click'); + + expect(store.dispatch).toHaveBeenCalledWith('setDiscussionSortDirection', DESC); + }); + }); + + it('shows the "Oldest First" as the dropdown', () => { + createComponent(); + + expect(wrapper.find('.js-dropdown-text').text()).toBe('Oldest first'); + }); + }); + + describe('when desc', () => { + beforeEach(() => { + store.state.discussionSortOrder = DESC; + createComponent(); + }); + + describe('when the dropdown item is clicked', () => { + it('calls the right actions', () => { + wrapper.find('.js-oldest-first').trigger('click'); + + expect(store.dispatch).toHaveBeenCalledWith('setDiscussionSortDirection', ASC); + }); + + it('applies the active class to the correct button in the dropdown', () => { + expect(wrapper.find('.js-newest-first').classes()).toContain('is-active'); + }); + }); + + it('shows the "Newest First" as the dropdown', () => { + expect(wrapper.find('.js-dropdown-text').text()).toBe('Newest first'); + }); + }); +}); diff --git a/spec/frontend/notes/stores/actions_spec.js b/spec/frontend/notes/stores/actions_spec.js index 40b0134e12e..544d482e7fc 100644 --- a/spec/frontend/notes/stores/actions_spec.js +++ b/spec/frontend/notes/stores/actions_spec.js @@ -902,4 +902,17 @@ describe('Actions Notes Store', () => { ]); }); }); + + describe('setDiscussionSortDirection', () => { + it('calls the correct mutation with the correct args', done => { + testAction( + actions.setDiscussionSortDirection, + notesConstants.DESC, + {}, + [{ type: mutationTypes.SET_DISCUSSIONS_SORT, payload: notesConstants.DESC }], + [], + done, + ); + }); + }); }); diff --git a/spec/frontend/notes/stores/getters_spec.js b/spec/frontend/notes/stores/getters_spec.js index 602e4c70741..a07aa45d812 100644 --- a/spec/frontend/notes/stores/getters_spec.js +++ b/spec/frontend/notes/stores/getters_spec.js @@ -1,4 +1,5 @@ import * as getters from '~/notes/stores/getters'; +import { DESC } from '~/notes/constants'; import { notesDataMock, userDataMock, @@ -36,6 +37,7 @@ describe('Getters Notes Store', () => { userData: userDataMock, noteableData: noteableDataMock, descriptionVersions: 'descriptionVersions', + discussionSortOrder: DESC, }; }); @@ -392,4 +394,10 @@ describe('Getters Notes Store', () => { expect(getters.descriptionVersions(state)).toEqual('descriptionVersions'); }); }); + + describe('sortDirection', () => { + it('should return `discussionSortOrder`', () => { + expect(getters.sortDirection(state)).toBe(DESC); + }); + }); }); diff --git a/spec/frontend/notes/stores/mutation_spec.js b/spec/frontend/notes/stores/mutation_spec.js index ea5658821b1..06d2654ceca 100644 --- a/spec/frontend/notes/stores/mutation_spec.js +++ b/spec/frontend/notes/stores/mutation_spec.js @@ -1,6 +1,6 @@ import Vue from 'vue'; import mutations from '~/notes/stores/mutations'; -import { DISCUSSION_NOTE } from '~/notes/constants'; +import { DISCUSSION_NOTE, ASC, DESC } from '~/notes/constants'; import { note, discussionMock, @@ -22,7 +22,10 @@ describe('Notes Store mutations', () => { let noteData; beforeEach(() => { - state = { discussions: [] }; + state = { + discussions: [], + discussionSortOrder: ASC, + }; noteData = { expanded: true, id: note.discussion_id, @@ -34,9 +37,7 @@ describe('Notes Store mutations', () => { }); it('should add a new note to an array of notes', () => { - expect(state).toEqual({ - discussions: [noteData], - }); + expect(state).toEqual(expect.objectContaining({ discussions: [noteData] })); expect(state.discussions.length).toBe(1); }); @@ -649,4 +650,18 @@ describe('Notes Store mutations', () => { expect(state.descriptionVersions[versionId]).toBe(deleted); }); }); + + describe('SET_DISCUSSIONS_SORT', () => { + let state; + + beforeEach(() => { + state = { discussionSortOrder: ASC }; + }); + + it('sets sort order', () => { + mutations.SET_DISCUSSIONS_SORT(state, DESC); + + expect(state.discussionSortOrder).toBe(DESC); + }); + }); }); diff --git a/spec/frontend/vue_shared/components/ordered_layout_spec.js b/spec/frontend/vue_shared/components/ordered_layout_spec.js new file mode 100644 index 00000000000..e8667d9ee4a --- /dev/null +++ b/spec/frontend/vue_shared/components/ordered_layout_spec.js @@ -0,0 +1,63 @@ +import { mount } from '@vue/test-utils'; +import orderedLayout from '~/vue_shared/components/ordered_layout.vue'; + +const children = ` + <template v-slot:header> + <header></header> + </template> + <template v-slot:footer> + <footer></footer> + </template> + `; + +const TestComponent = { + components: { orderedLayout }, + template: ` + <div> + <ordered-layout v-bind="$attrs"> + ${children} + </ordered-layout> + </div> + `, +}; + +const regularSlotOrder = ['header', 'footer']; + +describe('Ordered Layout', () => { + let wrapper; + + const verifyOrder = () => + wrapper.findAll('footer,header').wrappers.map(x => (x.is('footer') ? 'footer' : 'header')); + + const createComponent = (props = {}) => { + wrapper = mount(TestComponent, { + propsData: props, + }); + }; + + afterEach(() => { + wrapper.destroy(); + }); + + describe('when slotKeys are in initial slot order', () => { + beforeEach(() => { + createComponent({ slotKeys: regularSlotOrder }); + }); + + it('confirms order of the component is reflective of slotKeys', () => { + expect(verifyOrder()).toEqual(regularSlotOrder); + }); + }); + + describe('when slotKeys reverse the order of the props', () => { + const reversedSlotOrder = regularSlotOrder.reverse(); + + beforeEach(() => { + createComponent({ slotKeys: reversedSlotOrder }); + }); + + it('confirms order of the component is reflective of slotKeys', () => { + expect(verifyOrder()).toEqual(reversedSlotOrder); + }); + }); +}); diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index e7f03226826..ce3ee3fcfb0 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' describe Member do + using RSpec::Parameterized::TableSyntax + describe "Associations" do it { is_expected.to belong_to(:user) } end @@ -582,4 +584,54 @@ describe Member do expect(user.authorized_projects).not_to include(project) end end + + context 'when after_commit :update_highest_role' do + where(:member_type, :source_type) do + :project_member | :project + :group_member | :group + end + + with_them do + describe 'create member' do + it 'initializes a new Members::UpdateHighestRoleService object' do + source = create(source_type) # source owner initializes a new service object too + user = create(:user) + + expect(Members::UpdateHighestRoleService).to receive(:new).with(user.id).and_call_original + + create(member_type, :guest, user: user, source_type => source) + end + end + + context 'when member exists' do + let!(:member) { create(member_type) } + + describe 'update member' do + context 'when access level was changed' do + it 'initializes a new Members::UpdateHighestRoleService object' do + expect(Members::UpdateHighestRoleService).to receive(:new).with(member.user_id).and_call_original + + member.update(access_level: Gitlab::Access::GUEST) + end + end + + context 'when access level was not changed' do + it 'does not initialize a new Members::UpdateHighestRoleService object' do + expect(Members::UpdateHighestRoleService).not_to receive(:new).with(member.user_id) + + member.update(notification_level: NotificationSetting.levels[:disabled]) + end + end + end + + describe 'destroy member' do + it 'initializes a new Members::UpdateHighestRoleService object' do + expect(Members::UpdateHighestRoleService).to receive(:new).with(member.user_id).and_call_original + + member.destroy + end + end + end + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 2586289a699..51fcee29485 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -5718,7 +5718,7 @@ describe Project do subject { project.limited_protected_branches(1) } it 'returns limited number of protected branches based on specified limit' do - expect(subject).to eq([another_protected_branch]) + expect(subject.count).to eq(1) end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 3286a891203..bbd45afadd7 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -4360,4 +4360,24 @@ describe User, :do_not_mock_admin_mode do it { is_expected.to be expected_result } end end + + describe '#current_highest_access_level' do + let_it_be(:user) { create(:user) } + + context 'when no memberships exist' do + it 'returns nil' do + expect(user.current_highest_access_level).to be_nil + end + end + + context 'when memberships exist' do + it 'returns the highest access level for non requested memberships' do + create(:group_member, :reporter, user_id: user.id) + create(:project_member, :guest, user_id: user.id) + create(:project_member, :maintainer, user_id: user.id, requested_at: Time.current) + + expect(user.current_highest_access_level).to eq(Gitlab::Access::REPORTER) + end + end + end end diff --git a/spec/services/members/update_highest_role_service_spec.rb b/spec/services/members/update_highest_role_service_spec.rb new file mode 100644 index 00000000000..b56a51f83f9 --- /dev/null +++ b/spec/services/members/update_highest_role_service_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'sidekiq/testing' + +describe Members::UpdateHighestRoleService, :clean_gitlab_redis_shared_state do + include ExclusiveLeaseHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:lease_key) { "update_highest_role:#{user.id}" } + let(:service) { described_class.new(user.id) } + + describe '#perform' do + subject { service.execute } + + context 'when lease is obtained' do + it 'takes the lease but does not release it', :aggregate_failures do + expect_to_obtain_exclusive_lease(lease_key, 'uuid', timeout: described_class::LEASE_TIMEOUT) + + subject + + expect(service.exclusive_lease.exists?).to be_truthy + end + + it 'schedules a job' do + Sidekiq::Testing.fake! do + expect { subject }.to change(UpdateHighestRoleWorker.jobs, :size).by(1) + end + end + end + + context 'when lease cannot be obtained' do + it 'only schedules one job' do + Sidekiq::Testing.fake! do + stub_exclusive_lease_taken(lease_key, timeout: described_class::LEASE_TIMEOUT) + + expect { subject }.not_to change(UpdateHighestRoleWorker.jobs, :size) + end + end + end + end +end diff --git a/spec/services/users/update_highest_member_role_service_spec.rb b/spec/services/users/update_highest_member_role_service_spec.rb new file mode 100644 index 00000000000..8063abffc2a --- /dev/null +++ b/spec/services/users/update_highest_member_role_service_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Users::UpdateHighestMemberRoleService do + let(:user) { create(:user) } + let(:execute_service) { described_class.new(user).execute } + + describe '#execute' do + context 'when user_highest_role already exists' do + let!(:user_highest_role) { create(:user_highest_role, :guest, user: user) } + + context 'when the current highest access level equals the already stored highest access level' do + it 'does not update the highest access level' do + create(:group_member, :guest, user: user) + + expect { execute_service }.not_to change { user_highest_role.reload.highest_access_level } + end + end + + context 'when the current highest access level does not equal the already stored highest access level' do + it 'updates the highest access level' do + create(:group_member, :developer, user: user) + + expect { execute_service } + .to change { user_highest_role.reload.highest_access_level } + .from(Gitlab::Access::GUEST) + .to(Gitlab::Access::DEVELOPER) + end + end + end + + context 'when user_highest_role does not exist' do + it 'creates an user_highest_role object to store the highest access level' do + create(:group_member, :guest, user: user) + + expect { execute_service }.to change { UserHighestRole.count }.from(0).to(1) + end + end + end +end diff --git a/spec/workers/cluster_update_app_worker_spec.rb b/spec/workers/cluster_update_app_worker_spec.rb index e540ede4bc0..d91104334e5 100644 --- a/spec/workers/cluster_update_app_worker_spec.rb +++ b/spec/workers/cluster_update_app_worker_spec.rb @@ -47,11 +47,11 @@ describe ClusterUpdateAppWorker do end context 'with exclusive lease' do + let_it_be(:user) { create(:user) } let(:application) { create(:clusters_applications_prometheus, :installed) } let(:lease_key) { "#{described_class.name.underscore}-#{application.id}" } before do - allow(Gitlab::ExclusiveLease).to receive(:new) stub_exclusive_lease_taken(lease_key) end @@ -61,8 +61,10 @@ describe ClusterUpdateAppWorker do subject.perform(application.name, application.id, project.id, Time.now) end - it 'does not allow same app to be updated concurrently by different project' do - project1 = create(:project) + it 'does not allow same app to be updated concurrently by different project', :aggregate_failures do + stub_exclusive_lease("refresh_authorized_projects:#{user.id}") + stub_exclusive_lease("update_highest_role:#{user.id}") + project1 = create(:project, namespace: create(:namespace, owner: user)) expect(Clusters::Applications::PrometheusUpdateService).not_to receive(:new) @@ -81,10 +83,13 @@ describe ClusterUpdateAppWorker do subject.perform(application2.name, application2.id, project.id, Time.now) end - it 'allows different app to be updated by different project' do + it 'allows different app to be updated by different project', :aggregate_failures do application2 = create(:clusters_applications_prometheus, :installed) lease_key2 = "#{described_class.name.underscore}-#{application2.id}" - project2 = create(:project) + + stub_exclusive_lease("refresh_authorized_projects:#{user.id}") + stub_exclusive_lease("update_highest_role:#{user.id}") + project2 = create(:project, namespace: create(:namespace, owner: user)) stub_exclusive_lease(lease_key2) diff --git a/spec/workers/update_highest_role_worker_spec.rb b/spec/workers/update_highest_role_worker_spec.rb new file mode 100644 index 00000000000..cb112ebe07e --- /dev/null +++ b/spec/workers/update_highest_role_worker_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe UpdateHighestRoleWorker, :clean_gitlab_redis_shared_state do + include ExclusiveLeaseHelpers + + let(:worker) { described_class.new } + + describe '#perform' do + let(:active_scope_attributes) do + { + state: 'active', + ghost: false, + user_type: nil + } + end + let(:user) { create(:user, attributes) } + + subject { worker.perform(user.id) } + + context 'when user is found' do + let(:attributes) { active_scope_attributes } + + it 'updates the highest role for the user' do + user_highest_role = create(:user_highest_role, user: user) + create(:group_member, :developer, user: user) + + expect { subject } + .to change { user_highest_role.reload.highest_access_level } + .from(nil) + .to(Gitlab::Access::DEVELOPER) + end + end + + context 'when user is not found' do + shared_examples 'no update' do + it 'does not update any highest role' do + expect(Users::UpdateHighestMemberRoleService).not_to receive(:new) + + worker.perform(user.id) + end + end + + context 'when user is blocked' do + let(:attributes) { active_scope_attributes.merge(state: 'blocked') } + + it_behaves_like 'no update' + end + + context 'when user is a ghost' do + let(:attributes) { active_scope_attributes.merge(ghost: true) } + + it_behaves_like 'no update' + end + + context 'when user has a user type' do + let(:attributes) { active_scope_attributes.merge(user_type: :alert_bot) } + + it_behaves_like 'no update' + end + end + end +end |