diff options
51 files changed, 559 insertions, 386 deletions
diff --git a/app/assets/javascripts/environments/components/enable_review_app_modal.vue b/app/assets/javascripts/environments/components/enable_review_app_modal.vue index d770a2302e8..b757c55bfdb 100644 --- a/app/assets/javascripts/environments/components/enable_review_app_modal.vue +++ b/app/assets/javascripts/environments/components/enable_review_app_modal.vue @@ -12,11 +12,20 @@ export default { ModalCopyButton, }, inject: ['defaultBranchName'], + model: { + prop: 'visible', + event: 'change', + }, props: { modalId: { type: String, required: true, }, + visible: { + type: Boolean, + required: false, + default: false, + }, }, instructionText: { step1: s__( @@ -57,12 +66,15 @@ export default { </script> <template> <gl-modal + :visible="visible" :modal-id="modalId" :title="$options.modalInfo.title" + static size="lg" ok-only ok-variant="light" :ok-title="$options.modalInfo.closeText" + @change="$emit('change', $event)" > <p> <gl-sprintf :message="$options.instructionText.step1"> diff --git a/app/assets/javascripts/environments/components/new_environments_app.vue b/app/assets/javascripts/environments/components/new_environments_app.vue index bfb5689d623..fb86076b83c 100644 --- a/app/assets/javascripts/environments/components/new_environments_app.vue +++ b/app/assets/javascripts/environments/components/new_environments_app.vue @@ -1,12 +1,15 @@ <script> import { GlBadge, GlTab, GlTabs } from '@gitlab/ui'; +import { s__ } from '~/locale'; import environmentAppQuery from '../graphql/queries/environment_app.query.graphql'; import pollIntervalQuery from '../graphql/queries/poll_interval.query.graphql'; import EnvironmentFolder from './new_environment_folder.vue'; +import EnableReviewAppModal from './enable_review_app_modal.vue'; export default { components: { EnvironmentFolder, + EnableReviewAppModal, GlBadge, GlTab, GlTabs, @@ -22,22 +25,73 @@ export default { query: pollIntervalQuery, }, }, + inject: ['newEnvironmentPath', 'canCreateEnvironment'], + i18n: { + newEnvironmentButtonLabel: s__('Environments|New environment'), + reviewAppButtonLabel: s__('Environments|Enable review app'), + }, + modalId: 'enable-review-app-info', data() { - return { interval: undefined }; + return { interval: undefined, isReviewAppModalVisible: false }; }, computed: { + canSetupReviewApp() { + return this.environmentApp?.reviewApp?.canSetupReviewApp; + }, folders() { return this.environmentApp?.environments.filter((e) => e.size > 1) ?? []; }, availableCount() { return this.environmentApp?.availableCount; }, + addEnvironment() { + if (!this.canCreateEnvironment) { + return null; + } + + return { + text: this.$options.i18n.newEnvironmentButtonLabel, + attributes: { + href: this.newEnvironmentPath, + category: 'primary', + variant: 'confirm', + }, + }; + }, + openReviewAppModal() { + if (!this.canSetupReviewApp) { + return null; + } + + return { + text: this.$options.i18n.reviewAppButtonLabel, + attributes: { + category: 'secondary', + variant: 'confirm', + }, + }; + }, + }, + methods: { + showReviewAppModal() { + this.isReviewAppModalVisible = true; + }, }, }; </script> <template> <div> - <gl-tabs> + <enable-review-app-modal + v-if="canSetupReviewApp" + v-model="isReviewAppModalVisible" + :modal-id="$options.modalId" + data-testid="enable-review-app-modal" + /> + <gl-tabs + :action-secondary="addEnvironment" + :action-primary="openReviewAppModal" + @primary="showReviewAppModal" + > <gl-tab> <template #title> <span>{{ __('Available') }}</span> diff --git a/app/assets/javascripts/invite_members/components/invite_members_modal.vue b/app/assets/javascripts/invite_members/components/invite_members_modal.vue index 7163d1be773..91a139a5105 100644 --- a/app/assets/javascripts/invite_members/components/invite_members_modal.vue +++ b/app/assets/javascripts/invite_members/components/invite_members_modal.vue @@ -20,7 +20,6 @@ import { BV_SHOW_MODAL } from '~/lib/utils/constants'; import { getParameterValues } from '~/lib/utils/url_utility'; import { sprintf } from '~/locale'; import { - INVITE_MEMBERS_IN_COMMENT, GROUP_FILTERS, USERS_FILTER_ALL, INVITE_MEMBERS_FOR_TASK, @@ -254,11 +253,6 @@ export default { this.submitInviteMembers(); } }, - trackInvite() { - if (this.source === INVITE_MEMBERS_IN_COMMENT) { - this.trackEvent(INVITE_MEMBERS_IN_COMMENT, 'comment_invite_success'); - } - }, trackinviteMembersForTask() { const label = 'selected_tasks_to_be_done'; const property = this.selectedTasksToBeDone.join(','); @@ -312,7 +306,6 @@ export default { promises.push(apiAddByUserId(this.id, this.addByUserIdPostData(usersToAddById))); } - this.trackInvite(); this.trackinviteMembersForTask(); Promise.all(promises) diff --git a/app/assets/javascripts/invite_members/components/invite_members_trigger.vue b/app/assets/javascripts/invite_members/components/invite_members_trigger.vue index bf3250f63a5..7dd74f8803a 100644 --- a/app/assets/javascripts/invite_members/components/invite_members_trigger.vue +++ b/app/assets/javascripts/invite_members/components/invite_members_trigger.vue @@ -1,6 +1,5 @@ <script> import { GlButton, GlLink, GlIcon } from '@gitlab/ui'; -import ExperimentTracking from '~/experimentation/experiment_tracking'; import { s__ } from '~/locale'; import eventHub from '../event_hub'; import { TRIGGER_ELEMENT_BUTTON, TRIGGER_ELEMENT_SIDE_NAV } from '../constants'; @@ -32,11 +31,6 @@ export default { type: String, required: true, }, - trackExperiment: { - type: String, - required: false, - default: undefined, - }, triggerElement: { type: String, required: false, @@ -72,9 +66,6 @@ export default { return baseAttributes; }, }, - mounted() { - this.trackExperimentOnShow(); - }, methods: { checkTrigger(targetTriggerElement) { return this.triggerElement === targetTriggerElement; @@ -82,12 +73,6 @@ export default { openModal() { eventHub.$emit('openModal', { inviteeType: 'members', source: this.triggerSource }); }, - trackExperimentOnShow() { - if (this.trackExperiment) { - const tracking = new ExperimentTracking(this.trackExperiment); - tracking.event('comment_invite_shown'); - } - }, }, TRIGGER_ELEMENT_BUTTON, TRIGGER_ELEMENT_SIDE_NAV, diff --git a/app/assets/javascripts/invite_members/constants.js b/app/assets/javascripts/invite_members/constants.js index 87d2fbc6aac..ec59b3909fe 100644 --- a/app/assets/javascripts/invite_members/constants.js +++ b/app/assets/javascripts/invite_members/constants.js @@ -2,7 +2,6 @@ import { __, s__ } from '~/locale'; export const SEARCH_DELAY = 200; -export const INVITE_MEMBERS_IN_COMMENT = 'invite_members_in_comment'; export const INVITE_MEMBERS_FOR_TASK = { minimum_access_level: 30, name: 'invite_members_for_task', diff --git a/app/assets/javascripts/repository/commits_service.js b/app/assets/javascripts/repository/commits_service.js index 504efaea8cc..5fd9cfd4e53 100644 --- a/app/assets/javascripts/repository/commits_service.js +++ b/app/assets/javascripts/repository/commits_service.js @@ -52,14 +52,9 @@ export const loadCommits = async (projectPath, path, ref, offset) => { } // We fetch in batches of 25, so this ensures we don't refetch - Array.from(Array(COMMIT_BATCH_SIZE)).forEach((_, i) => { - addRequestedOffset(offset - i); - addRequestedOffset(offset + i); - }); + Array.from(Array(COMMIT_BATCH_SIZE)).forEach((_, i) => addRequestedOffset(offset + i)); - // Since a user could scroll either up or down, we want to support lazy loading in both directions - const commitsBatchUp = await fetchData(projectPath, path, ref, offset - COMMIT_BATCH_SIZE); - const commitsBatchDown = await fetchData(projectPath, path, ref, offset); + const commits = await fetchData(projectPath, path, ref, offset); - return commitsBatchUp.concat(commitsBatchDown); + return commits; }; diff --git a/app/assets/javascripts/repository/components/table/row.vue b/app/assets/javascripts/repository/components/table/row.vue index bd06c064ab7..8fcec5fb893 100644 --- a/app/assets/javascripts/repository/components/table/row.vue +++ b/app/assets/javascripts/repository/components/table/row.vue @@ -13,7 +13,7 @@ import { import { escapeRegExp } from 'lodash'; import paginatedTreeQuery from 'shared_queries/repository/paginated_tree.query.graphql'; import { escapeFileUrl } from '~/lib/utils/url_utility'; -import { TREE_PAGE_SIZE } from '~/repository/constants'; +import { TREE_PAGE_SIZE, ROW_APPEAR_DELAY } from '~/repository/constants'; import FileIcon from '~/vue_shared/components/file_icon.vue'; import TimeagoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; @@ -128,6 +128,7 @@ export default { return { commit: null, hasRowAppeared: false, + delayedRowAppear: null, }; }, computed: { @@ -202,14 +203,19 @@ export default { rowAppeared() { this.hasRowAppeared = true; + if (this.commitInfo) { + return; + } + if (this.glFeatures.lazyLoadCommits) { - this.$emit('row-appear', { - rowNumber: this.rowNumber, - hasCommit: Boolean(this.commitInfo), - }); + this.delayedRowAppear = setTimeout( + () => this.$emit('row-appear', this.rowNumber), + ROW_APPEAR_DELAY, + ); } }, rowDisappeared() { + clearTimeout(this.delayedRowAppear); this.hasRowAppeared = false; }, }, diff --git a/app/assets/javascripts/repository/components/tree_content.vue b/app/assets/javascripts/repository/components/tree_content.vue index ffe8d5531f8..130ebf77361 100644 --- a/app/assets/javascripts/repository/components/tree_content.vue +++ b/app/assets/javascripts/repository/components/tree_content.vue @@ -3,7 +3,12 @@ import paginatedTreeQuery from 'shared_queries/repository/paginated_tree.query.g import createFlash from '~/flash'; import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { __ } from '../../locale'; -import { TREE_PAGE_SIZE, TREE_INITIAL_FETCH_COUNT, TREE_PAGE_LIMIT } from '../constants'; +import { + TREE_PAGE_SIZE, + TREE_INITIAL_FETCH_COUNT, + TREE_PAGE_LIMIT, + COMMIT_BATCH_SIZE, +} from '../constants'; import getRefMixin from '../mixins/get_ref'; import projectPathQuery from '../queries/project_path.query.graphql'; import { readmeFile } from '../utils/readme'; @@ -151,11 +156,19 @@ export default { .concat(data.trees.pageInfo, data.submodules.pageInfo, data.blobs.pageInfo) .find(({ hasNextPage }) => hasNextPage); }, - loadCommitData({ rowNumber = 0, hasCommit } = {}) { - if (!this.glFeatures.lazyLoadCommits || hasCommit || isRequested(rowNumber)) { + handleRowAppear(rowNumber) { + if (!this.glFeatures.lazyLoadCommits || isRequested(rowNumber)) { return; } + // Since a user could scroll either up or down, we want to support lazy loading in both directions + this.loadCommitData(rowNumber); + + if (rowNumber - COMMIT_BATCH_SIZE >= 0) { + this.loadCommitData(rowNumber - COMMIT_BATCH_SIZE); + } + }, + loadCommitData(rowNumber) { loadCommits(this.projectPath, this.path, this.ref, rowNumber) .then(this.setCommitData) .catch(() => {}); @@ -182,7 +195,7 @@ export default { :has-more="hasShowMore" :commits="commits" @showMore="handleShowMore" - @row-appear="loadCommitData" + @row-appear="handleRowAppear" /> <file-preview v-if="readme" :blob="readme" /> </div> diff --git a/app/assets/javascripts/repository/constants.js b/app/assets/javascripts/repository/constants.js index cdc4818e493..d01757d6141 100644 --- a/app/assets/javascripts/repository/constants.js +++ b/app/assets/javascripts/repository/constants.js @@ -23,3 +23,5 @@ export const I18N_COMMIT_DATA_FETCH_ERROR = __('An error occurred while fetching export const PDF_MAX_FILE_SIZE = 10000000; // 10 MB export const PDF_MAX_PAGE_LIMIT = 50; + +export const ROW_APPEAR_DELAY = 150; diff --git a/app/assets/javascripts/vue_shared/components/markdown/toolbar.vue b/app/assets/javascripts/vue_shared/components/markdown/toolbar.vue index 912aa8ce294..f1c293c87f4 100644 --- a/app/assets/javascripts/vue_shared/components/markdown/toolbar.vue +++ b/app/assets/javascripts/vue_shared/components/markdown/toolbar.vue @@ -1,18 +1,13 @@ <script> import { GlButton, GlLink, GlLoadingIcon, GlSprintf, GlIcon } from '@gitlab/ui'; -import { isExperimentVariant } from '~/experimentation/utils'; -import InviteMembersTrigger from '~/invite_members/components/invite_members_trigger.vue'; -import { INVITE_MEMBERS_IN_COMMENT } from '~/invite_members/constants'; export default { - inviteMembersInComment: INVITE_MEMBERS_IN_COMMENT, components: { GlButton, GlLink, GlLoadingIcon, GlSprintf, GlIcon, - InviteMembersTrigger, }, props: { markdownDocsPath: { @@ -34,9 +29,6 @@ export default { hasQuickActionsDocsPath() { return this.quickActionsDocsPath !== ''; }, - inviteCommentEnabled() { - return isExperimentVariant(INVITE_MEMBERS_IN_COMMENT, 'invite_member_link'); - }, }, }; </script> @@ -67,16 +59,6 @@ export default { </template> </div> <span v-if="canAttachFile" class="uploading-container"> - <invite-members-trigger - v-if="inviteCommentEnabled" - classes="gl-mr-3 gl-vertical-align-text-bottom" - :display-text="s__('InviteMember|Invite Member')" - icon="assignee" - variant="link" - :track-experiment="$options.inviteMembersInComment" - :trigger-source="$options.inviteMembersInComment" - data-track-action="comment_invite_click" - /> <span class="uploading-progress-container hide"> <gl-icon name="media" /> <span class="attaching-file-message"></span> diff --git a/app/assets/stylesheets/highlight/common.scss b/app/assets/stylesheets/highlight/common.scss index fb4266a2f41..97dd7edef13 100644 --- a/app/assets/stylesheets/highlight/common.scss +++ b/app/assets/stylesheets/highlight/common.scss @@ -45,12 +45,12 @@ } } -@mixin line-number-hover($color) { - background-color: $color; - border-color: darken($color, 5%); +@mixin line-number-hover { + background-color: $purple-100; + border-color: $purple-200; a { - color: darken($color, 15%); + color: $gray-600; } } diff --git a/app/assets/stylesheets/highlight/themes/dark.scss b/app/assets/stylesheets/highlight/themes/dark.scss index 2d180f49f97..156f7ec3802 100644 --- a/app/assets/stylesheets/highlight/themes/dark.scss +++ b/app/assets/stylesheets/highlight/themes/dark.scss @@ -22,7 +22,6 @@ $dark-highlight-bg: #ffe792; $dark-highlight-color: $black; $dark-pre-hll-bg: #373b41; $dark-hll-bg: #373b41; -$dark-over-bg: #9f9ab5; $dark-expanded-bg: #3e3e3e; $dark-coverage: #b3e841; $dark-no-coverage: #ff4f33; @@ -93,7 +92,7 @@ $dark-il: #de935f; .file-line-num { @include line-number-link($dark-line-num-color); } - + .line-numbers, .diff-line-num { background-color: $dark-main-bg; @@ -171,14 +170,14 @@ $dark-il: #de935f; .diff-grid-left:hover, .diff-grid-right:hover { .diff-line-num:not(.empty-cell) { - @include line-number-hover($dark-over-bg); + @include line-number-hover; } } .diff-line-num { &.is-over, &.hll:not(.empty-cell).is-over { - @include line-number-hover($dark-over-bg); + @include line-number-hover; } } diff --git a/app/assets/stylesheets/highlight/themes/monokai.scss b/app/assets/stylesheets/highlight/themes/monokai.scss index c0931188cc3..5e259b72442 100644 --- a/app/assets/stylesheets/highlight/themes/monokai.scss +++ b/app/assets/stylesheets/highlight/themes/monokai.scss @@ -15,7 +15,6 @@ $monokai-line-empty-bg: #49483e; $monokai-line-empty-border: darken($monokai-line-empty-bg, 15%); $monokai-diff-border: #808080; $monokai-highlight-bg: #ffe792; -$monokai-over-bg: #9f9ab5; $monokai-expanded-bg: #3e3e3e; $monokai-coverage: #a6e22e; $monokai-no-coverage: #fd971f; @@ -172,14 +171,14 @@ $monokai-gh: #75715e; .diff-grid-left:hover, .diff-grid-right:hover { .diff-line-num:not(.empty-cell) { - @include line-number-hover($monokai-over-bg); + @include line-number-hover; } } .diff-line-num { &.is-over, &.hll:not(.empty-cell).is-over { - @include line-number-hover($monokai-over-bg); + @include line-number-hover; } } diff --git a/app/assets/stylesheets/highlight/themes/none.scss b/app/assets/stylesheets/highlight/themes/none.scss index ef7eb244b61..999719685b6 100644 --- a/app/assets/stylesheets/highlight/themes/none.scss +++ b/app/assets/stylesheets/highlight/themes/none.scss @@ -43,7 +43,6 @@ } // Diff line - $none-over-bg: #ded7fc; $none-expanded-border: #e0e0e0; $none-expanded-bg: #e0e0e0; @@ -69,7 +68,7 @@ .diff-grid-left:hover, .diff-grid-right:hover { .diff-line-num:not(.empty-cell) { - @include line-number-hover($none-over-bg); + @include line-number-hover; } } @@ -88,7 +87,7 @@ &.is-over, &.hll:not(.empty-cell).is-over { - @include line-number-hover($none-over-bg); + @include line-number-hover; } &.hll:not(.empty-cell) { diff --git a/app/assets/stylesheets/highlight/themes/solarized-dark.scss b/app/assets/stylesheets/highlight/themes/solarized-dark.scss index 8f09a178af1..8a20b323332 100644 --- a/app/assets/stylesheets/highlight/themes/solarized-dark.scss +++ b/app/assets/stylesheets/highlight/themes/solarized-dark.scss @@ -19,7 +19,6 @@ $solarized-dark-line-color-new: #5a766c; $solarized-dark-line-color-old: #7a6c71; $solarized-dark-highlight: #094554; $solarized-dark-hll-bg: #174652; -$solarized-dark-over-bg: #9f9ab5; $solarized-dark-expanded-bg: #010d10; $solarized-dark-coverage: #859900; $solarized-dark-no-coverage: #cb4b16; @@ -151,7 +150,7 @@ $solarized-dark-il: #2aa198; .diff-grid-left:hover, .diff-grid-right:hover { .diff-line-num:not(.empty-cell) { - @include line-number-hover($solarized-dark-over-bg); + @include line-number-hover; } } @@ -182,7 +181,7 @@ $solarized-dark-il: #2aa198; .diff-line-num { &.is-over, &.hll:not(.empty-cell).is-over { - @include line-number-hover($solarized-dark-over-bg); + @include line-number-hover; } } diff --git a/app/assets/stylesheets/highlight/themes/solarized-light.scss b/app/assets/stylesheets/highlight/themes/solarized-light.scss index 747cc639f91..1f1b13d0faa 100644 --- a/app/assets/stylesheets/highlight/themes/solarized-light.scss +++ b/app/assets/stylesheets/highlight/themes/solarized-light.scss @@ -20,7 +20,6 @@ $solarized-light-line-color-new: #a1a080; $solarized-light-line-color-old: #ad9186; $solarized-light-highlight: #eee8d5; $solarized-light-hll-bg: #ddd8c5; -$solarized-light-over-bg: #ded7fc; $solarized-light-expanded-border: #d2cdbd; $solarized-light-expanded-bg: #ece6d4; $solarized-light-coverage: #859900; @@ -171,7 +170,7 @@ $solarized-light-il: #2aa198; .diff-grid-left:hover, .diff-grid-right:hover { .diff-line-num:not(.empty-cell) { - @include line-number-hover($solarized-light-over-bg); + @include line-number-hover; } } @@ -190,7 +189,7 @@ $solarized-light-il: #2aa198; .diff-line-num { &.is-over, &.hll:not(.empty-cell).is-over { - @include line-number-hover($solarized-light-over-bg); + @include line-number-hover; } } diff --git a/app/assets/stylesheets/highlight/white_base.scss b/app/assets/stylesheets/highlight/white_base.scss index 86b01926dd7..f13c8bb3891 100644 --- a/app/assets/stylesheets/highlight/white_base.scss +++ b/app/assets/stylesheets/highlight/white_base.scss @@ -9,7 +9,6 @@ $white-code-color: $gl-text-color; $white-highlight: #fafe3d; $white-pre-hll-bg: #f8eec7; $white-hll-bg: #f8f8f8; -$white-over-bg: #ded7fc; $white-expanded-border: #e0e0e0; $white-expanded-bg: #f7f7f7; $white-c: #998; @@ -131,7 +130,7 @@ pre.code, .diff-grid-left:hover, .diff-grid-right:hover { .diff-line-num:not(.empty-cell):not(.conflict_marker_their):not(.conflict_marker_our) { - @include line-number-hover($white-over-bg); + @include line-number-hover; } } @@ -156,7 +155,7 @@ pre.code, &.is-over, &.hll:not(.empty-cell).is-over { - @include line-number-hover($white-over-bg); + @include line-number-hover; } &.hll:not(.empty-cell) { diff --git a/app/assets/stylesheets/pages/profile.scss b/app/assets/stylesheets/pages/profile.scss index af9f10c9a26..a7ed7172f5f 100644 --- a/app/assets/stylesheets/pages/profile.scss +++ b/app/assets/stylesheets/pages/profile.scss @@ -329,16 +329,6 @@ table.u2f-registrations { } } -.email-badge { - display: inline; - margin-right: $gl-padding / 2; - - .email-badge-email { - display: inline; - margin-right: $gl-padding / 4; - } -} - .edit-user { svg { fill: $gl-text-color-secondary; diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 345e4434f4d..d2d7ecfab6f 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -52,15 +52,6 @@ class Projects::IssuesController < Projects::ApplicationController push_frontend_feature_flag(:confidential_notes, @project, default_enabled: :yaml) push_frontend_feature_flag(:issue_assignees_widget, @project, default_enabled: :yaml) push_frontend_feature_flag(:paginated_issue_discussions, @project, default_enabled: :yaml) - - experiment(:invite_members_in_comment, namespace: @project.root_ancestor) do |experiment_instance| - experiment_instance.exclude! unless helpers.can_admin_project_member?(@project) - - experiment_instance.use {} - experiment_instance.try(:invite_member_link) {} - - experiment_instance.track(:view, property: @project.root_ancestor.id.to_s) - end end around_action :allow_gitaly_ref_name_caching, only: [:discussions] diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 3cffa9136d6..ccc34e2940e 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -47,15 +47,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo push_frontend_feature_flag(:users_expanding_widgets_usage_data, @project, default_enabled: :yaml) push_frontend_feature_flag(:diff_settings_usage_data, default_enabled: :yaml) push_frontend_feature_flag(:diff_searching_usage_data, @project, default_enabled: :yaml) - - experiment(:invite_members_in_comment, namespace: @project.root_ancestor) do |experiment_instance| - experiment_instance.exclude! unless helpers.can_admin_project_member?(@project) - - experiment_instance.use {} - experiment_instance.try(:invite_member_link) {} - - experiment_instance.track(:view, property: @project.root_ancestor.id.to_s) - end end before_action do diff --git a/app/views/projects/mirrors/_regenerate_public_ssh_key_confirm_modal.html.haml b/app/views/projects/mirrors/_regenerate_public_ssh_key_confirm_modal.html.haml deleted file mode 100644 index e6f3060af3e..00000000000 --- a/app/views/projects/mirrors/_regenerate_public_ssh_key_confirm_modal.html.haml +++ /dev/null @@ -1,13 +0,0 @@ -.modal.js-regenerate-public-ssh-key-confirm-modal{ tabindex: -1 } - .modal-dialog - .modal-content - .modal-header - %h3.modal-title.page-title - Regenerate public SSH key? - %button.close.js-cancel{ type: 'button', 'data-dismiss': 'modal', 'aria-label' => _('Close') } - %span{ 'aria-hidden': true } × - .modal-body - %p= _('Are you sure you want to regenerate the public key? You will have to update the public key on the remote server before mirroring will work again.') - .form-actions.modal-footer - = button_tag _('Cancel'), type: 'button', class: 'btn gl-button js-cancel' - = button_tag _('Regenerate key'), type: 'button', class: 'btn gl-button btn-inverted btn-warning js-confirm' diff --git a/app/views/shared/_email_with_badge.html.haml b/app/views/shared/_email_with_badge.html.haml index 8b9ca966ed6..5d837657943 100644 --- a/app/views/shared/_email_with_badge.html.haml +++ b/app/views/shared/_email_with_badge.html.haml @@ -1,8 +1,5 @@ -- css_classes = %w(badge gl-badge) -- css_classes << (verified ? 'badge-success': 'badge-danger') +- variant = verified ? :success : :danger - text = verified ? _('Verified') : _('Unverified') -.email-badge - .email-badge-email= email - %div{ class: css_classes } - = text += email += gl_badge_tag text, { variant: variant }, { class: 'gl-ml-3' } diff --git a/bin/metrics-server b/bin/metrics-server index 48d3a6402c3..16c98f65f52 100755 --- a/bin/metrics-server +++ b/bin/metrics-server @@ -8,8 +8,9 @@ begin raise "Required: METRICS_SERVER_TARGET=[sidekiq]" unless target == 'sidekiq' metrics_dir = ENV["prometheus_multiproc_dir"] || File.absolute_path("tmp/prometheus_multiproc_dir/#{target}") + wipe_metrics_dir = Gitlab::Utils.to_boolean(ENV['WIPE_METRICS_DIR']) || false # Re-raise exceptions in threads on the main thread. Thread.abort_on_exception = true - MetricsServer.new(target, metrics_dir).start + MetricsServer.new(target, metrics_dir, wipe_metrics_dir).start end diff --git a/config/feature_flags/experiment/invite_members_in_comment.yml b/config/feature_flags/experiment/invite_members_in_comment.yml deleted file mode 100644 index 521574ad71b..00000000000 --- a/config/feature_flags/experiment/invite_members_in_comment.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: invite_members_in_comment -introduced_by_url: 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/51400' -rollout_issue_url: 'https://gitlab.com/gitlab-org/growth/team-tasks/-/issues/300' -milestone: '13.10' -type: experiment -group: group::expansion -default_enabled: false diff --git a/lib/gitlab/process_management.rb b/lib/gitlab/process_management.rb index 2c2aaadb29a..604b6129648 100644 --- a/lib/gitlab/process_management.rb +++ b/lib/gitlab/process_management.rb @@ -70,11 +70,17 @@ module Gitlab end def self.process_alive?(pid) + return false if pid.nil? + # Signal 0 tests whether the process exists and we have access to send signals # but is otherwise a noop (doesn't actually send a signal to the process) signal(pid, 0) end + def self.process_died?(pid) + !process_alive?(pid) + end + def self.write_pid(path) File.open(path, 'w') do |handle| handle.write(Process.pid.to_s) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index a4518e914ec..323287837f3 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4619,9 +4619,6 @@ msgstr "" msgid "Are you sure you want to re-deploy this environment?" msgstr "" -msgid "Are you sure you want to regenerate the public key? You will have to update the public key on the remote server before mirroring will work again." -msgstr "" - msgid "Are you sure you want to reindex?" msgstr "" @@ -19317,9 +19314,6 @@ msgstr "" msgid "InviteMember|Add members to this project and start collaborating with your team." msgstr "" -msgid "InviteMember|Invite Member" -msgstr "" - msgid "InviteMember|Invite Members (optional)" msgstr "" @@ -28858,9 +28852,6 @@ msgstr "" msgid "Regenerate instance ID" msgstr "" -msgid "Regenerate key" -msgstr "" - msgid "Regenerate recovery codes" msgstr "" diff --git a/metrics_server/dependencies.rb b/metrics_server/dependencies.rb index ecfef502feb..80e68b95ed1 100644 --- a/metrics_server/dependencies.rb +++ b/metrics_server/dependencies.rb @@ -13,6 +13,7 @@ require 'rack' require_relative 'settings_overrides' require_relative '../lib/gitlab/daemon' +require_relative '../lib/gitlab/utils' require_relative '../lib/gitlab/utils/strong_memoize' require_relative '../lib/prometheus/cleanup_multiproc_dir_service' require_relative '../lib/gitlab/metrics/prometheus' diff --git a/metrics_server/metrics_server.rb b/metrics_server/metrics_server.rb index 09171d8220b..9dc3ba91536 100644 --- a/metrics_server/metrics_server.rb +++ b/metrics_server/metrics_server.rb @@ -1,16 +1,17 @@ # frozen_string_literal: true -require_relative '../config/bundler_setup' +require_relative '../config/boot' require_relative 'dependencies' class MetricsServer # rubocop:disable Gitlab/NamespacedClass class << self - def spawn(target, gitlab_config: nil) + def spawn(target, gitlab_config: nil, wipe_metrics_dir: false) cmd = "#{Rails.root}/bin/metrics-server" env = { 'METRICS_SERVER_TARGET' => target, - 'GITLAB_CONFIG' => gitlab_config + 'GITLAB_CONFIG' => gitlab_config, + 'WIPE_METRICS_DIR' => wipe_metrics_dir.to_s } Process.spawn(env, cmd, err: $stderr, out: $stdout).tap do |pid| @@ -19,9 +20,10 @@ class MetricsServer # rubocop:disable Gitlab/NamespacedClass end end - def initialize(target, metrics_dir) + def initialize(target, metrics_dir, wipe_metrics_dir) @target = target @metrics_dir = metrics_dir + @wipe_metrics_dir = wipe_metrics_dir end def start @@ -30,7 +32,7 @@ class MetricsServer # rubocop:disable Gitlab/NamespacedClass end FileUtils.mkdir_p(@metrics_dir, mode: 0700) - ::Prometheus::CleanupMultiprocDirService.new.execute + ::Prometheus::CleanupMultiprocDirService.new.execute if @wipe_metrics_dir settings = Settings.monitoring.sidekiq_exporter exporter_class = "Gitlab::Metrics::Exporter::#{@target.camelize}Exporter".constantize diff --git a/qa/qa/service/kubernetes_cluster.rb b/qa/qa/service/kubernetes_cluster.rb index ec53b9d8163..dafce4acc33 100644 --- a/qa/qa/service/kubernetes_cluster.rb +++ b/qa/qa/service/kubernetes_cluster.rb @@ -77,7 +77,7 @@ module QA install_ingress # need to wait since the ingress-nginx service has an initial delay set of 10 seconds - sleep 10 + sleep 12 ingress_ip = `kubectl get svc --all-namespaces --no-headers=true -l app.kubernetes.io/name=ingress-nginx -o custom-columns=:'status.loadBalancer.ingress[0].ip' | grep -v 'none'` QA::Runtime::Logger.debug "Has ingress address set to: #{ingress_ip}" ingress_ip diff --git a/sidekiq_cluster/cli.rb b/sidekiq_cluster/cli.rb index 2dbb1e9c7c7..274b7c03e14 100644 --- a/sidekiq_cluster/cli.rb +++ b/sidekiq_cluster/cli.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require_relative '../config/bundler_setup' + require 'optparse' require 'logger' require 'time' @@ -12,6 +14,7 @@ require_relative '../lib/gitlab/sidekiq_config/cli_methods' require_relative '../lib/gitlab/sidekiq_config/worker_matcher' require_relative '../lib/gitlab/sidekiq_logging/json_formatter' require_relative '../lib/gitlab/process_management' +require_relative '../metrics_server/metrics_server' require_relative 'sidekiq_cluster' module Gitlab @@ -89,6 +92,8 @@ module Gitlab @logger.info("Starting cluster with #{queue_groups.length} processes") end + start_metrics_server(wipe_metrics_dir: true) + @processes = SidekiqCluster.start( queue_groups, env: @environment, @@ -154,17 +159,69 @@ module Gitlab while @alive sleep(@interval) + if metrics_server_enabled? && ProcessManagement.process_died?(@metrics_server_pid) + @logger.warn('Metrics server went away') + start_metrics_server(wipe_metrics_dir: false) + end + unless ProcessManagement.all_alive?(@processes) # If a child process died we'll just terminate the whole cluster. It's up to # runit and such to then restart the cluster. @logger.info('A worker terminated, shutting down the cluster') + stop_metrics_server ProcessManagement.signal_processes(@processes, :TERM) break end end end + def start_metrics_server(wipe_metrics_dir: false) + return unless metrics_server_enabled? + + @logger.info("Starting metrics server on port #{sidekiq_exporter_port}") + @metrics_server_pid = MetricsServer.spawn('sidekiq', wipe_metrics_dir: wipe_metrics_dir) + end + + def sidekiq_exporter_enabled? + ::Settings.monitoring.sidekiq_exporter.enabled + rescue Settingslogic::MissingSetting + nil + end + + def exporter_has_a_unique_port? + # In https://gitlab.com/gitlab-org/gitlab/-/issues/345802 we added settings for sidekiq_health_checks. + # These settings default to the same values as sidekiq_exporter for backwards compatibility. + # If a different port for sidekiq_health_checks has been set up, we know that the + # user wants to serve health checks and metrics from different servers. + return false if sidekiq_health_check_port.nil? || sidekiq_exporter_port.nil? + + sidekiq_exporter_port != sidekiq_health_check_port + end + + def sidekiq_exporter_port + ::Settings.monitoring.sidekiq_exporter.port + rescue Settingslogic::MissingSetting + nil + end + + def sidekiq_health_check_port + ::Settings.monitoring.sidekiq_health_checks.port + rescue Settingslogic::MissingSetting + nil + end + + def metrics_server_enabled? + !@dryrun && sidekiq_exporter_enabled? && exporter_has_a_unique_port? + end + + def stop_metrics_server + return unless @metrics_server_pid + + @logger.info("Stopping metrics server (PID #{@metrics_server_pid})") + ProcessManagement.signal(@metrics_server_pid, :TERM) + end + def option_parser OptionParser.new do |opt| opt.banner = "#{File.basename(__FILE__)} [QUEUE,QUEUE] [QUEUE] ... [OPTIONS]" diff --git a/sidekiq_cluster/dependencies.rb b/sidekiq_cluster/dependencies.rb deleted file mode 100644 index 91e91475f15..00000000000 --- a/sidekiq_cluster/dependencies.rb +++ /dev/null @@ -1,6 +0,0 @@ -# rubocop:disable Naming/FileName -# frozen_string_literal: true - -require 'shellwords' - -# rubocop:enable Naming/FileName diff --git a/sidekiq_cluster/sidekiq_cluster.rb b/sidekiq_cluster/sidekiq_cluster.rb index c3aa9e05a09..c5139ab8874 100644 --- a/sidekiq_cluster/sidekiq_cluster.rb +++ b/sidekiq_cluster/sidekiq_cluster.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -require_relative 'dependencies' require_relative '../lib/gitlab/process_management' module Gitlab @@ -67,14 +66,19 @@ module Gitlab return end - pid = Process.spawn( - { 'ENABLE_SIDEKIQ_CLUSTER' => '1', - 'SIDEKIQ_WORKER_ID' => worker_id.to_s }, - *cmd, - pgroup: true, - err: $stderr, - out: $stdout - ) + # We need to remove Bundler specific env vars, since otherwise the + # child process will think we are passing an alternative Gemfile + # and will clear and reset LOAD_PATH. + pid = Bundler.with_original_env do + Process.spawn( + { 'ENABLE_SIDEKIQ_CLUSTER' => '1', + 'SIDEKIQ_WORKER_ID' => worker_id.to_s }, + *cmd, + pgroup: true, + err: $stderr, + out: $stdout + ) + end ProcessManagement.wait_async(pid) diff --git a/spec/commands/metrics_server/metrics_server_spec.rb b/spec/commands/metrics_server/metrics_server_spec.rb index 4eff9136f2b..7a03ff3fe75 100644 --- a/spec/commands/metrics_server/metrics_server_spec.rb +++ b/spec/commands/metrics_server/metrics_server_spec.rb @@ -29,7 +29,7 @@ RSpec.describe 'bin/metrics-server', :aggregate_failures do config_file.write(YAML.dump(config)) config_file.close - @pid = MetricsServer.spawn('sidekiq', gitlab_config: config_file.path) + @pid = MetricsServer.spawn('sidekiq', gitlab_config: config_file.path, wipe_metrics_dir: true) end after do diff --git a/spec/commands/sidekiq_cluster/cli_spec.rb b/spec/commands/sidekiq_cluster/cli_spec.rb index f5642c00ca4..c0576608088 100644 --- a/spec/commands/sidekiq_cluster/cli_spec.rb +++ b/spec/commands/sidekiq_cluster/cli_spec.rb @@ -12,8 +12,23 @@ RSpec.describe Gitlab::SidekiqCluster::CLI do # rubocop:disable RSpec/FilePath { env: 'test', directory: Dir.pwd, max_concurrency: 50, min_concurrency: 0, dryrun: false, timeout: timeout } end + let(:sidekiq_exporter_enabled) { false } + let(:sidekiq_exporter_port) { '3807' } + let(:sidekiq_health_checks_port) { '3807' } + before do stub_env('RAILS_ENV', 'test') + stub_config( + monitoring: { + sidekiq_exporter: { + enabled: sidekiq_exporter_enabled, + port: sidekiq_exporter_port + }, + sidekiq_health_checks: { + port: sidekiq_health_checks_port + } + } + ) end describe '#run' do @@ -241,6 +256,163 @@ RSpec.describe Gitlab::SidekiqCluster::CLI do # rubocop:disable RSpec/FilePath end end end + + context 'metrics server' do + context 'starting the server' do + context 'without --dryrun' do + context 'when there are no sidekiq_health_checks settings set' do + before do + stub_config( + monitoring: { + sidekiq_exporter: { + enabled: true, + port: sidekiq_exporter_port + } + } + ) + + allow(Gitlab::SidekiqCluster).to receive(:start) + allow(cli).to receive(:write_pid) + allow(cli).to receive(:trap_signals) + allow(cli).to receive(:start_loop) + end + + it 'does not start a sidekiq metrics server' do + expect(MetricsServer).not_to receive(:spawn) + + cli.run(%w(foo)) + end + + it 'rescues Settingslogic::MissingSetting' do + expect { cli.run(%w(foo)) }.not_to raise_error(Settingslogic::MissingSetting) + end + end + + context 'when the sidekiq_exporter.port setting is not set' do + before do + stub_config( + monitoring: { + sidekiq_exporter: { + enabled: true + }, + sidekiq_health_checks: { + port: sidekiq_health_checks_port + } + } + ) + + allow(Gitlab::SidekiqCluster).to receive(:start) + allow(cli).to receive(:write_pid) + allow(cli).to receive(:trap_signals) + allow(cli).to receive(:start_loop) + end + + it 'does not start a sidekiq metrics server' do + expect(MetricsServer).not_to receive(:spawn) + + cli.run(%w(foo)) + end + + it 'rescues Settingslogic::MissingSetting' do + expect { cli.run(%w(foo)) }.not_to raise_error(Settingslogic::MissingSetting) + end + end + + context 'when sidekiq_exporter.enabled setting is not set' do + before do + stub_config( + monitoring: { + sidekiq_exporter: {}, + sidekiq_health_checks: { + port: sidekiq_health_checks_port + } + } + ) + + allow(Gitlab::SidekiqCluster).to receive(:start) + allow(cli).to receive(:write_pid) + allow(cli).to receive(:trap_signals) + allow(cli).to receive(:start_loop) + end + + it 'does not start a sidekiq metrics server' do + expect(MetricsServer).not_to receive(:spawn) + + cli.run(%w(foo)) + end + end + + using RSpec::Parameterized::TableSyntax + + where(:sidekiq_exporter_enabled, :sidekiq_exporter_port, :sidekiq_health_checks_port, :start_metrics_server) do + true | '3807' | '3907' | true + true | '3807' | '3807' | false + false | '3807' | '3907' | false + false | '3807' | '3907' | false + end + + with_them do + before do + allow(Gitlab::SidekiqCluster).to receive(:start) + allow(cli).to receive(:write_pid) + allow(cli).to receive(:trap_signals) + allow(cli).to receive(:start_loop) + end + + specify do + if start_metrics_server + expect(MetricsServer).to receive(:spawn).with('sidekiq', wipe_metrics_dir: true) + else + expect(MetricsServer).not_to receive(:spawn) + end + + cli.run(%w(foo)) + end + end + end + + context 'with --dryrun set' do + let(:sidekiq_exporter_enabled) { true } + + it 'does not start the server' do + expect(MetricsServer).not_to receive(:spawn) + + cli.run(%w(foo --dryrun)) + end + end + end + + context 'supervising the server' do + let(:sidekiq_exporter_enabled) { true } + let(:sidekiq_health_checks_port) { '3907' } + + before do + allow(cli).to receive(:sleep).with(a_kind_of(Numeric)) + allow(MetricsServer).to receive(:spawn).with('sidekiq', wipe_metrics_dir: false).and_return(99) + cli.start_metrics_server + end + + it 'stops the metrics server when one of the processes has been terminated' do + allow(Gitlab::ProcessManagement).to receive(:process_died?).and_return(false) + allow(Gitlab::ProcessManagement).to receive(:all_alive?).with(an_instance_of(Array)).and_return(false) + allow(Gitlab::ProcessManagement).to receive(:signal_processes).with(an_instance_of(Array), :TERM) + + expect(Process).to receive(:kill).with(:TERM, 99) + + cli.start_loop + end + + it 'starts the metrics server when it is down' do + allow(Gitlab::ProcessManagement).to receive(:process_died?).and_return(true) + allow(Gitlab::ProcessManagement).to receive(:all_alive?).with(an_instance_of(Array)).and_return(false) + allow(cli).to receive(:stop_metrics_server) + + expect(MetricsServer).to receive(:spawn).with('sidekiq', wipe_metrics_dir: false) + + cli.start_loop + end + end + end end describe '#write_pid' do diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index ce8bbf92cdf..763c3e43e27 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -201,32 +201,6 @@ RSpec.describe Projects::IssuesController do expect(response).to have_gitlab_http_status(:ok) expect(json_response['issue_email_participants']).to contain_exactly({ "email" => participants[0].email }, { "email" => participants[1].email }) end - - context 'with the invite_members_in_comment experiment', :experiment do - context 'when user can invite' do - before do - stub_experiments(invite_members_in_comment: :invite_member_link) - project.add_maintainer(user) - end - - it 'assigns the candidate experience and tracks the event' do - expect(experiment(:invite_members_in_comment)).to track(:view, property: project.root_ancestor.id.to_s) - .for(:invite_member_link) - .with_context(namespace: project.root_ancestor) - .on_next_instance - - get :show, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } - end - end - - context 'when user can not invite' do - it 'does not track the event' do - expect(experiment(:invite_members_in_comment)).not_to track(:view) - - get :show, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } - end - end - end end describe 'GET #new' do diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index f4a5044c049..36b6df59ef5 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -42,32 +42,6 @@ RSpec.describe Projects::MergeRequestsController do get :show, params: params.merge(extra_params) end - context 'with the invite_members_in_comment experiment', :experiment do - context 'when user can invite' do - before do - stub_experiments(invite_members_in_comment: :invite_member_link) - project.add_maintainer(user) - end - - it 'assigns the candidate experience and tracks the event' do - expect(experiment(:invite_members_in_comment)).to track(:view, property: project.root_ancestor.id.to_s) - .for(:invite_member_link) - .with_context(namespace: project.root_ancestor) - .on_next_instance - - go - end - end - - context 'when user can not invite' do - it 'does not track the event' do - expect(experiment(:invite_members_in_comment)).not_to track(:view) - - go - end - end - end - context 'with view param' do before do go(view: 'parallel') diff --git a/spec/features/issues/user_invites_from_a_comment_spec.rb b/spec/features/issues/user_invites_from_a_comment_spec.rb deleted file mode 100644 index 82061f6ed79..00000000000 --- a/spec/features/issues/user_invites_from_a_comment_spec.rb +++ /dev/null @@ -1,25 +0,0 @@ -# frozen_string_literal: true - -require "spec_helper" - -RSpec.describe "User invites from a comment", :js do - let_it_be(:project) { create(:project_empty_repo, :public) } - let_it_be(:issue) { create(:issue, project: project) } - let_it_be(:user) { project.owner } - - before do - sign_in(user) - end - - it "launches the invite modal from invite link on a comment" do - stub_experiments(invite_members_in_comment: :invite_member_link) - - visit project_issue_path(project, issue) - - page.within(".new-note") do - click_button 'Invite Member' - end - - expect(page).to have_content("You're inviting members to the") - end -end diff --git a/spec/features/merge_request/user_invites_from_a_comment_spec.rb b/spec/features/merge_request/user_invites_from_a_comment_spec.rb deleted file mode 100644 index 79865094fd0..00000000000 --- a/spec/features/merge_request/user_invites_from_a_comment_spec.rb +++ /dev/null @@ -1,25 +0,0 @@ -# frozen_string_literal: true - -require "spec_helper" - -RSpec.describe "User invites from a comment", :js do - let_it_be(:project) { create(:project, :public, :repository) } - let_it_be(:merge_request) { create(:merge_request, source_project: project) } - let_it_be(:user) { project.owner } - - before do - sign_in(user) - end - - it "launches the invite modal from invite link on a comment" do - stub_experiments(invite_members_in_comment: :invite_member_link) - - visit project_merge_request_path(project, merge_request) - - page.within(".new-note") do - click_button 'Invite Member' - end - - expect(page).to have_content("You're inviting members to the") - end -end diff --git a/spec/frontend/environments/enable_review_app_modal_spec.js b/spec/frontend/environments/enable_review_app_modal_spec.js index 9a3f13f19d5..17ae10a2884 100644 --- a/spec/frontend/environments/enable_review_app_modal_spec.js +++ b/spec/frontend/environments/enable_review_app_modal_spec.js @@ -1,10 +1,12 @@ import { shallowMount } from '@vue/test-utils'; +import { GlModal } from '@gitlab/ui'; import { extendedWrapper } from 'helpers/vue_test_utils_helper'; import EnableReviewAppButton from '~/environments/components/enable_review_app_modal.vue'; import ModalCopyButton from '~/vue_shared/components/modal_copy_button.vue'; describe('Enable Review App Button', () => { let wrapper; + let modal; afterEach(() => { wrapper.destroy(); @@ -16,12 +18,15 @@ describe('Enable Review App Button', () => { shallowMount(EnableReviewAppButton, { propsData: { modalId: 'fake-id', + visible: true, }, provide: { defaultBranchName: 'main', }, }), ); + + modal = wrapper.findComponent(GlModal); }); it('renders the defaultBranchName copy', () => { @@ -32,5 +37,15 @@ describe('Enable Review App Button', () => { it('renders the copyToClipboard button', () => { expect(wrapper.findComponent(ModalCopyButton).exists()).toBe(true); }); + + it('emits change events from the modal up', () => { + modal.vm.$emit('change', false); + + expect(wrapper.emitted('change')).toEqual([[false]]); + }); + + it('passes visible to the modal', () => { + expect(modal.props('visible')).toBe(true); + }); }); }); diff --git a/spec/frontend/environments/new_environments_app_spec.js b/spec/frontend/environments/new_environments_app_spec.js index 0ad8e8f442c..c4613bfe182 100644 --- a/spec/frontend/environments/new_environments_app_spec.js +++ b/spec/frontend/environments/new_environments_app_spec.js @@ -1,8 +1,9 @@ import Vue from 'vue'; import VueApollo from 'vue-apollo'; -import { mount } from '@vue/test-utils'; +import { mountExtended } from 'helpers/vue_test_utils_helper'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; +import { s__ } from '~/locale'; import EnvironmentsApp from '~/environments/components/new_environments_app.vue'; import EnvironmentsFolder from '~/environments/components/new_environment_folder.vue'; import { resolvedEnvironmentsApp, resolvedFolder } from './graphql/mock_data'; @@ -22,7 +23,16 @@ describe('~/environments/components/new_environments_app.vue', () => { return createMockApollo([], mockResolvers); }; - const createWrapper = (apolloProvider) => mount(EnvironmentsApp, { apolloProvider }); + const createWrapper = ({ provide = {}, apolloProvider } = {}) => + mountExtended(EnvironmentsApp, { + provide: { + newEnvironmentPath: '/environments/new', + canCreateEnvironment: true, + defaultBranchName: 'main', + ...provide, + }, + apolloProvider, + }); beforeEach(() => { environmentAppMock = jest.fn(); @@ -37,7 +47,7 @@ describe('~/environments/components/new_environments_app.vue', () => { environmentAppMock.mockReturnValue(resolvedEnvironmentsApp); environmentFolderMock.mockReturnValue(resolvedFolder); const apolloProvider = createApolloProvider(); - wrapper = createWrapper(apolloProvider); + wrapper = createWrapper({ apolloProvider }); await waitForPromises(); await Vue.nextTick(); @@ -47,4 +57,66 @@ describe('~/environments/components/new_environments_app.vue', () => { expect(text).toContainEqual(expect.stringMatching('review')); expect(text).not.toContainEqual(expect.stringMatching('production')); }); + + it('should show a button to create a new environment', async () => { + environmentAppMock.mockReturnValue(resolvedEnvironmentsApp); + environmentFolderMock.mockReturnValue(resolvedFolder); + const apolloProvider = createApolloProvider(); + wrapper = createWrapper({ apolloProvider }); + + await waitForPromises(); + await Vue.nextTick(); + + const button = wrapper.findByRole('link', { name: s__('Environments|New environment') }); + expect(button.attributes('href')).toBe('/environments/new'); + }); + + it('should not show a button to create a new environment if the user has no permissions', async () => { + environmentAppMock.mockReturnValue(resolvedEnvironmentsApp); + environmentFolderMock.mockReturnValue(resolvedFolder); + const apolloProvider = createApolloProvider(); + wrapper = createWrapper({ + apolloProvider, + provide: { canCreateEnvironment: false, newEnvironmentPath: '' }, + }); + + await waitForPromises(); + await Vue.nextTick(); + + const button = wrapper.findByRole('link', { name: s__('Environments|New environment') }); + expect(button.exists()).toBe(false); + }); + + it('should show a button to open the review app modal', async () => { + environmentAppMock.mockReturnValue(resolvedEnvironmentsApp); + environmentFolderMock.mockReturnValue(resolvedFolder); + const apolloProvider = createApolloProvider(); + wrapper = createWrapper({ apolloProvider }); + + await waitForPromises(); + await Vue.nextTick(); + + const button = wrapper.findByRole('button', { name: s__('Environments|Enable review app') }); + button.trigger('click'); + + await Vue.nextTick(); + + expect(wrapper.findByText(s__('ReviewApp|Enable Review App')).exists()).toBe(true); + }); + + it('should not show a button to open the review app modal if review apps are configured', async () => { + environmentAppMock.mockReturnValue({ + ...resolvedEnvironmentsApp, + reviewApp: { canSetupReviewApp: false }, + }); + environmentFolderMock.mockReturnValue(resolvedFolder); + const apolloProvider = createApolloProvider(); + wrapper = createWrapper({ apolloProvider }); + + await waitForPromises(); + await Vue.nextTick(); + + const button = wrapper.findByRole('button', { name: s__('Environments|Enable review app') }); + expect(button.exists()).toBe(false); + }); }); diff --git a/spec/frontend/invite_members/components/invite_members_modal_spec.js b/spec/frontend/invite_members/components/invite_members_modal_spec.js index f3cad3fc066..e190ddf243e 100644 --- a/spec/frontend/invite_members/components/invite_members_modal_spec.js +++ b/spec/frontend/invite_members/components/invite_members_modal_spec.js @@ -17,7 +17,6 @@ import InviteMembersModal from '~/invite_members/components/invite_members_modal import ModalConfetti from '~/invite_members/components/confetti.vue'; import MembersTokenSelect from '~/invite_members/components/members_token_select.vue'; import { - INVITE_MEMBERS_IN_COMMENT, INVITE_MEMBERS_FOR_TASK, CANCEL_BUTTON_TEXT, INVITE_BUTTON_TEXT, @@ -746,7 +745,6 @@ describe('InviteMembersModal', () => { wrapper.vm.$toast = { show: jest.fn() }; jest.spyOn(Api, 'inviteGroupMembersByEmail').mockResolvedValue({ data: postData }); jest.spyOn(Api, 'addGroupMembersByUserId').mockResolvedValue({ data: postData }); - jest.spyOn(wrapper.vm, 'trackInvite'); }); describe('when triggered from regular mounting', () => { @@ -864,31 +862,6 @@ describe('InviteMembersModal', () => { jest.spyOn(Api, 'inviteGroupMembersByEmail').mockResolvedValue({}); }); - it('tracks the invite', () => { - eventHub.$emit('openModal', { inviteeType: 'members', source: INVITE_MEMBERS_IN_COMMENT }); - - clickInviteButton(); - - expect(ExperimentTracking).toHaveBeenCalledWith(INVITE_MEMBERS_IN_COMMENT); - expect(ExperimentTracking.prototype.event).toHaveBeenCalledWith('comment_invite_success'); - }); - - it('does not track invite for unknown source', () => { - eventHub.$emit('openModal', { inviteeType: 'members', source: 'unknown' }); - - clickInviteButton(); - - expect(ExperimentTracking).not.toHaveBeenCalledWith(INVITE_MEMBERS_IN_COMMENT); - }); - - it('does not track invite undefined source', () => { - eventHub.$emit('openModal', { inviteeType: 'members' }); - - clickInviteButton(); - - expect(ExperimentTracking).not.toHaveBeenCalledWith(INVITE_MEMBERS_IN_COMMENT); - }); - it('tracks the view for learn_gitlab source', () => { eventHub.$emit('openModal', { inviteeType: 'members', source: LEARN_GITLAB }); diff --git a/spec/frontend/invite_members/components/invite_members_trigger_spec.js b/spec/frontend/invite_members/components/invite_members_trigger_spec.js index 3fce23f854c..429b6fad24a 100644 --- a/spec/frontend/invite_members/components/invite_members_trigger_spec.js +++ b/spec/frontend/invite_members/components/invite_members_trigger_spec.js @@ -1,6 +1,5 @@ import { GlButton, GlLink, GlIcon } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; -import ExperimentTracking from '~/experimentation/experiment_tracking'; import InviteMembersTrigger from '~/invite_members/components/invite_members_trigger.vue'; import eventHub from '~/invite_members/event_hub'; import { TRIGGER_ELEMENT_BUTTON, TRIGGER_ELEMENT_SIDE_NAV } from '~/invite_members/constants'; @@ -79,19 +78,6 @@ describe.each(triggerItems)('with triggerElement as %s', (triggerItem) => { }); describe('tracking', () => { - it('tracks on mounting', () => { - createComponent({ trackExperiment: '_track_experiment_' }); - - expect(ExperimentTracking).toHaveBeenCalledWith('_track_experiment_'); - expect(ExperimentTracking.prototype.event).toHaveBeenCalledWith('comment_invite_shown'); - }); - - it('does not track on mounting', () => { - createComponent(); - - expect(ExperimentTracking).not.toHaveBeenCalledWith('_track_experiment_'); - }); - it('does not add tracking attributes', () => { createComponent(); diff --git a/spec/frontend/repository/commits_service_spec.js b/spec/frontend/repository/commits_service_spec.js index d924974aede..697fa7c4fd1 100644 --- a/spec/frontend/repository/commits_service_spec.js +++ b/spec/frontend/repository/commits_service_spec.js @@ -52,13 +52,6 @@ describe('commits service', () => { expect(axios.get.mock.calls.length).toEqual(1); }); - it('calls axios get twice if an offset is larger than 25', async () => { - await requestCommits(100); - - expect(axios.get.mock.calls[0][1]).toEqual({ params: { format: 'json', offset: 75 } }); - expect(axios.get.mock.calls[1][1]).toEqual({ params: { format: 'json', offset: 100 } }); - }); - it('updates the list of requested offsets', async () => { await requestCommits(200); diff --git a/spec/frontend/repository/components/table/row_spec.js b/spec/frontend/repository/components/table/row_spec.js index 76e9f7da011..7f59dbfe0d1 100644 --- a/spec/frontend/repository/components/table/row_spec.js +++ b/spec/frontend/repository/components/table/row_spec.js @@ -4,6 +4,7 @@ import { createMockDirective, getBinding } from 'helpers/vue_mock_directive'; import TableRow from '~/repository/components/table/row.vue'; import FileIcon from '~/vue_shared/components/file_icon.vue'; import { FILE_SYMLINK_MODE } from '~/vue_shared/constants'; +import { ROW_APPEAR_DELAY } from '~/repository/constants'; const COMMIT_MOCK = { lockLabel: 'Locked by Root', committedDate: '2019-01-01' }; @@ -17,12 +18,12 @@ function factory(propsData = {}) { vm = shallowMount(TableRow, { propsData: { + commitInfo: COMMIT_MOCK, ...propsData, name: propsData.path, projectPath: 'gitlab-org/gitlab-ce', url: `https://test.com`, totalEntries: 10, - commitInfo: COMMIT_MOCK, rowNumber: 123, }, directives: { @@ -251,6 +252,8 @@ describe('Repository table row component', () => { }); describe('row visibility', () => { + beforeAll(() => jest.useFakeTimers()); + beforeEach(() => { factory({ id: '1', @@ -258,18 +261,20 @@ describe('Repository table row component', () => { path: 'test', type: 'tree', currentPath: '/', + commitInfo: null, }); }); - it('emits a `row-appear` event', () => { + + afterAll(() => jest.useRealTimers()); + + it('emits a `row-appear` event', async () => { findIntersectionObserver().vm.$emit('appear'); - expect(vm.emitted('row-appear')).toEqual([ - [ - { - hasCommit: true, - rowNumber: 123, - }, - ], - ]); + + jest.runAllTimers(); + + expect(setTimeout).toHaveBeenCalledTimes(1); + expect(setTimeout).toHaveBeenLastCalledWith(expect.any(Function), ROW_APPEAR_DELAY); + expect(vm.emitted('row-appear')).toEqual([[123]]); }); }); }); diff --git a/spec/frontend/repository/components/tree_content_spec.js b/spec/frontend/repository/components/tree_content_spec.js index bc12577fa8e..9c5d07eede3 100644 --- a/spec/frontend/repository/components/tree_content_spec.js +++ b/spec/frontend/repository/components/tree_content_spec.js @@ -190,14 +190,28 @@ describe('Repository table component', () => { }); }); - it('loads commit data when row-appear event is emitted', () => { + describe('commit data', () => { const path = 'some/path'; - const rowNumber = 1; - factory(path); - findFileTable().vm.$emit('row-appear', { hasCommit: false, rowNumber }); + it('loads commit data for both top and bottom batches when row-appear event is emitted', () => { + const rowNumber = 50; - expect(isRequested).toHaveBeenCalledWith(rowNumber); - expect(loadCommits).toHaveBeenCalledWith('', path, '', rowNumber); + factory(path); + findFileTable().vm.$emit('row-appear', rowNumber); + + expect(isRequested).toHaveBeenCalledWith(rowNumber); + + expect(loadCommits.mock.calls).toEqual([ + ['', path, '', rowNumber], + ['', path, '', rowNumber - 25], + ]); + }); + + it('loads commit data once if rowNumber is zero', () => { + factory(path); + findFileTable().vm.$emit('row-appear', 0); + + expect(loadCommits.mock.calls).toEqual([['', path, '', 0]]); + }); }); }); diff --git a/spec/frontend/vue_shared/components/markdown/toolbar_spec.js b/spec/frontend/vue_shared/components/markdown/toolbar_spec.js index eddc4033a65..8bff85b0bda 100644 --- a/spec/frontend/vue_shared/components/markdown/toolbar_spec.js +++ b/spec/frontend/vue_shared/components/markdown/toolbar_spec.js @@ -1,24 +1,17 @@ import { mount } from '@vue/test-utils'; -import { isExperimentVariant } from '~/experimentation/utils'; -import InviteMembersTrigger from '~/invite_members/components/invite_members_trigger.vue'; -import { INVITE_MEMBERS_IN_COMMENT } from '~/invite_members/constants'; import Toolbar from '~/vue_shared/components/markdown/toolbar.vue'; -jest.mock('~/experimentation/utils', () => ({ isExperimentVariant: jest.fn() })); - describe('toolbar', () => { let wrapper; const createMountedWrapper = (props = {}) => { wrapper = mount(Toolbar, { propsData: { markdownDocsPath: '', ...props }, - stubs: { 'invite-members-trigger': true }, }); }; afterEach(() => { wrapper.destroy(); - isExperimentVariant.mockReset(); }); describe('user can attach file', () => { @@ -40,36 +33,4 @@ describe('toolbar', () => { expect(wrapper.vm.$el.querySelector('.uploading-container')).toBeNull(); }); }); - - describe('user can invite member', () => { - const findInviteLink = () => wrapper.find(InviteMembersTrigger); - - beforeEach(() => { - isExperimentVariant.mockReturnValue(true); - createMountedWrapper(); - }); - - it('should render the invite members trigger', () => { - expect(findInviteLink().exists()).toBe(true); - }); - - it('should have correct props', () => { - expect(findInviteLink().props().displayText).toBe('Invite Member'); - expect(findInviteLink().props().trackExperiment).toBe(INVITE_MEMBERS_IN_COMMENT); - expect(findInviteLink().props().triggerSource).toBe(INVITE_MEMBERS_IN_COMMENT); - }); - }); - - describe('user can not invite member', () => { - const findInviteLink = () => wrapper.find(InviteMembersTrigger); - - beforeEach(() => { - isExperimentVariant.mockReturnValue(false); - createMountedWrapper(); - }); - - it('should render the invite members trigger', () => { - expect(findInviteLink().exists()).toBe(false); - }); - }); }); diff --git a/spec/lib/gitlab/process_management_spec.rb b/spec/lib/gitlab/process_management_spec.rb index 71c4acdecb9..4e367e258f2 100644 --- a/spec/lib/gitlab/process_management_spec.rb +++ b/spec/lib/gitlab/process_management_spec.rb @@ -78,7 +78,7 @@ RSpec.describe Gitlab::ProcessManagement do end describe '.process_alive?' do - it 'returns true if the proces is alive' do + it 'returns true if the process is alive' do process = Process.pid expect(described_class.process_alive?(process)).to eq(true) @@ -89,6 +89,32 @@ RSpec.describe Gitlab::ProcessManagement do expect(described_class.process_alive?(process)).to eq(false) end + + it 'returns false when no pid is given' do + process = nil + + expect(described_class.process_alive?(process)).to eq(false) + end + end + + describe '.process_died?' do + it 'returns false if the process is alive' do + process = Process.pid + + expect(described_class.process_died?(process)).to eq(false) + end + + it 'returns true when a thread was not alive' do + process = -2 + + expect(described_class.process_died?(process)).to eq(true) + end + + it 'returns true when no pid is given' do + process = nil + + expect(described_class.process_died?(process)).to eq(true) + end end describe '.pids_alive' do diff --git a/spec/metrics_server/metrics_server_spec.rb b/spec/metrics_server/metrics_server_spec.rb index 58556d2cf5a..fcd5b911d2a 100644 --- a/spec/metrics_server/metrics_server_spec.rb +++ b/spec/metrics_server/metrics_server_spec.rb @@ -12,7 +12,8 @@ RSpec.describe MetricsServer do # rubocop:disable RSpec/FilePath let(:env) do { 'METRICS_SERVER_TARGET' => 'sidekiq', - 'GITLAB_CONFIG' => nil + 'GITLAB_CONFIG' => nil, + 'WIPE_METRICS_DIR' => 'false' } end @@ -32,7 +33,7 @@ RSpec.describe MetricsServer do # rubocop:disable RSpec/FilePath let(:metrics_dir) { Dir.mktmpdir } let(:settings_double) { double(:settings, sidekiq_exporter: {}) } - subject(:metrics_server) { described_class.new('fake', metrics_dir)} + subject(:metrics_server) { described_class.new('fake', metrics_dir, true)} before do stub_env('prometheus_multiproc_dir', metrics_dir) @@ -42,6 +43,7 @@ RSpec.describe MetricsServer do # rubocop:disable RSpec/FilePath end after do + ::Prometheus::CleanupMultiprocDirService.new.execute Dir.rmdir(metrics_dir) end @@ -59,10 +61,24 @@ RSpec.describe MetricsServer do # rubocop:disable RSpec/FilePath metrics_server.start end - it 'removes any old metrics files' do - FileUtils.touch("#{metrics_dir}/remove_this.db") + context 'when wipe_metrics_dir is true' do + subject(:metrics_server) { described_class.new('fake', metrics_dir, true)} - expect { metrics_server.start }.to change { Dir.empty?(metrics_dir) }.from(false).to(true) + it 'removes any old metrics files' do + FileUtils.touch("#{metrics_dir}/remove_this.db") + + expect { metrics_server.start }.to change { Dir.empty?(metrics_dir) }.from(false).to(true) + end + end + + context 'when wipe_metrics_dir is false' do + subject(:metrics_server) { described_class.new('fake', metrics_dir, false)} + + it 'does not remove any old metrics files' do + FileUtils.touch("#{metrics_dir}/remove_this.db") + + expect { metrics_server.start }.not_to change { Dir.empty?(metrics_dir) }.from(false) + end end it 'starts a metrics server' do diff --git a/spec/sidekiq_cluster/sidekiq_cluster_spec.rb b/spec/sidekiq_cluster/sidekiq_cluster_spec.rb index 86b3fc6a48c..c0a919a4aec 100644 --- a/spec/sidekiq_cluster/sidekiq_cluster_spec.rb +++ b/spec/sidekiq_cluster/sidekiq_cluster_spec.rb @@ -13,6 +13,8 @@ RSpec.describe Gitlab::SidekiqCluster do # rubocop:disable RSpec/FilePath out: $stdout } + expect(Bundler).to receive(:with_original_env).and_call_original.twice + expect(Process).to receive(:spawn).ordered.with({ "ENABLE_SIDEKIQ_CLUSTER" => "1", "SIDEKIQ_WORKER_ID" => "0" diff --git a/spec/tooling/quality/test_level_spec.rb b/spec/tooling/quality/test_level_spec.rb index 3348e495732..8a944a473d7 100644 --- a/spec/tooling/quality/test_level_spec.rb +++ b/spec/tooling/quality/test_level_spec.rb @@ -28,7 +28,7 @@ RSpec.describe Quality::TestLevel do context 'when level is unit' do it 'returns a pattern' do expect(subject.pattern(:unit)) - .to eq("spec/{bin,channels,config,db,dependencies,elastic,elastic_integration,experiments,factories,finders,frontend,graphql,haml_lint,helpers,initializers,javascripts,lib,metrics_server,models,policies,presenters,rack_servers,replicators,routing,rubocop,scripts,serializers,services,sidekiq,spam,support_specs,tasks,uploaders,validators,views,workers,tooling}{,/**/}*_spec.rb") + .to eq("spec/{bin,channels,config,db,dependencies,elastic,elastic_integration,experiments,factories,finders,frontend,graphql,haml_lint,helpers,initializers,javascripts,lib,metrics_server,models,policies,presenters,rack_servers,replicators,routing,rubocop,scripts,serializers,services,sidekiq,sidekiq_cluster,spam,support_specs,tasks,uploaders,validators,views,workers,tooling}{,/**/}*_spec.rb") end end @@ -110,7 +110,7 @@ RSpec.describe Quality::TestLevel do context 'when level is unit' do it 'returns a regexp' do expect(subject.regexp(:unit)) - .to eq(%r{spec/(bin|channels|config|db|dependencies|elastic|elastic_integration|experiments|factories|finders|frontend|graphql|haml_lint|helpers|initializers|javascripts|lib|metrics_server|models|policies|presenters|rack_servers|replicators|routing|rubocop|scripts|serializers|services|sidekiq|spam|support_specs|tasks|uploaders|validators|views|workers|tooling)}) + .to eq(%r{spec/(bin|channels|config|db|dependencies|elastic|elastic_integration|experiments|factories|finders|frontend|graphql|haml_lint|helpers|initializers|javascripts|lib|metrics_server|models|policies|presenters|rack_servers|replicators|routing|rubocop|scripts|serializers|services|sidekiq|sidekiq_cluster|spam|support_specs|tasks|uploaders|validators|views|workers|tooling)}) end end diff --git a/tooling/quality/test_level.rb b/tooling/quality/test_level.rb index 3cc1d87eb36..50cbc69beb2 100644 --- a/tooling/quality/test_level.rb +++ b/tooling/quality/test_level.rb @@ -45,6 +45,7 @@ module Quality serializers services sidekiq + sidekiq_cluster spam support_specs tasks |