diff options
50 files changed, 1260 insertions, 400 deletions
diff --git a/app/assets/javascripts/admin/users/components/actions/activate.vue b/app/assets/javascripts/admin/users/components/actions/activate.vue new file mode 100644 index 00000000000..99c260bf11e --- /dev/null +++ b/app/assets/javascripts/admin/users/components/actions/activate.vue @@ -0,0 +1,44 @@ +<script> +import { GlDropdownItem } from '@gitlab/ui'; +import { sprintf, s__ } from '~/locale'; + +export default { + components: { + GlDropdownItem, + }, + props: { + username: { + type: String, + required: true, + }, + path: { + type: String, + required: true, + }, + }, + computed: { + modalAttributes() { + return { + 'data-path': this.path, + 'data-method': 'put', + 'data-modal-attributes': JSON.stringify({ + title: sprintf(s__('AdminUsers|Activate user %{username}?'), { + username: this.username, + }), + message: s__('AdminUsers|You can always deactivate their account again if needed.'), + okVariant: 'confirm', + okTitle: s__('AdminUsers|Activate'), + }), + }; + }, + }, +}; +</script> + +<template> + <div class="js-confirm-modal-button" v-bind="{ ...modalAttributes }"> + <gl-dropdown-item> + <slot></slot> + </gl-dropdown-item> + </div> +</template> diff --git a/app/assets/javascripts/admin/users/components/actions/approve.vue b/app/assets/javascripts/admin/users/components/actions/approve.vue new file mode 100644 index 00000000000..6fc43c246ea --- /dev/null +++ b/app/assets/javascripts/admin/users/components/actions/approve.vue @@ -0,0 +1,21 @@ +<script> +import { GlDropdownItem } from '@gitlab/ui'; + +export default { + components: { + GlDropdownItem, + }, + props: { + path: { + type: String, + required: true, + }, + }, +}; +</script> + +<template> + <gl-dropdown-item :href="path" data-method="put"> + <slot></slot> + </gl-dropdown-item> +</template> diff --git a/app/assets/javascripts/admin/users/components/actions/block.vue b/app/assets/javascripts/admin/users/components/actions/block.vue new file mode 100644 index 00000000000..68dfefe14c2 --- /dev/null +++ b/app/assets/javascripts/admin/users/components/actions/block.vue @@ -0,0 +1,53 @@ +<script> +import { GlDropdownItem } from '@gitlab/ui'; +import { sprintf, s__ } from '~/locale'; + +// TODO: To be replaced with <template> content in https://gitlab.com/gitlab-org/gitlab/-/issues/320922 +const messageHtml = ` + <p>${s__('AdminUsers|Blocking user has the following effects:')}</p> + <ul> + <li>${s__('AdminUsers|User will not be able to login')}</li> + <li>${s__('AdminUsers|User will not be able to access git repositories')}</li> + <li>${s__('AdminUsers|Personal projects will be left')}</li> + <li>${s__('AdminUsers|Owned groups will be left')}</li> + </ul> +`; + +export default { + components: { + GlDropdownItem, + }, + props: { + username: { + type: String, + required: true, + }, + path: { + type: String, + required: true, + }, + }, + computed: { + modalAttributes() { + return { + 'data-path': this.path, + 'data-method': 'put', + 'data-modal-attributes': JSON.stringify({ + title: sprintf(s__('AdminUsers|Block user %{username}?'), { username: this.username }), + okVariant: 'confirm', + okTitle: s__('AdminUsers|Block'), + messageHtml, + }), + }; + }, + }, +}; +</script> + +<template> + <div class="js-confirm-modal-button" v-bind="{ ...modalAttributes }"> + <gl-dropdown-item> + <slot></slot> + </gl-dropdown-item> + </div> +</template> diff --git a/app/assets/javascripts/admin/users/components/actions/deactivate.vue b/app/assets/javascripts/admin/users/components/actions/deactivate.vue new file mode 100644 index 00000000000..7e0c17ba296 --- /dev/null +++ b/app/assets/javascripts/admin/users/components/actions/deactivate.vue @@ -0,0 +1,60 @@ +<script> +import { GlDropdownItem } from '@gitlab/ui'; +import { sprintf, s__ } from '~/locale'; + +// TODO: To be replaced with <template> content in https://gitlab.com/gitlab-org/gitlab/-/issues/320922 +const messageHtml = ` + <p>${s__('AdminUsers|Deactivating a user has the following effects:')}</p> + <ul> + <li>${s__('AdminUsers|The user will be logged out')}</li> + <li>${s__('AdminUsers|The user will not be able to access git repositories')}</li> + <li>${s__('AdminUsers|The user will not be able to access the API')}</li> + <li>${s__('AdminUsers|The user will not receive any notifications')}</li> + <li>${s__('AdminUsers|The user will not be able to use slash commands')}</li> + <li>${s__( + 'AdminUsers|When the user logs back in, their account will reactivate as a fully active account', + )}</li> + <li>${s__('AdminUsers|Personal projects, group and user history will be left intact')}</li> + </ul> +`; + +export default { + components: { + GlDropdownItem, + }, + props: { + username: { + type: String, + required: true, + }, + path: { + type: String, + required: true, + }, + }, + computed: { + modalAttributes() { + return { + 'data-path': this.path, + 'data-method': 'put', + 'data-modal-attributes': JSON.stringify({ + title: sprintf(s__('AdminUsers|Deactivate user %{username}?'), { + username: this.username, + }), + okVariant: 'confirm', + okTitle: s__('AdminUsers|Deactivate'), + messageHtml, + }), + }; + }, + }, +}; +</script> + +<template> + <div class="js-confirm-modal-button" v-bind="{ ...modalAttributes }"> + <gl-dropdown-item> + <slot></slot> + </gl-dropdown-item> + </div> +</template> diff --git a/app/assets/javascripts/admin/users/components/actions/delete.vue b/app/assets/javascripts/admin/users/components/actions/delete.vue new file mode 100644 index 00000000000..725d3dbf388 --- /dev/null +++ b/app/assets/javascripts/admin/users/components/actions/delete.vue @@ -0,0 +1,25 @@ +<script> +import SharedDeleteAction from './shared/shared_delete_action.vue'; + +export default { + components: { + SharedDeleteAction, + }, + props: { + username: { + type: String, + required: true, + }, + paths: { + type: Object, + required: true, + }, + }, +}; +</script> + +<template> + <shared-delete-action modal-type="delete" :username="username" :paths="paths"> + <slot></slot> + </shared-delete-action> +</template> diff --git a/app/assets/javascripts/admin/users/components/actions/delete_with_contributions.vue b/app/assets/javascripts/admin/users/components/actions/delete_with_contributions.vue new file mode 100644 index 00000000000..0ae15bfbebb --- /dev/null +++ b/app/assets/javascripts/admin/users/components/actions/delete_with_contributions.vue @@ -0,0 +1,25 @@ +<script> +import SharedDeleteAction from './shared/shared_delete_action.vue'; + +export default { + components: { + SharedDeleteAction, + }, + props: { + username: { + type: String, + required: true, + }, + paths: { + type: Object, + required: true, + }, + }, +}; +</script> + +<template> + <shared-delete-action modal-type="delete-with-contributions" :username="username" :paths="paths"> + <slot></slot> + </shared-delete-action> +</template> diff --git a/app/assets/javascripts/admin/users/components/actions/index.js b/app/assets/javascripts/admin/users/components/actions/index.js new file mode 100644 index 00000000000..697bf284453 --- /dev/null +++ b/app/assets/javascripts/admin/users/components/actions/index.js @@ -0,0 +1,21 @@ +import Activate from './activate.vue'; +import Approve from './approve.vue'; +import Block from './block.vue'; +import Deactivate from './deactivate.vue'; +import Delete from './delete.vue'; +import DeleteWithContributions from './delete_with_contributions.vue'; +import Unblock from './unblock.vue'; +import Unlock from './unlock.vue'; +import Reject from './reject.vue'; + +export default { + Activate, + Approve, + Block, + Deactivate, + Delete, + DeleteWithContributions, + Unblock, + Unlock, + Reject, +}; diff --git a/app/assets/javascripts/admin/users/components/actions/reject.vue b/app/assets/javascripts/admin/users/components/actions/reject.vue new file mode 100644 index 00000000000..a80c1ff5458 --- /dev/null +++ b/app/assets/javascripts/admin/users/components/actions/reject.vue @@ -0,0 +1,21 @@ +<script> +import { GlDropdownItem } from '@gitlab/ui'; + +export default { + components: { + GlDropdownItem, + }, + props: { + path: { + type: String, + required: true, + }, + }, +}; +</script> + +<template> + <gl-dropdown-item :href="path" data-method="delete"> + <slot></slot> + </gl-dropdown-item> +</template> diff --git a/app/assets/javascripts/admin/users/components/actions/shared/shared_delete_action.vue b/app/assets/javascripts/admin/users/components/actions/shared/shared_delete_action.vue new file mode 100644 index 00000000000..9107d9ccdd9 --- /dev/null +++ b/app/assets/javascripts/admin/users/components/actions/shared/shared_delete_action.vue @@ -0,0 +1,43 @@ +<script> +import { GlDropdownItem } from '@gitlab/ui'; + +export default { + components: { + GlDropdownItem, + }, + props: { + username: { + type: String, + required: true, + }, + paths: { + type: Object, + required: true, + }, + modalType: { + type: String, + required: true, + }, + }, + computed: { + modalAttributes() { + return { + 'data-block-user-url': this.paths.block, + 'data-delete-user-url': this.paths.delete, + 'data-gl-modal-action': this.modalType, + 'data-username': this.username, + }; + }, + }, +}; +</script> + +<template> + <div class="js-delete-user-modal-button" v-bind="{ ...modalAttributes }"> + <gl-dropdown-item> + <span class="gl-text-red-500"> + <slot></slot> + </span> + </gl-dropdown-item> + </div> +</template> diff --git a/app/assets/javascripts/admin/users/components/actions/unblock.vue b/app/assets/javascripts/admin/users/components/actions/unblock.vue new file mode 100644 index 00000000000..f2b501caf09 --- /dev/null +++ b/app/assets/javascripts/admin/users/components/actions/unblock.vue @@ -0,0 +1,44 @@ +<script> +import { GlDropdownItem } from '@gitlab/ui'; +import { sprintf, s__ } from '~/locale'; + +export default { + components: { + GlDropdownItem, + }, + props: { + username: { + type: String, + required: true, + }, + path: { + type: String, + required: true, + }, + }, + computed: { + modalAttributes() { + return { + 'data-path': this.path, + 'data-method': 'put', + 'data-modal-attributes': JSON.stringify({ + title: sprintf(s__('AdminUsers|Unblock user %{username}?'), { username: this.username }), + message: s__( + 'AdminUsers|You can always unblock their account, their data will remain intact.', + ), + okVariant: 'confirm', + okTitle: s__('AdminUsers|Unblock'), + }), + }; + }, + }, +}; +</script> + +<template> + <div class="js-confirm-modal-button" v-bind="{ ...modalAttributes }"> + <gl-dropdown-item> + <slot></slot> + </gl-dropdown-item> + </div> +</template> diff --git a/app/assets/javascripts/admin/users/components/actions/unlock.vue b/app/assets/javascripts/admin/users/components/actions/unlock.vue new file mode 100644 index 00000000000..294aaade7c1 --- /dev/null +++ b/app/assets/javascripts/admin/users/components/actions/unlock.vue @@ -0,0 +1,42 @@ +<script> +import { GlDropdownItem } from '@gitlab/ui'; +import { sprintf, s__, __ } from '~/locale'; + +export default { + components: { + GlDropdownItem, + }, + props: { + username: { + type: String, + required: true, + }, + path: { + type: String, + required: true, + }, + }, + computed: { + modalAttributes() { + return { + 'data-path': this.path, + 'data-method': 'put', + 'data-modal-attributes': JSON.stringify({ + title: sprintf(s__('AdminUsers|Unlock user %{username}?'), { username: this.username }), + message: __('Are you sure?'), + okVariant: 'confirm', + okTitle: s__('AdminUsers|Unlock'), + }), + }; + }, + }, +}; +</script> + +<template> + <div class="js-confirm-modal-button" v-bind="{ ...modalAttributes }"> + <gl-dropdown-item> + <slot></slot> + </gl-dropdown-item> + </div> +</template> diff --git a/app/assets/javascripts/admin/users/components/user_actions.vue b/app/assets/javascripts/admin/users/components/user_actions.vue index 6c7c434cdf4..f6d2f6b7497 100644 --- a/app/assets/javascripts/admin/users/components/user_actions.vue +++ b/app/assets/javascripts/admin/users/components/user_actions.vue @@ -6,9 +6,11 @@ import { GlDropdownSectionHeader, GlDropdownDivider, } from '@gitlab/ui'; -import { s__, __ } from '~/locale'; +import { capitalizeFirstCharacter } from '~/lib/utils/text_utility'; import { convertArrayToCamelCase } from '~/lib/utils/common_utils'; import { generateUserPaths } from '../utils'; +import { I18N_USER_ACTIONS } from '../constants'; +import Actions from './actions'; export default { components: { @@ -17,6 +19,7 @@ export default { GlDropdownItem, GlDropdownSectionHeader, GlDropdownDivider, + ...Actions, }, props: { user: { @@ -58,21 +61,11 @@ export default { isLdapAction(action) { return action === 'ldapBlocked'; }, + getActionComponent(action) { + return Actions[capitalizeFirstCharacter(action)]; + }, }, - i18n: { - edit: __('Edit'), - settings: __('Settings'), - unlock: __('Unlock'), - block: s__('AdminUsers|Block'), - unblock: s__('AdminUsers|Unblock'), - approve: s__('AdminUsers|Approve'), - reject: s__('AdminUsers|Reject'), - deactivate: s__('AdminUsers|Deactivate'), - activate: s__('AdminUsers|Activate'), - ldapBlocked: s__('AdminUsers|Cannot unblock LDAP blocked users'), - delete: s__('AdminUsers|Delete user'), - deleteWithContributions: s__('AdminUsers|Delete user and contributions'), - }, + i18n: I18N_USER_ACTIONS, }; </script> @@ -92,24 +85,35 @@ export default { <gl-dropdown-section-header>{{ $options.i18n.settings }}</gl-dropdown-section-header> <template v-for="action in dropdownSafeActions"> - <gl-dropdown-item v-if="isLdapAction(action)" :key="action" :data-testid="action"> - {{ $options.i18n.ldap }} - </gl-dropdown-item> - <gl-dropdown-item v-else :key="action" :href="userPaths[action]" :data-testid="action"> + <component + :is="getActionComponent(action)" + v-if="getActionComponent(action)" + :key="action" + :path="userPaths[action]" + :username="user.name" + :data-testid="action" + > + {{ $options.i18n[action] }} + </component> + <gl-dropdown-item v-else-if="isLdapAction(action)" :key="action" :data-testid="action"> {{ $options.i18n[action] }} </gl-dropdown-item> </template> <gl-dropdown-divider v-if="hasDeleteActions" /> - <gl-dropdown-item - v-for="action in dropdownDeleteActions" - :key="action" - :href="userPaths[action]" - :data-testid="`delete-${action}`" - > - <span class="gl-text-red-500">{{ $options.i18n[action] }}</span> - </gl-dropdown-item> + <template v-for="action in dropdownDeleteActions"> + <component + :is="getActionComponent(action)" + v-if="getActionComponent(action)" + :key="action" + :paths="userPaths" + :username="user.name" + :data-testid="`delete-${action}`" + > + {{ $options.i18n[action] }} + </component> + </template> </gl-dropdown> </div> </template> diff --git a/app/assets/javascripts/admin/users/constants.js b/app/assets/javascripts/admin/users/constants.js index e26643cad60..8ea1bd3ca7a 100644 --- a/app/assets/javascripts/admin/users/constants.js +++ b/app/assets/javascripts/admin/users/constants.js @@ -1,5 +1,22 @@ +import { s__, __ } from '~/locale'; + export const USER_AVATAR_SIZE = 32; export const SHORT_DATE_FORMAT = 'd mmm, yyyy'; export const LENGTH_OF_USER_NOTE_TOOLTIP = 100; + +export const I18N_USER_ACTIONS = { + edit: __('Edit'), + settings: __('Settings'), + unlock: __('Unlock'), + block: s__('AdminUsers|Block'), + unblock: s__('AdminUsers|Unblock'), + approve: s__('AdminUsers|Approve'), + reject: s__('AdminUsers|Reject'), + deactivate: s__('AdminUsers|Deactivate'), + activate: s__('AdminUsers|Activate'), + ldapBlocked: s__('AdminUsers|Cannot unblock LDAP blocked users'), + delete: s__('AdminUsers|Delete user'), + deleteWithContributions: s__('AdminUsers|Delete user and contributions'), +}; diff --git a/app/assets/javascripts/issuable_show/components/issuable_header.vue b/app/assets/javascripts/issuable_show/components/issuable_header.vue index 4c6df31a0f3..9cfbc2d9bbc 100644 --- a/app/assets/javascripts/issuable_show/components/issuable_header.vue +++ b/app/assets/javascripts/issuable_show/components/issuable_header.vue @@ -3,6 +3,7 @@ import { GlIcon, GlButton, GlTooltipDirective, GlAvatarLink, GlAvatarLabeled } f import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; +import { isExternal } from '~/lib/utils/url_utility'; export default { components: { @@ -49,6 +50,9 @@ export default { authorId() { return getIdFromGraphQLId(`${this.author.id}`); }, + isAuthorExternal() { + return isExternal(this.author.webUrl); + }, }, mounted() { this.toggleSidebarButtonEl = document.querySelector('.js-toggle-right-sidebar-button'); @@ -98,7 +102,11 @@ export default { :src="author.avatarUrl" :label="author.name" class="d-none d-sm-inline-flex gl-ml-1" - /> + > + <template #meta> + <gl-icon v-if="isAuthorExternal" name="external-link" /> + </template> + </gl-avatar-labeled> <strong class="author d-sm-none d-inline">@{{ author.username }}</strong> </gl-avatar-link> </div> diff --git a/app/assets/javascripts/jobs/components/job_container_item.vue b/app/assets/javascripts/jobs/components/job_container_item.vue index 6b61dc5902b..6a803c06df2 100644 --- a/app/assets/javascripts/jobs/components/job_container_item.vue +++ b/app/assets/javascripts/jobs/components/job_container_item.vue @@ -51,7 +51,7 @@ export default { v-gl-tooltip :href="job.status.details_path" :title="tooltipText" - class="js-job-link d-flex" + class="js-job-link gl-display-flex gl-align-items-center" > <gl-icon v-if="isActive" diff --git a/app/assets/javascripts/lib/utils/url_utility.js b/app/assets/javascripts/lib/utils/url_utility.js index 44d3e78b334..0b920ba8e7a 100644 --- a/app/assets/javascripts/lib/utils/url_utility.js +++ b/app/assets/javascripts/lib/utils/url_utility.js @@ -310,6 +310,20 @@ export function isAbsoluteOrRootRelative(url) { } /** + * Returns true if url is an external URL + * + * @param {String} url + * @returns {Boolean} + */ +export function isExternal(url) { + if (isRootRelative(url)) { + return false; + } + + return !url.includes(gon.gitlab_url); +} + +/** * Converts a relative path to an absolute or a root relative path depending * on what is passed as a basePath. * diff --git a/app/assets/javascripts/pages/admin/users/index.js b/app/assets/javascripts/pages/admin/users/index.js index ae5db5f5bdc..42afcf434f6 100644 --- a/app/assets/javascripts/pages/admin/users/index.js +++ b/app/assets/javascripts/pages/admin/users/index.js @@ -34,6 +34,8 @@ function loadModalsConfigurationFromHtml(modalsElement) { document.addEventListener('DOMContentLoaded', () => { Vue.use(Translate); + initAdminUsersApp(); + const modalConfiguration = loadModalsConfigurationFromHtml( document.querySelector(MODAL_TEXTS_CONTAINER_SELECTOR), ); @@ -60,7 +62,6 @@ document.addEventListener('DOMContentLoaded', () => { }); initConfirmModal(); - initAdminUsersApp(); initCohortsEmptyState(); initTabs(); }); diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index 9654b7cd9a9..1a5b578cc75 100644 --- a/app/helpers/search_helper.rb +++ b/app/helpers/search_helper.rb @@ -122,15 +122,24 @@ module SearchHelper end def search_sort_options - options = [] - options << { - title: _('Created date'), - sortable: true, - sortParam: { - asc: 'created_asc', - desc: 'created_desc' + [ + { + title: _('Created date'), + sortable: true, + sortParam: { + asc: 'created_asc', + desc: 'created_desc' + } + }, + { + title: _('Last updated'), + sortable: true, + sortParam: { + asc: 'updated_asc', + desc: 'updated_desc' + } } - } + ] end private diff --git a/app/models/concerns/repository_storage_movable.rb b/app/models/concerns/repository_storage_movable.rb index e584922025a..8607f0d94f4 100644 --- a/app/models/concerns/repository_storage_movable.rb +++ b/app/models/concerns/repository_storage_movable.rb @@ -68,6 +68,18 @@ module RepositoryStorageMovable storage_move.update_repository_storage(storage_move.destination_storage_name) end + after_transition started: :replicated do |storage_move| + # We have several scripts in place that replicate some statistics information + # to other databases. Some of them depend on the updated_at column + # to identify the models they need to extract. + # + # If we don't update the `updated_at` of the container after a repository storage move, + # the scripts won't know that they need to sync them. + # + # See https://gitlab.com/gitlab-data/analytics/-/issues/7868 + storage_move.container.touch + end + before_transition started: :failed do |storage_move| storage_move.container.set_repository_writable! end diff --git a/app/models/pages_deployment.rb b/app/models/pages_deployment.rb index 95949eeed9a..d67a92af6af 100644 --- a/app/models/pages_deployment.rb +++ b/app/models/pages_deployment.rb @@ -2,6 +2,7 @@ # PagesDeployment stores a zip archive containing GitLab Pages web-site class PagesDeployment < ApplicationRecord + include EachBatch include FileStoreMounter MIGRATED_FILE_NAME = "_migrated.zip" diff --git a/app/views/projects/no_repo.html.haml b/app/views/projects/no_repo.html.haml index ea14e2d6ca5..3c7afff57f6 100644 --- a/app/views/projects/no_repo.html.haml +++ b/app/views/projects/no_repo.html.haml @@ -15,14 +15,14 @@ = render 'projects/invite_members_modal', project: @project .no-repo-actions - = link_to project_repository_path(@project), method: :post, class: 'btn btn-primary' do + = link_to project_repository_path(@project), method: :post, class: 'btn gl-button btn-confirm' do #{ _('Create empty repository') } %strong.gl-ml-3.gl-mr-3 or - = link_to new_project_import_path(@project), class: 'btn' do + = link_to new_project_import_path(@project), class: 'btn gl-button btn-default' do #{ _('Import repository') } - if can? current_user, :remove_project, @project .prepend-top-20 - = link_to _('Delete project'), project_path(@project), data: { confirm: remove_project_message(@project)}, method: :delete, class: "gl-button btn btn-danger btn-danger-secondary float-right" + = link_to _('Delete project'), project_path(@project), data: { confirm: remove_project_message(@project)}, method: :delete, class: "btn gl-button btn-danger float-right" diff --git a/changelogs/unreleased/295311-sort-mr-issues-last-updated.yml b/changelogs/unreleased/295311-sort-mr-issues-last-updated.yml new file mode 100644 index 00000000000..16317279ee0 --- /dev/null +++ b/changelogs/unreleased/295311-sort-mr-issues-last-updated.yml @@ -0,0 +1,5 @@ +--- +title: 'Search: Add Sort by Last Updated to Issue/MR' +merge_request: 53589 +author: +type: changed diff --git a/changelogs/unreleased/fj-update-update-at-column-after-repository-storage-move.yml b/changelogs/unreleased/fj-update-update-at-column-after-repository-storage-move.yml new file mode 100644 index 00000000000..d43f621f6d6 --- /dev/null +++ b/changelogs/unreleased/fj-update-update-at-column-after-repository-storage-move.yml @@ -0,0 +1,5 @@ +--- +title: Update column 'updated_at' in container after repository storage move +merge_request: 53821 +author: +type: fixed diff --git a/changelogs/unreleased/gl-button-no-repo-actions.yml b/changelogs/unreleased/gl-button-no-repo-actions.yml new file mode 100644 index 00000000000..256cafd546f --- /dev/null +++ b/changelogs/unreleased/gl-button-no-repo-actions.yml @@ -0,0 +1,5 @@ +--- +title: Apply new GitLab UI for no repo action buttons +merge_request: 53580 +author: Yogi (@yo) +type: other diff --git a/config/feature_flags/development/usage_data_i_code_review_user_approval_rule_added.yml b/config/feature_flags/development/usage_data_i_code_review_user_approval_rule_added.yml new file mode 100644 index 00000000000..749b17f9e0c --- /dev/null +++ b/config/feature_flags/development/usage_data_i_code_review_user_approval_rule_added.yml @@ -0,0 +1,8 @@ +--- +name: usage_data_i_code_review_user_approval_rule_added +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/issues/320966 +rollout_issue_url: +milestone: '13.9' +type: development +group: group::code review +default_enabled: true diff --git a/config/feature_flags/development/usage_data_i_code_review_user_approval_rule_deleted.yml b/config/feature_flags/development/usage_data_i_code_review_user_approval_rule_deleted.yml new file mode 100644 index 00000000000..ef97df30aeb --- /dev/null +++ b/config/feature_flags/development/usage_data_i_code_review_user_approval_rule_deleted.yml @@ -0,0 +1,8 @@ +--- +name: usage_data_i_code_review_user_approval_rule_deleted +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/issues/320966 +rollout_issue_url: +milestone: '13.9' +type: development +group: group::code review +default_enabled: true diff --git a/config/feature_flags/development/usage_data_i_code_review_user_approval_rule_edited.yml b/config/feature_flags/development/usage_data_i_code_review_user_approval_rule_edited.yml new file mode 100644 index 00000000000..5ecdaf3a2d0 --- /dev/null +++ b/config/feature_flags/development/usage_data_i_code_review_user_approval_rule_edited.yml @@ -0,0 +1,8 @@ +--- +name: usage_data_i_code_review_user_approval_rule_edited +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/issues/320966 +rollout_issue_url: +milestone: '13.9' +type: development +group: group::code review +default_enabled: true diff --git a/lib/gitlab/search/sort_options.rb b/lib/gitlab/search/sort_options.rb index 3395c34d171..2ab38147462 100644 --- a/lib/gitlab/search/sort_options.rb +++ b/lib/gitlab/search/sort_options.rb @@ -11,6 +11,10 @@ module Gitlab :created_at_asc when %w[created_at desc], [nil, 'created_desc'] :created_at_desc + when %w[updated_at asc], [nil, 'updated_asc'] + :updated_at_asc + when %w[updated_at desc], [nil, 'updated_desc'] + :updated_at_desc else :unknown end diff --git a/lib/gitlab/search_results.rb b/lib/gitlab/search_results.rb index 0091ae1e8ce..d0beb74c289 100644 --- a/lib/gitlab/search_results.rb +++ b/lib/gitlab/search_results.rb @@ -136,6 +136,10 @@ module Gitlab scope.reorder('created_at ASC') when :created_at_desc scope.reorder('created_at DESC') + when :updated_at_asc + scope.reorder('updated_at ASC') + when :updated_at_desc + scope.reorder('updated_at DESC') else scope.reorder('created_at DESC') end diff --git a/lib/gitlab/usage_data_counters/known_events/code_review_events.yml b/lib/gitlab/usage_data_counters/known_events/code_review_events.yml index b6aa4b5ec3a..df9a94fe9be 100644 --- a/lib/gitlab/usage_data_counters/known_events/code_review_events.yml +++ b/lib/gitlab/usage_data_counters/known_events/code_review_events.yml @@ -124,3 +124,18 @@ category: code_review aggregation: weekly feature_flag: usage_data_i_code_review_user_review_requested +- name: i_code_review_user_approval_rule_added + redis_slot: code_review + category: code_review + aggregation: weekly + feature_flag: usage_data_i_code_review_user_approval_rule_added +- name: i_code_review_user_approval_rule_deleted + redis_slot: code_review + category: code_review + aggregation: weekly + feature_flag: usage_data_i_code_review_user_approval_rule_deleted +- name: i_code_review_user_approval_rule_edited + redis_slot: code_review + category: code_review + aggregation: weekly + feature_flag: usage_data_i_code_review_user_approval_rule_edited diff --git a/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter.rb b/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter.rb index b5227f79261..c0b3e6b3861 100644 --- a/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter.rb +++ b/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter.rb @@ -26,6 +26,9 @@ module Gitlab MR_UNRESOLVE_THREAD_ACTION = 'i_code_review_user_unresolve_thread' MR_ASSIGNED_USERS_ACTION = 'i_code_review_user_assigned' MR_REVIEW_REQUESTED_USERS_ACTION = 'i_code_review_user_review_requested' + MR_APPROVAL_RULE_ADDED_USERS_ACTION = 'i_code_review_user_approval_rule_added' + MR_APPROVAL_RULE_EDITED_USERS_ACTION = 'i_code_review_user_approval_rule_edited' + MR_APPROVAL_RULE_DELETED_USERS_ACTION = 'i_code_review_user_approval_rule_deleted' MR_EDIT_MR_TITLE_ACTION = 'i_code_review_edit_mr_title' MR_EDIT_MR_DESC_ACTION = 'i_code_review_edit_mr_desc' @@ -118,6 +121,18 @@ module Gitlab track_unique_action_by_user(MR_EDIT_MR_DESC_ACTION, user) end + def track_approval_rule_added_action(user:) + track_unique_action_by_user(MR_APPROVAL_RULE_ADDED_USERS_ACTION, user) + end + + def track_approval_rule_edited_action(user:) + track_unique_action_by_user(MR_APPROVAL_RULE_EDITED_USERS_ACTION, user) + end + + def track_approval_rule_deleted_action(user:) + track_unique_action_by_user(MR_APPROVAL_RULE_DELETED_USERS_ACTION, user) + end + private def track_unique_action_by_merge_request(action, merge_request) diff --git a/lib/peek/views/external_http.rb b/lib/peek/views/external_http.rb index bd0e4c64127..b925e3db7b6 100644 --- a/lib/peek/views/external_http.rb +++ b/lib/peek/views/external_http.rb @@ -86,12 +86,16 @@ module Peek uri.hostname = call[:host] uri.port = call[:port] uri.path = call[:path] - uri.query = call[:query] + uri.query = generate_query(call[:query]) uri.to_s - rescue URI::Error + rescue StandardError 'unknown' end + + def generate_query(query_string) + query_string.is_a?(Hash) ? query_string.to_query : query_string.to_s + end end end end diff --git a/lib/tasks/gitlab/pages.rake b/lib/tasks/gitlab/pages.rake index 80550317dba..d9b7864e1c5 100644 --- a/lib/tasks/gitlab/pages.rake +++ b/lib/tasks/gitlab/pages.rake @@ -6,7 +6,6 @@ namespace :gitlab do namespace :pages do desc "GitLab | Pages | Migrate legacy storage to zip format" task migrate_legacy_storage: :gitlab_environment do - logger = Logger.new(STDOUT) logger.info('Starting to migrate legacy pages storage to zip deployments') result = ::Pages::MigrateFromLegacyStorageService.new(logger, migration_threads, batch_size).execute @@ -16,6 +15,26 @@ namespace :gitlab do logger.info("- The #{result[:errored]} projects failed to be migrated") end + desc "GitLab | Pages | DANGER: Removes data which was migrated from legacy storage on zip storage. Can be used if some bugs in migration are discovered and migration needs to be restarted from scratch." + task clean_migrated_zip_storage: :gitlab_environment do + destroyed_deployments = 0 + + logger.info("Starting to delete migrated pages deployments") + + ::PagesDeployment.migrated_from_legacy_storage.each_batch(of: batch_size) do |batch| + destroyed_deployments += batch.count + + # we need to destroy associated files, so can't use delete_all + batch.destroy_all # rubocop: disable Cop/DestroyAll + + logger.info("#{destroyed_deployments} deployments were deleted") + end + end + + def logger + @logger ||= Logger.new(STDOUT) + end + def migration_threads ENV.fetch('PAGES_MIGRATION_THREADS', '3').to_i end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 5e94b9865c5..36be2b217c1 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2376,6 +2376,12 @@ msgstr "" msgid "AdminUsers|Unblock user %{username}?" msgstr "" +msgid "AdminUsers|Unlock" +msgstr "" + +msgid "AdminUsers|Unlock user %{username}?" +msgstr "" + msgid "AdminUsers|User will not be able to access git repositories" msgstr "" @@ -34149,6 +34155,9 @@ msgstr "" msgid "ciReport|Coverage fuzzing" msgstr "" +msgid "ciReport|Create Jira issue" +msgstr "" + msgid "ciReport|Create a merge request to implement this solution, or download and apply the patch manually." msgstr "" diff --git a/spec/factories/pages_deployments.rb b/spec/factories/pages_deployments.rb index 56aab4fa9f3..d3e2fefb4ae 100644 --- a/spec/factories/pages_deployments.rb +++ b/spec/factories/pages_deployments.rb @@ -4,12 +4,20 @@ FactoryBot.define do factory :pages_deployment, class: 'PagesDeployment' do project - after(:build) do |deployment, _evaluator| - filepath = Rails.root.join("spec/fixtures/pages.zip") + transient do + filename { nil } + end + + trait(:migrated) do + filename { PagesDeployment::MIGRATED_FILE_NAME } + end + + after(:build) do |deployment, evaluator| + file = UploadedFile.new("spec/fixtures/pages.zip", filename: evaluator.filename) - deployment.file = fixture_file_upload(filepath) - deployment.file_sha256 = Digest::SHA256.file(filepath).hexdigest - ::Zip::File.open(filepath) do |zip_archive| + deployment.file = file + deployment.file_sha256 = Digest::SHA256.file(file.path).hexdigest + ::Zip::File.open(file.path) do |zip_archive| deployment.file_count = zip_archive.count end end diff --git a/spec/frontend/admin/users/components/actions/actions_spec.js b/spec/frontend/admin/users/components/actions/actions_spec.js new file mode 100644 index 00000000000..22cd908449e --- /dev/null +++ b/spec/frontend/admin/users/components/actions/actions_spec.js @@ -0,0 +1,98 @@ +import { nextTick } from 'vue'; +import { shallowMount } from '@vue/test-utils'; +import { GlDropdownItem } from '@gitlab/ui'; +import { kebabCase } from 'lodash'; +import { capitalizeFirstCharacter } from '~/lib/utils/text_utility'; +import Actions from '~/admin/users/components/actions'; +import SharedDeleteAction from '~/admin/users/components/actions/shared/shared_delete_action.vue'; + +import { CONFIRMATION_ACTIONS, DELETE_ACTIONS } from '../../constants'; + +describe('Action components', () => { + let wrapper; + + const findDropdownItem = () => wrapper.find(GlDropdownItem); + + const initComponent = ({ component, props, stubs = {} } = {}) => { + wrapper = shallowMount(component, { + propsData: { + ...props, + }, + stubs, + }); + }; + + afterEach(() => { + wrapper.destroy(); + wrapper = null; + }); + + describe('CONFIRMATION_ACTIONS', () => { + it.each(CONFIRMATION_ACTIONS)('renders a dropdown item for "%s"', async (action) => { + initComponent({ + component: Actions[capitalizeFirstCharacter(action)], + props: { + username: 'John Doe', + path: '/test', + }, + }); + + await nextTick(); + + const div = wrapper.find('div'); + expect(div.attributes('data-path')).toBe('/test'); + expect(div.attributes('data-modal-attributes')).toContain('John Doe'); + expect(findDropdownItem().exists()).toBe(true); + }); + }); + + describe('LINK_ACTIONS', () => { + it.each` + action | method + ${'Approve'} | ${'put'} + ${'Reject'} | ${'delete'} + `( + 'renders a dropdown item link with method "$method" for "$action"', + async ({ action, method }) => { + initComponent({ + component: Actions[action], + props: { + path: '/test', + }, + }); + + await nextTick(); + + const item = wrapper.find(GlDropdownItem); + expect(item.attributes('href')).toBe('/test'); + expect(item.attributes('data-method')).toContain(method); + }, + ); + }); + + describe('DELETE_ACTION_COMPONENTS', () => { + it.each(DELETE_ACTIONS)('renders a dropdown item for "%s"', async (action) => { + initComponent({ + component: Actions[capitalizeFirstCharacter(action)], + props: { + username: 'John Doe', + paths: { + delete: '/delete', + block: '/block', + }, + }, + stubs: { SharedDeleteAction }, + }); + + await nextTick(); + + const sharedAction = wrapper.find(SharedDeleteAction); + + expect(sharedAction.attributes('data-block-user-url')).toBe('/block'); + expect(sharedAction.attributes('data-delete-user-url')).toBe('/delete'); + expect(sharedAction.attributes('data-gl-modal-action')).toBe(kebabCase(action)); + expect(sharedAction.attributes('data-username')).toBe('John Doe'); + expect(findDropdownItem().exists()).toBe(true); + }); + }); +}); diff --git a/spec/frontend/admin/users/components/user_actions_spec.js b/spec/frontend/admin/users/components/user_actions_spec.js index 78bc37233c2..ec625c228be 100644 --- a/spec/frontend/admin/users/components/user_actions_spec.js +++ b/spec/frontend/admin/users/components/user_actions_spec.js @@ -2,14 +2,12 @@ import { shallowMount } from '@vue/test-utils'; import { GlDropdownDivider } from '@gitlab/ui'; import AdminUserActions from '~/admin/users/components/user_actions.vue'; import { generateUserPaths } from '~/admin/users/utils'; +import { I18N_USER_ACTIONS } from '~/admin/users/constants'; +import Actions from '~/admin/users/components/actions'; +import { capitalizeFirstCharacter } from '~/lib/utils/text_utility'; import { users, paths } from '../mock_data'; - -const BLOCK = 'block'; -const EDIT = 'edit'; -const LDAP = 'ldapBlocked'; -const DELETE = 'delete'; -const DELETE_WITH_CONTRIBUTIONS = 'deleteWithContributions'; +import { CONFIRMATION_ACTIONS, DELETE_ACTIONS, LINK_ACTIONS, LDAP, EDIT } from '../constants'; describe('AdminUserActions component', () => { let wrapper; @@ -62,7 +60,7 @@ describe('AdminUserActions component', () => { describe('actions dropdown', () => { describe('when there are actions', () => { - const actions = [EDIT, BLOCK]; + const actions = [EDIT, ...LINK_ACTIONS]; beforeEach(() => { initComponent({ actions }); @@ -72,10 +70,31 @@ describe('AdminUserActions component', () => { expect(findActionsDropdown().exists()).toBe(true); }); - it.each(actions)('renders a dropdown item for %s', (action) => { - const dropdownAction = wrapper.find(`[data-testid="${action}"]`); - expect(dropdownAction.exists()).toBe(true); - expect(dropdownAction.attributes('href')).toBe(userPaths[action]); + describe('when there are actions that should render as links', () => { + beforeEach(() => { + initComponent({ actions: LINK_ACTIONS }); + }); + + it.each(LINK_ACTIONS)('renders an action component item for "%s"', (action) => { + const component = wrapper.find(Actions[capitalizeFirstCharacter(action)]); + + expect(component.props('path')).toBe(userPaths[action]); + expect(component.text()).toBe(I18N_USER_ACTIONS[action]); + }); + }); + + describe('when there are actions that require confirmation', () => { + beforeEach(() => { + initComponent({ actions: CONFIRMATION_ACTIONS }); + }); + + it.each(CONFIRMATION_ACTIONS)('renders an action component item for "%s"', (action) => { + const component = wrapper.find(Actions[capitalizeFirstCharacter(action)]); + + expect(component.props('username')).toBe(user.name); + expect(component.props('path')).toBe(userPaths[action]); + expect(component.text()).toBe(I18N_USER_ACTIONS[action]); + }); }); describe('when there is a LDAP action', () => { @@ -87,14 +106,13 @@ describe('AdminUserActions component', () => { const dropdownAction = wrapper.find(`[data-testid="${LDAP}"]`); expect(dropdownAction.exists()).toBe(true); expect(dropdownAction.attributes('href')).toBe(undefined); + expect(dropdownAction.text()).toBe(I18N_USER_ACTIONS[LDAP]); }); }); describe('when there is a delete action', () => { - const deleteActions = [DELETE, DELETE_WITH_CONTRIBUTIONS]; - beforeEach(() => { - initComponent({ actions: [BLOCK, ...deleteActions] }); + initComponent({ actions: [LDAP, ...DELETE_ACTIONS] }); }); it('renders a dropdown divider', () => { @@ -103,13 +121,15 @@ describe('AdminUserActions component', () => { it('only renders delete dropdown items for actions containing the word "delete"', () => { const { length } = wrapper.findAll(`[data-testid*="delete-"]`); - expect(length).toBe(deleteActions.length); + expect(length).toBe(DELETE_ACTIONS.length); }); - it.each(deleteActions)('renders a delete dropdown item for %s', (action) => { - const deleteAction = wrapper.find(`[data-testid="delete-${action}"]`); - expect(deleteAction.exists()).toBe(true); - expect(deleteAction.attributes('href')).toBe(userPaths[action]); + it.each(DELETE_ACTIONS)('renders a delete action component item for "%s"', (action) => { + const component = wrapper.find(Actions[capitalizeFirstCharacter(action)]); + + expect(component.props('username')).toBe(user.name); + expect(component.props('paths')).toEqual(userPaths); + expect(component.text()).toBe(I18N_USER_ACTIONS[action]); }); }); diff --git a/spec/frontend/admin/users/constants.js b/spec/frontend/admin/users/constants.js new file mode 100644 index 00000000000..60abdc6c248 --- /dev/null +++ b/spec/frontend/admin/users/constants.js @@ -0,0 +1,19 @@ +const BLOCK = 'block'; +const UNBLOCK = 'unblock'; +const DELETE = 'delete'; +const DELETE_WITH_CONTRIBUTIONS = 'deleteWithContributions'; +const UNLOCK = 'unlock'; +const ACTIVATE = 'activate'; +const DEACTIVATE = 'deactivate'; +const REJECT = 'reject'; +const APPROVE = 'approve'; + +export const EDIT = 'edit'; + +export const LDAP = 'ldapBlocked'; + +export const LINK_ACTIONS = [APPROVE, REJECT]; + +export const CONFIRMATION_ACTIONS = [ACTIVATE, BLOCK, DEACTIVATE, UNLOCK, UNBLOCK]; + +export const DELETE_ACTIONS = [DELETE, DELETE_WITH_CONTRIBUTIONS]; diff --git a/spec/frontend/issuable_show/components/issuable_header_spec.js b/spec/frontend/issuable_show/components/issuable_header_spec.js index f9c20ab04b8..3363a127347 100644 --- a/spec/frontend/issuable_show/components/issuable_header_spec.js +++ b/spec/frontend/issuable_show/components/issuable_header_spec.js @@ -1,5 +1,6 @@ import { shallowMount } from '@vue/test-utils'; import { GlIcon, GlAvatarLabeled } from '@gitlab/ui'; +import { extendedWrapper } from 'helpers/vue_test_utils_helper'; import IssuableHeader from '~/issuable_show/components/issuable_header.vue'; @@ -10,21 +11,23 @@ const issuableHeaderProps = { ...mockIssuableShowProps, }; -const createComponent = (propsData = issuableHeaderProps) => - shallowMount(IssuableHeader, { - propsData, - slots: { - 'status-badge': 'Open', - 'header-actions': ` +const createComponent = (propsData = issuableHeaderProps, { stubs } = {}) => + extendedWrapper( + shallowMount(IssuableHeader, { + propsData, + slots: { + 'status-badge': 'Open', + 'header-actions': ` <button class="js-close">Close issuable</button> <a class="js-new" href="/gitlab-org/gitlab-shell/-/issues/new">New issuable</a> `, - }, - }); + }, + stubs, + }), + ); describe('IssuableHeader', () => { let wrapper; - const findByTestId = (testId) => wrapper.find(`[data-testid="${testId}"]`); beforeEach(() => { wrapper = createComponent(); @@ -63,7 +66,7 @@ describe('IssuableHeader', () => { describe('template', () => { it('renders issuable status icon and text', () => { - const statusBoxEl = findByTestId('status'); + const statusBoxEl = wrapper.findByTestId('status'); expect(statusBoxEl.exists()).toBe(true); expect(statusBoxEl.find(GlIcon).props('name')).toBe(mockIssuableShowProps.statusIcon); @@ -77,7 +80,7 @@ describe('IssuableHeader', () => { await wrapper.vm.$nextTick(); - const blockedEl = findByTestId('blocked'); + const blockedEl = wrapper.findByTestId('blocked'); expect(blockedEl.exists()).toBe(true); expect(blockedEl.find(GlIcon).props('name')).toBe('lock'); @@ -90,7 +93,7 @@ describe('IssuableHeader', () => { await wrapper.vm.$nextTick(); - const confidentialEl = findByTestId('confidential'); + const confidentialEl = wrapper.findByTestId('confidential'); expect(confidentialEl.exists()).toBe(true); expect(confidentialEl.find(GlIcon).props('name')).toBe('eye-slash'); @@ -105,7 +108,7 @@ describe('IssuableHeader', () => { href: webUrl, target: '_blank', }; - const avatarEl = findByTestId('avatar'); + const avatarEl = wrapper.findByTestId('avatar'); expect(avatarEl.exists()).toBe(true); expect(avatarEl.attributes()).toMatchObject(avatarElAttrs); expect(avatarEl.find(GlAvatarLabeled).attributes()).toMatchObject({ @@ -113,20 +116,46 @@ describe('IssuableHeader', () => { src: avatarUrl, label: name, }); + expect(avatarEl.find(GlAvatarLabeled).find(GlIcon).exists()).toBe(false); }); it('renders sidebar toggle button', () => { - const toggleButtonEl = findByTestId('sidebar-toggle'); + const toggleButtonEl = wrapper.findByTestId('sidebar-toggle'); expect(toggleButtonEl.exists()).toBe(true); expect(toggleButtonEl.props('icon')).toBe('chevron-double-lg-left'); }); it('renders header actions', () => { - const actionsEl = findByTestId('header-actions'); + const actionsEl = wrapper.findByTestId('header-actions'); expect(actionsEl.find('button.js-close').exists()).toBe(true); expect(actionsEl.find('a.js-new').exists()).toBe(true); }); + + describe('when author exists outside of GitLab', () => { + it("renders 'external-link' icon in avatar label", () => { + wrapper = createComponent( + { + ...issuableHeaderProps, + author: { + ...issuableHeaderProps.author, + webUrl: 'https://jira.com/test-user/author.jpg', + }, + }, + { + stubs: { + GlAvatarLabeled, + }, + }, + ); + + const avatarEl = wrapper.findComponent(GlAvatarLabeled); + const icon = avatarEl.find(GlIcon); + + expect(icon.exists()).toBe(true); + expect(icon.props('name')).toBe('external-link'); + }); + }); }); }); diff --git a/spec/frontend/lib/utils/url_utility_spec.js b/spec/frontend/lib/utils/url_utility_spec.js index 5846acbdb79..61df3aa19c2 100644 --- a/spec/frontend/lib/utils/url_utility_spec.js +++ b/spec/frontend/lib/utils/url_utility_spec.js @@ -492,6 +492,28 @@ describe('URL utility', () => { }); }); + describe('isExternal', () => { + const gitlabUrl = 'https://gitlab.com/'; + + beforeEach(() => { + gon.gitlab_url = gitlabUrl; + }); + + afterEach(() => { + gon.gitlab_url = ''; + }); + + it.each` + url | urlType | external + ${'/gitlab-org/gitlab-test/-/issues/2'} | ${'relative'} | ${false} + ${gitlabUrl} | ${'absolute and internal'} | ${false} + ${`${gitlabUrl}/gitlab-org/gitlab-test`} | ${'absolute and internal'} | ${false} + ${'http://jira.atlassian.net/browse/IG-1'} | ${'absolute and external'} | ${true} + `('returns $external for $url ($urlType)', ({ url, external }) => { + expect(urlUtils.isExternal(url)).toBe(external); + }); + }); + describe('isBase64DataUrl', () => { it.each` url | valid diff --git a/spec/helpers/search_helper_spec.rb b/spec/helpers/search_helper_spec.rb index de7c0a46e47..4881d4a5b51 100644 --- a/spec/helpers/search_helper_spec.rb +++ b/spec/helpers/search_helper_spec.rb @@ -557,21 +557,31 @@ RSpec.describe SearchHelper do describe '#search_sort_options' do let(:user) { create(:user) } - mock_created_sort = { - title: _('Created date'), - sortable: true, - sortParam: { - asc: 'created_asc', - desc: 'created_desc' + mock_created_sort = [ + { + title: _('Created date'), + sortable: true, + sortParam: { + asc: 'created_asc', + desc: 'created_desc' + } + }, + { + title: _('Last updated'), + sortable: true, + sortParam: { + asc: 'updated_asc', + desc: 'updated_desc' + } } - } + ] before do allow(self).to receive(:current_user).and_return(user) end it 'returns the correct data' do - expect(search_sort_options).to eq([mock_created_sort]) + expect(search_sort_options).to eq(mock_created_sort) end end diff --git a/spec/lib/gitlab/auth/auth_finders_spec.rb b/spec/lib/gitlab/auth/auth_finders_spec.rb index 775f8f056b5..cddcaf09b74 100644 --- a/spec/lib/gitlab/auth/auth_finders_spec.rb +++ b/spec/lib/gitlab/auth/auth_finders_spec.rb @@ -7,7 +7,18 @@ RSpec.describe Gitlab::Auth::AuthFinders do include HttpBasicAuthHelpers # Create the feed_token and static_object_token for the user - let_it_be(:user) { create(:user).tap(&:feed_token).tap(&:static_object_token) } + let_it_be(:user, freeze: true) { create(:user).tap(&:feed_token).tap(&:static_object_token) } + let_it_be(:personal_access_token, freeze: true) { create(:personal_access_token, user: user) } + + let_it_be(:project, freeze: true) { create(:project, :private) } + let_it_be(:pipeline, freeze: true) { create(:ci_pipeline, project: project) } + let_it_be(:job, freeze: true) { create(:ci_build, :running, pipeline: pipeline, user: user) } + let_it_be(:failed_job, freeze: true) { create(:ci_build, :failed, pipeline: pipeline, user: user) } + + let_it_be(:project2, freeze: true) { create(:project, :private) } + let_it_be(:pipeline2, freeze: true) { create(:ci_pipeline, project: project2) } + let_it_be(:job2, freeze: true) { create(:ci_build, :running, pipeline: pipeline2, user: user) } + let(:env) do { 'rack.input' => '' @@ -15,6 +26,12 @@ RSpec.describe Gitlab::Auth::AuthFinders do end let(:request) { ActionDispatch::Request.new(env) } + let(:params) { {} } + + before_all do + project.add_developer(user) + project2.add_developer(user) + end def set_param(key, value) request.update_param(key, value) @@ -28,75 +45,93 @@ RSpec.describe Gitlab::Auth::AuthFinders do env.merge!(basic_auth_header(username, password)) end - shared_examples 'find user from job token' do + def set_bearer_token(token) + env['HTTP_AUTHORIZATION'] = "Bearer #{token}" + end + + shared_examples 'find user from job token' do |without_job_token_allowed| context 'when route is allowed to be authenticated' do let(:route_authentication_setting) { { job_token_allowed: true } } - it "returns an Unauthorized exception for an invalid token" do - set_token('invalid token') + context 'for an invalid token' do + let(:token) { 'invalid token' } - expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError) + it "returns an Unauthorized exception" do + expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError) + expect(@current_authenticated_job).to be_nil + end end context 'with a running job' do - before do - job.update!(status: :running) - end - - it 'return user if token is valid' do - set_token(job.token) + let(:token) { job.token } + it 'return user' do expect(subject).to eq(user) expect(@current_authenticated_job).to eq job end end context 'with a job that is not running' do - before do - job.update!(status: :failed) - end + let(:token) { failed_job.token } it 'returns an Unauthorized exception' do - set_token(job.token) - expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError) + expect(@current_authenticated_job).to be_nil + end + end + end + + context 'when route is not allowed to be authenticated' do + let(:route_authentication_setting) { { job_token_allowed: false } } + + context 'with a running job' do + let(:token) { job.token } + + if without_job_token_allowed == :error + it 'returns an Unauthorized exception' do + expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError) + expect(@current_authenticated_job).to be_nil + end + elsif without_job_token_allowed == :user + it 'returns the user' do + expect(subject).to eq(user) + expect(@current_authenticated_job).to eq job + end + else + it 'returns nil' do + is_expected.to be_nil + expect(@current_authenticated_job).to be_nil + end end end end end describe '#find_user_from_bearer_token' do - let_it_be_with_reload(:job) { create(:ci_build, user: user) } - subject { find_user_from_bearer_token } context 'when the token is passed as an oauth token' do - def set_token(token) - env['HTTP_AUTHORIZATION'] = "Bearer #{token}" + before do + set_bearer_token(token) end - context 'with a job token' do - it_behaves_like 'find user from job token' - end + it_behaves_like 'find user from job token', :error + end - context 'with oauth token' do - let(:application) { Doorkeeper::Application.create!(name: 'MyApp', redirect_uri: 'https://app.com', owner: user) } - let(:token) { Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: 'api').token } + context 'with oauth token' do + let(:application) { Doorkeeper::Application.create!(name: 'MyApp', redirect_uri: 'https://app.com', owner: user) } + let(:doorkeeper_access_token) { Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: 'api') } - before do - set_token(token) - end - - it { is_expected.to eq user } + before do + set_bearer_token(doorkeeper_access_token.token) end + + it { is_expected.to eq user } end context 'with a personal access token' do - let_it_be(:pat) { create(:personal_access_token, user: user) } - let(:token) { pat.token } - before do - env[described_class::PRIVATE_TOKEN_HEADER] = pat.token + env[described_class::PRIVATE_TOKEN_HEADER] = personal_access_token.token end it { is_expected.to eq user } @@ -277,7 +312,7 @@ RSpec.describe Gitlab::Auth::AuthFinders do end describe '#deploy_token_from_request' do - let_it_be(:deploy_token) { create(:deploy_token) } + let_it_be(:deploy_token, freeze: true) { create(:deploy_token) } let_it_be(:route_authentication_setting) { { deploy_token_allowed: true } } subject { deploy_token_from_request } @@ -293,11 +328,13 @@ RSpec.describe Gitlab::Auth::AuthFinders do end context 'with deploy token headers' do - before do - set_header(described_class::DEPLOY_TOKEN_HEADER, deploy_token.token) - end + context 'with valid deploy token' do + before do + set_header(described_class::DEPLOY_TOKEN_HEADER, deploy_token.token) + end - it { is_expected.to eq deploy_token } + it { is_expected.to eq deploy_token } + end it_behaves_like 'an unauthenticated route' @@ -311,17 +348,19 @@ RSpec.describe Gitlab::Auth::AuthFinders do end context 'with oauth headers' do - before do - set_header('HTTP_AUTHORIZATION', "Bearer #{deploy_token.token}") - end + context 'with valid token' do + before do + set_bearer_token(deploy_token.token) + end - it { is_expected.to eq deploy_token } + it { is_expected.to eq deploy_token } - it_behaves_like 'an unauthenticated route' + it_behaves_like 'an unauthenticated route' + end context 'with invalid token' do before do - set_header('HTTP_AUTHORIZATION', "Bearer invalid_token") + set_bearer_token('invalid_token') end it { is_expected.to be_nil } @@ -348,8 +387,6 @@ RSpec.describe Gitlab::Auth::AuthFinders do end describe '#find_user_from_access_token' do - let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } - before do set_header('SCRIPT_NAME', 'url.atom') end @@ -374,24 +411,34 @@ RSpec.describe Gitlab::Auth::AuthFinders do end context 'with OAuth headers' do - it 'returns user' do - set_header('HTTP_AUTHORIZATION', "Bearer #{personal_access_token.token}") + context 'with valid personal access token' do + before do + set_bearer_token(personal_access_token.token) + end - expect(find_user_from_access_token).to eq user + it 'returns user' do + expect(find_user_from_access_token).to eq user + end end - it 'returns exception if invalid personal_access_token' do - env['HTTP_AUTHORIZATION'] = 'Bearer invalid_20byte_token' + context 'with invalid personal_access_token' do + before do + set_bearer_token('invalid_20byte_token') + end - expect { find_personal_access_token }.to raise_error(Gitlab::Auth::UnauthorizedError) + it 'returns exception' do + expect { find_personal_access_token }.to raise_error(Gitlab::Auth::UnauthorizedError) + end end context 'when using a non-prefixed access token' do - let_it_be(:personal_access_token) { create(:personal_access_token, :no_prefix, user: user) } + let_it_be(:personal_access_token, freeze: true) { create(:personal_access_token, :no_prefix, user: user) } - it 'returns user' do - set_header('HTTP_AUTHORIZATION', "Bearer #{personal_access_token.token}") + before do + set_bearer_token(personal_access_token.token) + end + it 'returns user' do expect(find_user_from_access_token).to eq user end end @@ -399,8 +446,6 @@ RSpec.describe Gitlab::Auth::AuthFinders do end describe '#find_user_from_web_access_token' do - let_it_be_with_reload(:personal_access_token) { create(:personal_access_token, user: user) } - before do set_header(described_class::PRIVATE_TOKEN_HEADER, personal_access_token.token) end @@ -451,9 +496,9 @@ RSpec.describe Gitlab::Auth::AuthFinders do end context 'when the token has read_api scope' do - before do - personal_access_token.update!(scopes: ['read_api']) + let_it_be(:personal_access_token, freeze: true) { create(:personal_access_token, user: user, scopes: ['read_api']) } + before do set_header('SCRIPT_NAME', '/api/endpoint') end @@ -481,8 +526,6 @@ RSpec.describe Gitlab::Auth::AuthFinders do end describe '#find_personal_access_token' do - let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } - before do set_header('SCRIPT_NAME', 'url.atom') end @@ -516,21 +559,23 @@ RSpec.describe Gitlab::Auth::AuthFinders do describe '#find_oauth_access_token' do let(:application) { Doorkeeper::Application.create!(name: 'MyApp', redirect_uri: 'https://app.com', owner: user) } - let(:token) { Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: 'api') } + let(:doorkeeper_access_token) { Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: 'api') } context 'passed as header' do - it 'returns token if valid oauth_access_token' do - set_header('HTTP_AUTHORIZATION', "Bearer #{token.token}") + before do + set_bearer_token(doorkeeper_access_token.token) + end - expect(find_oauth_access_token.token).to eq token.token + it 'returns token if valid oauth_access_token' do + expect(find_oauth_access_token.token).to eq doorkeeper_access_token.token end end context 'passed as param' do it 'returns user if valid oauth_access_token' do - set_param(:access_token, token.token) + set_param(:access_token, doorkeeper_access_token.token) - expect(find_oauth_access_token.token).to eq token.token + expect(find_oauth_access_token.token).to eq doorkeeper_access_token.token end end @@ -538,10 +583,14 @@ RSpec.describe Gitlab::Auth::AuthFinders do expect(find_oauth_access_token).to be_nil end - it 'returns exception if invalid oauth_access_token' do - set_header('HTTP_AUTHORIZATION', "Bearer invalid_token") + context 'with invalid token' do + before do + set_bearer_token('invalid_token') + end - expect { find_oauth_access_token }.to raise_error(Gitlab::Auth::UnauthorizedError) + it 'returns exception if invalid oauth_access_token' do + expect { find_oauth_access_token }.to raise_error(Gitlab::Auth::UnauthorizedError) + end end end @@ -551,7 +600,6 @@ RSpec.describe Gitlab::Auth::AuthFinders do end context 'access token is valid' do - let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } let(:route_authentication_setting) { { basic_auth_personal_access_token: true } } it 'finds the token from basic auth' do @@ -572,8 +620,6 @@ RSpec.describe Gitlab::Auth::AuthFinders do end context 'route_setting is not set' do - let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } - it 'returns nil' do auth_header_with(personal_access_token.token) @@ -582,7 +628,6 @@ RSpec.describe Gitlab::Auth::AuthFinders do end context 'route_setting is not correct' do - let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } let(:route_authentication_setting) { { basic_auth_personal_access_token: false } } it 'returns nil' do @@ -629,44 +674,18 @@ RSpec.describe Gitlab::Auth::AuthFinders do context 'with CI username' do let(:username) { ::Gitlab::Auth::CI_JOB_USER } - let_it_be(:user) { create(:user) } - let_it_be(:build) { create(:ci_build, user: user, status: :running) } - - it 'returns nil without password' do - set_basic_auth_header(username, nil) - - is_expected.to be_nil - end - - it 'returns user with valid token' do - set_basic_auth_header(username, build.token) - - is_expected.to eq user - expect(@current_authenticated_job).to eq build - end - - it 'raises error with invalid token' do - set_basic_auth_header(username, 'token') - - expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError) + before do + set_basic_auth_header(username, token) end - it 'returns exception if the job is not running' do - set_basic_auth_header(username, build.token) - build.success! - - expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError) - end + it_behaves_like 'find user from job token', :user end end describe '#validate_access_token!' do subject { validate_access_token! } - let_it_be_with_reload(:personal_access_token) { create(:personal_access_token, user: user) } - context 'with a job token' do - let_it_be(:job) { create(:ci_build, user: user, status: :running) } let(:route_authentication_setting) { { job_token_allowed: true } } before do @@ -684,6 +703,8 @@ RSpec.describe Gitlab::Auth::AuthFinders do end context 'token is not valid' do + let_it_be_with_reload(:personal_access_token) { create(:personal_access_token, user: user) } + before do allow_any_instance_of(described_class).to receive(:access_token).and_return(personal_access_token) end @@ -706,7 +727,7 @@ RSpec.describe Gitlab::Auth::AuthFinders do end context 'with impersonation token' do - let_it_be(:personal_access_token) { create(:personal_access_token, :impersonation, user: user) } + let_it_be(:personal_access_token, freeze: true) { create(:personal_access_token, :impersonation, user: user) } context 'when impersonation is disabled' do before do @@ -722,96 +743,30 @@ RSpec.describe Gitlab::Auth::AuthFinders do end describe '#find_user_from_job_token' do - let_it_be(:job) { create(:ci_build, user: user, status: :running) } - let(:route_authentication_setting) { { job_token_allowed: true } } - subject { find_user_from_job_token } - context 'when the job token is in the headers' do - it 'returns the user if valid job token' do - set_header(described_class::JOB_TOKEN_HEADER, job.token) - - is_expected.to eq(user) - expect(@current_authenticated_job).to eq(job) - end - - it 'returns nil without job token' do - set_header(described_class::JOB_TOKEN_HEADER, '') - - is_expected.to be_nil - end - - it 'returns exception if invalid job token' do - set_header(described_class::JOB_TOKEN_HEADER, 'invalid token') - - expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError) + context 'when the token is in the headers' do + before do + set_header(described_class::JOB_TOKEN_HEADER, token) end - it 'returns exception if the job is not running' do - set_header(described_class::JOB_TOKEN_HEADER, job.token) - job.success! + it_behaves_like 'find user from job token' + end - expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError) + context 'when the token is in the job_token param' do + before do + set_param(described_class::JOB_TOKEN_PARAM, token) end - context 'when route is not allowed to be authenticated' do - let(:route_authentication_setting) { { job_token_allowed: false } } - - it 'sets current_user to nil' do - set_header(described_class::JOB_TOKEN_HEADER, job.token) - - allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(true) - - is_expected.to be_nil - end - end + it_behaves_like 'find user from job token' end - context 'when the job token is in the params' do - shared_examples 'job token params' do |token_key_name| - before do - set_param(token_key_name, token) - end - - context 'with valid job token' do - let(:token) { job.token } - - it 'returns the user' do - is_expected.to eq(user) - expect(@current_authenticated_job).to eq(job) - end - end - - context 'with empty job token' do - let(:token) { '' } - - it 'returns nil' do - is_expected.to be_nil - end - end - - context 'with invalid job token' do - let(:token) { 'invalid token' } - - it 'returns exception' do - expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError) - end - end - - context 'when route is not allowed to be authenticated' do - let(:route_authentication_setting) { { job_token_allowed: false } } - let(:token) { job.token } - - it 'sets current_user to nil' do - allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(true) - - is_expected.to be_nil - end - end + context 'when the token is in the token param' do + before do + set_param(described_class::RUNNER_JOB_TOKEN_PARAM, token) end - it_behaves_like 'job token params', described_class::JOB_TOKEN_PARAM - it_behaves_like 'job token params', described_class::RUNNER_JOB_TOKEN_PARAM + it_behaves_like 'find user from job token' end context 'when the job token is provided via basic auth' do @@ -834,7 +789,7 @@ RSpec.describe Gitlab::Auth::AuthFinders do end describe '#cluster_agent_token_from_authorization_token' do - let_it_be(:agent_token) { create(:cluster_agent_token) } + let_it_be(:agent_token, freeze: true) { create(:cluster_agent_token) } context 'when route_setting is empty' do it 'returns nil' do @@ -884,7 +839,7 @@ RSpec.describe Gitlab::Auth::AuthFinders do end describe '#find_runner_from_token' do - let_it_be(:runner) { create(:ci_runner) } + let_it_be(:runner, freeze: true) { create(:ci_runner) } context 'with API requests' do before do diff --git a/spec/lib/gitlab/search_results_spec.rb b/spec/lib/gitlab/search_results_spec.rb index 158d472f7ea..a1b18172a31 100644 --- a/spec/lib/gitlab/search_results_spec.rb +++ b/spec/lib/gitlab/search_results_spec.rb @@ -178,12 +178,18 @@ RSpec.describe Gitlab::SearchResults do end context 'ordering' do - let(:query) { 'sorted' } let!(:old_result) { create(:merge_request, :opened, source_project: project, source_branch: 'old-1', title: 'sorted old', created_at: 1.month.ago) } let!(:new_result) { create(:merge_request, :opened, source_project: project, source_branch: 'new-1', title: 'sorted recent', created_at: 1.day.ago) } let!(:very_old_result) { create(:merge_request, :opened, source_project: project, source_branch: 'very-old-1', title: 'sorted very old', created_at: 1.year.ago) } - include_examples 'search results sorted' + let!(:old_updated) { create(:merge_request, :opened, source_project: project, source_branch: 'updated-old-1', title: 'updated old', updated_at: 1.month.ago) } + let!(:new_updated) { create(:merge_request, :opened, source_project: project, source_branch: 'updated-new-1', title: 'updated recent', updated_at: 1.day.ago) } + let!(:very_old_updated) { create(:merge_request, :opened, source_project: project, source_branch: 'updated-very-old-1', title: 'updated very old', updated_at: 1.year.ago) } + + include_examples 'search results sorted' do + let(:results_created) { described_class.new(user, 'sorted', Project.order(:id), sort: sort, filters: filters) } + let(:results_updated) { described_class.new(user, 'updated', Project.order(:id), sort: sort, filters: filters) } + end end end @@ -214,12 +220,18 @@ RSpec.describe Gitlab::SearchResults do end context 'ordering' do - let(:query) { 'sorted' } let!(:old_result) { create(:issue, project: project, title: 'sorted old', created_at: 1.month.ago) } let!(:new_result) { create(:issue, project: project, title: 'sorted recent', created_at: 1.day.ago) } let!(:very_old_result) { create(:issue, project: project, title: 'sorted very old', created_at: 1.year.ago) } - include_examples 'search results sorted' + let!(:old_updated) { create(:issue, project: project, title: 'updated old', updated_at: 1.month.ago) } + let!(:new_updated) { create(:issue, project: project, title: 'updated recent', updated_at: 1.day.ago) } + let!(:very_old_updated) { create(:issue, project: project, title: 'updated very old', updated_at: 1.year.ago) } + + include_examples 'search results sorted' do + let(:results_created) { described_class.new(user, 'sorted', Project.order(:id), sort: sort, filters: filters) } + let(:results_updated) { described_class.new(user, 'updated', Project.order(:id), sort: sort, filters: filters) } + end end end diff --git a/spec/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter_spec.rb b/spec/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter_spec.rb index 85ea23a8a48..d939fceef0a 100644 --- a/spec/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter_spec.rb +++ b/spec/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter_spec.rb @@ -228,4 +228,28 @@ RSpec.describe Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter, :cl let(:action) { described_class::MR_REVIEW_REQUESTED_USERS_ACTION } end end + + describe '.track_approval_rule_added_action' do + subject { described_class.track_approval_rule_added_action(user: user) } + + it_behaves_like 'a tracked merge request unique event' do + let(:action) { described_class::MR_APPROVAL_RULE_ADDED_USERS_ACTION } + end + end + + describe '.track_approval_rule_edited_action' do + subject { described_class.track_approval_rule_edited_action(user: user) } + + it_behaves_like 'a tracked merge request unique event' do + let(:action) { described_class::MR_APPROVAL_RULE_EDITED_USERS_ACTION } + end + end + + describe '.track_approval_rule_deleted_action' do + subject { described_class.track_approval_rule_deleted_action(user: user) } + + it_behaves_like 'a tracked merge request unique event' do + let(:action) { described_class::MR_APPROVAL_RULE_DELETED_USERS_ACTION } + end + end end diff --git a/spec/lib/peek/views/external_http_spec.rb b/spec/lib/peek/views/external_http_spec.rb index cc1813db622..98c4f771f33 100644 --- a/spec/lib/peek/views/external_http_spec.rb +++ b/spec/lib/peek/views/external_http_spec.rb @@ -12,29 +12,29 @@ RSpec.describe Peek::Views::ExternalHttp, :request_store do end let(:event_1) do - double(:event, payload: { + { method: 'POST', code: "200", duration: 0.03, scheme: 'https', host: 'gitlab.com', port: 80, path: '/api/v4/projects', query: 'current=true' - }) + } end let(:event_2) do - double(:event, payload: { + { method: 'POST', duration: 1.3, scheme: 'http', host: 'gitlab.com', port: 80, path: '/api/v4/projects/2/issues', query: 'current=true', exception_object: Net::ReadTimeout.new - }) + } end let(:event_3) do - double(:event, payload: { + { method: 'GET', code: "301", duration: 0.005, scheme: 'http', host: 'gitlab.com', port: 80, path: '/api/v4/projects/2', query: 'current=true', proxy_host: 'proxy.gitlab.com', proxy_port: 8080 - }) + } end it 'returns no results' do @@ -44,9 +44,9 @@ RSpec.describe Peek::Views::ExternalHttp, :request_store do end it 'returns aggregated results' do - subscriber.request(event_1) - subscriber.request(event_2) - subscriber.request(event_3) + subscriber.request(double(:event, payload: event_1)) + subscriber.request(double(:event, payload: event_2)) + subscriber.request(double(:event, payload: event_3)) results = subject.results expect(results[:calls]).to eq(3) @@ -86,105 +86,129 @@ RSpec.describe Peek::Views::ExternalHttp, :request_store do end context 'when the host is in IPv4 format' do + before do + event_1[:host] = '1.2.3.4' + end + it 'displays IPv4 in the label' do - subscriber.request( - double(:event, payload: { - method: 'POST', code: "200", duration: 0.03, - scheme: 'https', host: '1.2.3.4', port: 80, path: '/api/v4/projects', - query: 'current=true' - }) - ) - expect( - subject.results[:details].map { |data| data.slice(:duration, :label, :code, :proxy, :error, :warnings) } - ).to match_array( - [ - { - duration: 30.0, - label: "POST https://1.2.3.4:80/api/v4/projects?current=true", - code: "Response status: 200", - proxy: nil, - error: nil, - warnings: [] - } - ] + subscriber.request(double(:event, payload: event_1)) + + expect(subject.results[:details]).to contain_exactly( + a_hash_including( + duration: 30.0, + label: "POST https://1.2.3.4:80/api/v4/projects?current=true", + code: "Response status: 200", + proxy: nil, + error: nil, + warnings: [] + ) ) end end context 'when the host is in IPv6 foramat' do + before do + event_1[:host] = '2606:4700:90:0:f22e:fbec:5bed:a9b9' + end + it 'displays IPv6 in the label' do - subscriber.request( - double(:event, payload: { - method: 'POST', code: "200", duration: 0.03, - scheme: 'https', host: '2606:4700:90:0:f22e:fbec:5bed:a9b9', port: 80, path: '/api/v4/projects', - query: 'current=true' - }) + subscriber.request(double(:event, payload: event_1)) + + expect(subject.results[:details]).to contain_exactly( + a_hash_including( + duration: 30.0, + label: "POST https://[2606:4700:90:0:f22e:fbec:5bed:a9b9]:80/api/v4/projects?current=true", + code: "Response status: 200", + proxy: nil, + error: nil, + warnings: [] + ) ) - expect( - subject.results[:details].map { |data| data.slice(:duration, :label, :code, :proxy, :error, :warnings) } - ).to match_array( - [ - { - duration: 30.0, - label: "POST https://[2606:4700:90:0:f22e:fbec:5bed:a9b9]:80/api/v4/projects?current=true", - code: "Response status: 200", - proxy: nil, - error: nil, - warnings: [] - } - ] + end + end + + context 'when the query is a hash' do + before do + event_1[:query] = { current: true, 'item1' => 'string', 'item2' => [1, 2] } + end + + it 'converts query hash into a query string' do + subscriber.request(double(:event, payload: event_1)) + + expect(subject.results[:details]).to contain_exactly( + a_hash_including( + duration: 30.0, + label: "POST https://gitlab.com:80/api/v4/projects?current=true&item1=string&item2%5B%5D=1&item2%5B%5D=2", + code: "Response status: 200", + proxy: nil, + error: nil, + warnings: [] + ) ) end end context 'when the host is invalid' do + before do + event_1[:host] = '!@#%!@#%!@#%' + end + it 'displays unknown in the label' do - subscriber.request( - double(:event, payload: { - method: 'POST', code: "200", duration: 0.03, - scheme: 'https', host: '!@#%!@#%!@#%', port: 80, path: '/api/v4/projects', - query: 'current=true' - }) - ) - expect( - subject.results[:details].map { |data| data.slice(:duration, :label, :code, :proxy, :error, :warnings) } - ).to match_array( - [ - { - duration: 30.0, - label: "POST unknown", - code: "Response status: 200", - proxy: nil, - error: nil, - warnings: [] - } - ] + subscriber.request(double(:event, payload: event_1)) + + expect(subject.results[:details]).to contain_exactly( + a_hash_including( + duration: 30.0, + label: "POST unknown", + code: "Response status: 200", + proxy: nil, + error: nil, + warnings: [] + ) ) end end - context 'when another URI component is invalid' do + context 'when URI creation raises an URI::Error' do + before do + # This raises an URI::Error exception + event_1[:port] = 'invalid' + end + it 'displays unknown in the label' do - subscriber.request( - double(:event, payload: { - method: 'POST', code: "200", duration: 0.03, - scheme: 'https', host: 'invalid', port: 'invalid', path: '/api/v4/projects', - query: 'current=true' - }) + subscriber.request(double(:event, payload: event_1)) + + expect(subject.results[:details]).to contain_exactly( + a_hash_including( + duration: 30.0, + label: "POST unknown", + code: "Response status: 200", + proxy: nil, + error: nil, + warnings: [] + ) ) - expect( - subject.results[:details].map { |data| data.slice(:duration, :label, :code, :proxy, :error, :warnings) } - ).to match_array( - [ - { - duration: 30.0, - label: "POST unknown", - code: "Response status: 200", - proxy: nil, - error: nil, - warnings: [] - } - ] + end + end + + context 'when URI creation raises a StandardError exception' do + before do + # This raises a TypeError exception + event_1[:scheme] = 1234 + end + + it 'displays unknown in the label' do + subscriber.request(double(:event, payload: event_1)) + + expect(subject.results[:details]).to contain_exactly( + a_hash_including( + duration: 30.0, + label: "POST unknown", + code: "Response status: 200", + proxy: nil, + error: nil, + warnings: [] + ) ) end end diff --git a/spec/services/search/global_service_spec.rb b/spec/services/search/global_service_spec.rb index 7b914a4d3d6..a3ec150cd14 100644 --- a/spec/services/search/global_service_spec.rb +++ b/spec/services/search/global_service_spec.rb @@ -56,14 +56,20 @@ RSpec.describe Search::GlobalService do context 'issues' do let(:scope) { 'issues' } - context 'sort by created_at' do + context 'sorting' do let!(:project) { create(:project, :public) } + let!(:old_result) { create(:issue, project: project, title: 'sorted old', created_at: 1.month.ago) } let!(:new_result) { create(:issue, project: project, title: 'sorted recent', created_at: 1.day.ago) } let!(:very_old_result) { create(:issue, project: project, title: 'sorted very old', created_at: 1.year.ago) } + let!(:old_updated) { create(:issue, project: project, title: 'updated old', updated_at: 1.month.ago) } + let!(:new_updated) { create(:issue, project: project, title: 'updated recent', updated_at: 1.day.ago) } + let!(:very_old_updated) { create(:issue, project: project, title: 'updated very old', updated_at: 1.year.ago) } + include_examples 'search results sorted' do - let(:results) { described_class.new(nil, search: 'sorted', sort: sort).execute } + let(:results_created) { described_class.new(nil, search: 'sorted', sort: sort).execute } + let(:results_updated) { described_class.new(nil, search: 'updated', sort: sort).execute } end end end @@ -71,14 +77,20 @@ RSpec.describe Search::GlobalService do context 'merge_request' do let(:scope) { 'merge_requests' } - context 'sort by created_at' do + context 'sorting' do let!(:project) { create(:project, :public) } + let!(:old_result) { create(:merge_request, :opened, source_project: project, source_branch: 'old-1', title: 'sorted old', created_at: 1.month.ago) } let!(:new_result) { create(:merge_request, :opened, source_project: project, source_branch: 'new-1', title: 'sorted recent', created_at: 1.day.ago) } let!(:very_old_result) { create(:merge_request, :opened, source_project: project, source_branch: 'very-old-1', title: 'sorted very old', created_at: 1.year.ago) } + let!(:old_updated) { create(:merge_request, :opened, source_project: project, source_branch: 'updated-old-1', title: 'updated old', updated_at: 1.month.ago) } + let!(:new_updated) { create(:merge_request, :opened, source_project: project, source_branch: 'updated-new-1', title: 'updated recent', updated_at: 1.day.ago) } + let!(:very_old_updated) { create(:merge_request, :opened, source_project: project, source_branch: 'updated-very-old-1', title: 'updated very old', updated_at: 1.year.ago) } + include_examples 'search results sorted' do - let(:results) { described_class.new(nil, search: 'sorted', sort: sort).execute } + let(:results_created) { described_class.new(nil, search: 'sorted', sort: sort).execute } + let(:results_updated) { described_class.new(nil, search: 'updated', sort: sort).execute } end end end diff --git a/spec/services/search/group_service_spec.rb b/spec/services/search/group_service_spec.rb index 2bfe714f393..b954bd5f975 100644 --- a/spec/services/search/group_service_spec.rb +++ b/spec/services/search/group_service_spec.rb @@ -44,15 +44,21 @@ RSpec.describe Search::GroupService do context 'issues' do let(:scope) { 'issues' } - context 'sort by created_at' do + context 'sorting' do let!(:group) { create(:group) } let!(:project) { create(:project, :public, group: group) } + let!(:old_result) { create(:issue, project: project, title: 'sorted old', created_at: 1.month.ago) } let!(:new_result) { create(:issue, project: project, title: 'sorted recent', created_at: 1.day.ago) } let!(:very_old_result) { create(:issue, project: project, title: 'sorted very old', created_at: 1.year.ago) } + let!(:old_updated) { create(:issue, project: project, title: 'updated old', updated_at: 1.month.ago) } + let!(:new_updated) { create(:issue, project: project, title: 'updated recent', updated_at: 1.day.ago) } + let!(:very_old_updated) { create(:issue, project: project, title: 'updated very old', updated_at: 1.year.ago) } + include_examples 'search results sorted' do - let(:results) { described_class.new(nil, group, search: 'sorted', sort: sort).execute } + let(:results_created) { described_class.new(nil, group, search: 'sorted', sort: sort).execute } + let(:results_updated) { described_class.new(nil, group, search: 'updated', sort: sort).execute } end end end @@ -60,15 +66,21 @@ RSpec.describe Search::GroupService do context 'merge requests' do let(:scope) { 'merge_requests' } - context 'sort by created_at' do + context 'sorting' do let!(:group) { create(:group) } let!(:project) { create(:project, :public, group: group) } + let!(:old_result) { create(:merge_request, :opened, source_project: project, source_branch: 'old-1', title: 'sorted old', created_at: 1.month.ago) } let!(:new_result) { create(:merge_request, :opened, source_project: project, source_branch: 'new-1', title: 'sorted recent', created_at: 1.day.ago) } let!(:very_old_result) { create(:merge_request, :opened, source_project: project, source_branch: 'very-old-1', title: 'sorted very old', created_at: 1.year.ago) } + let!(:old_updated) { create(:merge_request, :opened, source_project: project, source_branch: 'updated-old-1', title: 'updated old', updated_at: 1.month.ago) } + let!(:new_updated) { create(:merge_request, :opened, source_project: project, source_branch: 'updated-new-1', title: 'updated recent', updated_at: 1.day.ago) } + let!(:very_old_updated) { create(:merge_request, :opened, source_project: project, source_branch: 'updated-very-old-1', title: 'updated very old', updated_at: 1.year.ago) } + include_examples 'search results sorted' do - let(:results) { described_class.new(nil, group, search: 'sorted', sort: sort).execute } + let(:results_created) { described_class.new(nil, group, search: 'sorted', sort: sort).execute } + let(:results_updated) { described_class.new(nil, group, search: 'updated', sort: sort).execute } end end end diff --git a/spec/support/shared_examples/lib/gitlab/search_results_sorted_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/search_results_sorted_shared_examples.rb index 07d01d5c50e..eafb49cef71 100644 --- a/spec/support/shared_examples/lib/gitlab/search_results_sorted_shared_examples.rb +++ b/spec/support/shared_examples/lib/gitlab/search_results_sorted_shared_examples.rb @@ -1,19 +1,35 @@ # frozen_string_literal: true RSpec.shared_examples 'search results sorted' do - context 'sort: newest' do + context 'sort: created_desc' do let(:sort) { 'created_desc' } it 'sorts results by created_at' do - expect(results.objects(scope).map(&:id)).to eq([new_result.id, old_result.id, very_old_result.id]) + expect(results_created.objects(scope).map(&:id)).to eq([new_result.id, old_result.id, very_old_result.id]) end end - context 'sort: oldest' do + context 'sort: created_asc' do let(:sort) { 'created_asc' } it 'sorts results by created_at' do - expect(results.objects(scope).map(&:id)).to eq([very_old_result.id, old_result.id, new_result.id]) + expect(results_created.objects(scope).map(&:id)).to eq([very_old_result.id, old_result.id, new_result.id]) + end + end + + context 'sort: updated_desc' do + let(:sort) { 'updated_desc' } + + it 'sorts results by updated_desc' do + expect(results_updated.objects(scope).map(&:id)).to eq([new_updated.id, old_updated.id, very_old_updated.id]) + end + end + + context 'sort: updated_asc' do + let(:sort) { 'updated_asc' } + + it 'sorts results by updated_asc' do + expect(results_updated.objects(scope).map(&:id)).to eq([very_old_updated.id, old_updated.id, new_updated.id]) end end end diff --git a/spec/support/shared_examples/models/concerns/repository_storage_movable_shared_examples.rb b/spec/support/shared_examples/models/concerns/repository_storage_movable_shared_examples.rb index 7a09a437ab3..819cf6018fe 100644 --- a/spec/support/shared_examples/models/concerns/repository_storage_movable_shared_examples.rb +++ b/spec/support/shared_examples/models/concerns/repository_storage_movable_shared_examples.rb @@ -99,6 +99,11 @@ RSpec.shared_examples 'handles repository moves' do expect(container).not_to be_repository_read_only end + + it 'updates the updated_at column of the container', :aggregate_failures do + expect { storage_move.finish_replication! }.to change { container.updated_at } + expect(storage_move.container.updated_at).to be >= storage_move.updated_at + end end context 'and transits to failed' do diff --git a/spec/tasks/gitlab/pages_rake_spec.rb b/spec/tasks/gitlab/pages_rake_spec.rb index 9c26d3d73c8..608458b2393 100644 --- a/spec/tasks/gitlab/pages_rake_spec.rb +++ b/spec/tasks/gitlab/pages_rake_spec.rb @@ -2,38 +2,58 @@ require 'rake_helper' -RSpec.describe 'gitlab:pages:migrate_legacy_storagerake task' do +RSpec.describe 'gitlab:pages' do before(:context) do Rake.application.rake_require 'tasks/gitlab/pages' end - subject { run_rake_task('gitlab:pages:migrate_legacy_storage') } + describe 'migrate_legacy_storage task' do + subject { run_rake_task('gitlab:pages:migrate_legacy_storage') } - it 'calls migration service' do - expect_next_instance_of(::Pages::MigrateFromLegacyStorageService, anything, 3, 10) do |service| - expect(service).to receive(:execute).and_call_original + it 'calls migration service' do + expect_next_instance_of(::Pages::MigrateFromLegacyStorageService, anything, 3, 10) do |service| + expect(service).to receive(:execute).and_call_original + end + + subject end - subject - end + it 'uses PAGES_MIGRATION_THREADS environment variable' do + stub_env('PAGES_MIGRATION_THREADS', '5') - it 'uses PAGES_MIGRATION_THREADS environment variable' do - stub_env('PAGES_MIGRATION_THREADS', '5') + expect_next_instance_of(::Pages::MigrateFromLegacyStorageService, anything, 5, 10) do |service| + expect(service).to receive(:execute).and_call_original + end - expect_next_instance_of(::Pages::MigrateFromLegacyStorageService, anything, 5, 10) do |service| - expect(service).to receive(:execute).and_call_original + subject end - subject - end + it 'uses PAGES_MIGRATION_BATCH_SIZE environment variable' do + stub_env('PAGES_MIGRATION_BATCH_SIZE', '100') - it 'uses PAGES_MIGRATION_BATCH_SIZE environment variable' do - stub_env('PAGES_MIGRATION_BATCH_SIZE', '100') + expect_next_instance_of(::Pages::MigrateFromLegacyStorageService, anything, 3, 100) do |service| + expect(service).to receive(:execute).and_call_original + end - expect_next_instance_of(::Pages::MigrateFromLegacyStorageService, anything, 3, 100) do |service| - expect(service).to receive(:execute).and_call_original + subject end + end + + describe 'clean_migrated_zip_storage task' do + it 'removes only migrated deployments' do + regular_deployment = create(:pages_deployment) + migrated_deployment = create(:pages_deployment, :migrated) - subject + regular_deployment.project.update_pages_deployment!(regular_deployment) + migrated_deployment.project.update_pages_deployment!(migrated_deployment) + + expect(PagesDeployment.all).to contain_exactly(regular_deployment, migrated_deployment) + + run_rake_task('gitlab:pages:clean_migrated_zip_storage') + + expect(PagesDeployment.all).to contain_exactly(regular_deployment) + expect(PagesDeployment.find_by_id(regular_deployment.id)).not_to be_nil + expect(PagesDeployment.find_by_id(migrated_deployment.id)).to be_nil + end end end |