diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-02-28 18:09:07 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-02-28 18:09:07 +0000 |
| commit | 1c8fa70f9d0818e2a82089c8643a6e455bca47fd (patch) | |
| tree | f339f97de0425270bdd909e2f4d378927b6e0a18 | |
| parent | 736d36d8597d0d1ec1b47644e6d091c3f4a78f45 (diff) | |
| download | gitlab-ce-1c8fa70f9d0818e2a82089c8643a6e455bca47fd.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
77 files changed, 1204 insertions, 497 deletions
diff --git a/.haml-lint_todo.yml b/.haml-lint_todo.yml index 900dae7a3ab..3f0d3d8ddf8 100644 --- a/.haml-lint_todo.yml +++ b/.haml-lint_todo.yml @@ -374,6 +374,7 @@ linters: - 'app/views/shared/boards/components/sidebar/_due_date.html.haml' - 'app/views/shared/boards/components/sidebar/_labels.html.haml' - 'app/views/shared/boards/components/sidebar/_milestone.html.haml' + - 'app/views/shared/deploy_tokens/_revoke_modal.html.haml' - 'app/views/shared/empty_states/_priority_labels.html.haml' - 'app/views/shared/hook_logs/_content.html.haml' - 'app/views/shared/issuable/_assignees.html.haml' diff --git a/app/assets/javascripts/pages/groups/settings/ci_cd/show/index.js b/app/assets/javascripts/pages/groups/settings/ci_cd/show/index.js index 479c82265f2..1ef18b356f2 100644 --- a/app/assets/javascripts/pages/groups/settings/ci_cd/show/index.js +++ b/app/assets/javascripts/pages/groups/settings/ci_cd/show/index.js @@ -1,10 +1,13 @@ import initSettingsPanels from '~/settings_panels'; import AjaxVariableList from '~/ci_variable_list/ajax_variable_list'; import initVariableList from '~/ci_variable_list'; +import DueDateSelectors from '~/due_date_select'; document.addEventListener('DOMContentLoaded', () => { // Initialize expandable settings panels initSettingsPanels(); + // eslint-disable-next-line no-new + new DueDateSelectors(); if (gon.features.newVariablesUi) { initVariableList(); diff --git a/app/assets/javascripts/pages/projects/settings/ci_cd/show/index.js b/app/assets/javascripts/pages/projects/settings/ci_cd/show/index.js index e08d0407245..c83e2bdbf38 100644 --- a/app/assets/javascripts/pages/projects/settings/ci_cd/show/index.js +++ b/app/assets/javascripts/pages/projects/settings/ci_cd/show/index.js @@ -3,6 +3,7 @@ import SecretValues from '~/behaviors/secret_values'; import AjaxVariableList from '~/ci_variable_list/ajax_variable_list'; import registrySettingsApp from '~/registry/settings/registry_settings_bundle'; import initVariableList from '~/ci_variable_list'; +import DueDateSelectors from '~/due_date_select'; document.addEventListener('DOMContentLoaded', () => { // Initialize expandable settings panels @@ -39,5 +40,8 @@ document.addEventListener('DOMContentLoaded', () => { autoDevOpsExtraSettings.classList.toggle('hidden', !target.checked); }); + // eslint-disable-next-line no-new + new DueDateSelectors(); + registrySettingsApp(); }); diff --git a/app/assets/javascripts/registry/explorer/pages/details.vue b/app/assets/javascripts/registry/explorer/pages/details.vue index bfb9b0f4688..0f4ed1550ce 100644 --- a/app/assets/javascripts/registry/explorer/pages/details.vue +++ b/app/assets/javascripts/registry/explorer/pages/details.vue @@ -1,5 +1,5 @@ <script> -import { mapState, mapActions } from 'vuex'; +import { mapState, mapActions, mapGetters } from 'vuex'; import { GlTable, GlFormCheckbox, @@ -8,10 +8,10 @@ import { GlTooltipDirective, GlPagination, GlModal, - GlLoadingIcon, GlSprintf, GlEmptyState, GlResizeObserverDirective, + GlSkeletonLoader, } from '@gitlab/ui'; import { GlBreakpointInstance } from '@gitlab/ui/dist/utils'; import { n__, s__ } from '~/locale'; @@ -42,7 +42,7 @@ export default { ClipboardButton, GlPagination, GlModal, - GlLoadingIcon, + GlSkeletonLoader, GlSprintf, GlEmptyState, }, @@ -51,6 +51,11 @@ export default { GlResizeObserver: GlResizeObserverDirective, }, mixins: [timeagoMixin, Tracking.mixin()], + loader: { + repeat: 10, + width: 1000, + height: 40, + }, data() { return { selectedItems: [], @@ -61,15 +66,16 @@ export default { }; }, computed: { - ...mapState(['tags', 'tagsPagination', 'isLoading', 'config']), + ...mapGetters(['tags']), + ...mapState(['tagsPagination', 'isLoading', 'config']), imageName() { const { name } = decodeAndParse(this.$route.params.id); return name; }, fields() { return [ - { key: LIST_KEY_CHECKBOX, label: '' }, - { key: LIST_KEY_TAG, label: LIST_LABEL_TAG }, + { key: LIST_KEY_CHECKBOX, label: '', class: 'gl-w-16' }, + { key: LIST_KEY_TAG, label: LIST_LABEL_TAG, class: 'w-25' }, { key: LIST_KEY_IMAGE_ID, label: LIST_LABEL_IMAGE_ID }, { key: LIST_KEY_SIZE, label: LIST_LABEL_SIZE }, { key: LIST_KEY_LAST_UPDATED, label: LIST_LABEL_LAST_UPDATED }, @@ -209,122 +215,142 @@ export default { </gl-sprintf> </h4> </div> - <gl-loading-icon v-if="isLoading" /> - <template v-else-if="tags.length > 0"> - <gl-table :items="tags" :fields="fields" :stacked="!isDesktop"> - <template v-if="isDesktop" #head(checkbox)> - <gl-form-checkbox - ref="mainCheckbox" - :checked="selectAllChecked" - @change="onSelectAllChange" - /> - </template> - <template #head(actions)> - <gl-button - ref="bulkDeleteButton" - v-gl-tooltip - :disabled="!selectedItems || selectedItems.length === 0" - class="float-right" - variant="danger" - :title="s__('ContainerRegistry|Remove selected tags')" - :aria-label="s__('ContainerRegistry|Remove selected tags')" - @click="deleteMultipleItems()" - > - <gl-icon name="remove" /> - </gl-button> - </template> - <template #cell(checkbox)="{index}"> - <gl-form-checkbox - ref="rowCheckbox" - class="js-row-checkbox" - :checked="selectedItems.includes(index)" - @change="updateSelectedItems(index)" - /> - </template> - <template #cell(name)="{item}"> - <span ref="rowName"> - {{ item.name }} - </span> - <clipboard-button - v-if="item.location" - ref="rowClipboardButton" - :title="item.location" - :text="item.location" - css-class="btn-default btn-transparent btn-clipboard" - /> - </template> - <template #cell(short_revision)="{value}"> - <span ref="rowShortRevision"> - {{ value }} - </span> - </template> - <template #cell(total_size)="{item}"> - <span ref="rowSize"> - {{ formatSize(item.total_size) }} - <template v-if="item.total_size && item.layers"> - · - </template> - {{ layers(item.layers) }} - </span> - </template> - <template #cell(created_at)="{value}"> - <span ref="rowTime"> - {{ timeFormatted(value) }} - </span> - </template> - <template #cell(actions)="{index, item}"> - <gl-button - ref="singleDeleteButton" - :title="s__('ContainerRegistry|Remove tag')" - :aria-label="s__('ContainerRegistry|Remove tag')" - :disabled="!item.destroy_path" - variant="danger" - :class="['js-delete-registry float-right btn-inverted btn-border-color btn-icon']" - @click="deleteSingleItem(index)" + <gl-table :items="tags" :fields="fields" :stacked="!isDesktop" show-empty> + <template v-if="isDesktop" #head(checkbox)> + <gl-form-checkbox + ref="mainCheckbox" + :checked="selectAllChecked" + @change="onSelectAllChange" + /> + </template> + <template #head(actions)> + <gl-button + ref="bulkDeleteButton" + v-gl-tooltip + :disabled="!selectedItems || selectedItems.length === 0" + class="float-right" + variant="danger" + :title="s__('ContainerRegistry|Remove selected tags')" + :aria-label="s__('ContainerRegistry|Remove selected tags')" + @click="deleteMultipleItems()" + > + <gl-icon name="remove" /> + </gl-button> + </template> + + <template #cell(checkbox)="{index}"> + <gl-form-checkbox + ref="rowCheckbox" + class="js-row-checkbox" + :checked="selectedItems.includes(index)" + @change="updateSelectedItems(index)" + /> + </template> + <template #cell(name)="{item}"> + <span ref="rowName"> + {{ item.name }} + </span> + <clipboard-button + v-if="item.location" + ref="rowClipboardButton" + :title="item.location" + :text="item.location" + css-class="btn-default btn-transparent btn-clipboard" + /> + </template> + <template #cell(short_revision)="{value}"> + <span ref="rowShortRevision"> + {{ value }} + </span> + </template> + <template #cell(total_size)="{item}"> + <span ref="rowSize"> + {{ formatSize(item.total_size) }} + <template v-if="item.total_size && item.layers"> + · + </template> + {{ layers(item.layers) }} + </span> + </template> + <template #cell(created_at)="{value}"> + <span ref="rowTime"> + {{ timeFormatted(value) }} + </span> + </template> + <template #cell(actions)="{index, item}"> + <gl-button + ref="singleDeleteButton" + :title="s__('ContainerRegistry|Remove tag')" + :aria-label="s__('ContainerRegistry|Remove tag')" + :disabled="!item.destroy_path" + variant="danger" + class="js-delete-registry float-right btn-inverted btn-border-color btn-icon" + @click="deleteSingleItem(index)" + > + <gl-icon name="remove" /> + </gl-button> + </template> + + <template #empty> + <template v-if="isLoading"> + <gl-skeleton-loader + v-for="index in $options.loader.repeat" + :key="index" + :width="$options.loader.width" + :height="$options.loader.height" + preserve-aspect-ratio="xMinYMax meet" > - <gl-icon name="remove" /> - </gl-button> + <rect width="15" x="0" y="12.5" height="15" rx="4" /> + <rect width="250" x="25" y="10" height="20" rx="4" /> + <circle cx="290" cy="20" r="10" /> + <rect width="100" x="315" y="10" height="20" rx="4" /> + <rect width="100" x="500" y="10" height="20" rx="4" /> + <rect width="100" x="630" y="10" height="20" rx="4" /> + <rect x="960" y="0" width="40" height="40" rx="4" /> + </gl-skeleton-loader> </template> - </gl-table> - <gl-pagination - ref="pagination" - v-model="currentPage" - :per-page="tagsPagination.perPage" - :total-items="tagsPagination.total" - align="center" - class="w-100" - /> - <gl-modal - ref="deleteModal" - modal-id="delete-tag-modal" - ok-variant="danger" - @ok="onDeletionConfirmed" - @cancel="track('cancel_delete')" - > - <template #modal-title>{{ modalAction }}</template> - <template #modal-ok>{{ modalAction }}</template> - <p v-if="modalDescription"> - <gl-sprintf :message="modalDescription.message"> - <template #item> - <b>{{ modalDescription.item }}</b> - </template> - </gl-sprintf> - </p> - </gl-modal> - </template> - <gl-empty-state - v-else - :title="s__('ContainerRegistry|This image has no active tags')" - :svg-path="config.noContainersImage" - :description=" - s__( - `ContainerRegistry|The last tag related to this image was recently removed. + <gl-empty-state + v-else + :title="s__('ContainerRegistry|This image has no active tags')" + :svg-path="config.noContainersImage" + :description=" + s__( + `ContainerRegistry|The last tag related to this image was recently removed. This empty image and any associated data will be automatically removed as part of the regular Garbage Collection process. If you have any questions, contact your administrator.`, - ) - " - class="mx-auto my-0" + ) + " + class="mx-auto my-0" + /> + </template> + </gl-table> + + <gl-pagination + ref="pagination" + v-model="currentPage" + :per-page="tagsPagination.perPage" + :total-items="tagsPagination.total" + align="center" + class="w-100" /> + + <gl-modal + ref="deleteModal" + modal-id="delete-tag-modal" + ok-variant="danger" + @ok="onDeletionConfirmed" + @cancel="track('cancel_delete')" + > + <template #modal-title>{{ modalAction }}</template> + <template #modal-ok>{{ modalAction }}</template> + <p v-if="modalDescription"> + <gl-sprintf :message="modalDescription.message"> + <template #item> + <b>{{ modalDescription.item }}</b> + </template> + </gl-sprintf> + </p> + </gl-modal> </div> </template> diff --git a/app/assets/javascripts/registry/explorer/pages/list.vue b/app/assets/javascripts/registry/explorer/pages/list.vue index 5f8f4d8df1e..4e9f0a83501 100644 --- a/app/assets/javascripts/registry/explorer/pages/list.vue +++ b/app/assets/javascripts/registry/explorer/pages/list.vue @@ -1,7 +1,6 @@ <script> import { mapState, mapActions } from 'vuex'; import { - GlLoadingIcon, GlEmptyState, GlPagination, GlTooltipDirective, @@ -10,6 +9,7 @@ import { GlModal, GlSprintf, GlLink, + GlSkeletonLoader, } from '@gitlab/ui'; import Tracking from '~/tracking'; import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; @@ -20,7 +20,6 @@ export default { name: 'RegistryListApp', components: { GlEmptyState, - GlLoadingIcon, GlPagination, ProjectEmptyState, GroupEmptyState, @@ -30,11 +29,17 @@ export default { GlModal, GlSprintf, GlLink, + GlSkeletonLoader, }, directives: { GlTooltip: GlTooltipDirective, }, mixins: [Tracking.mixin()], + loader: { + repeat: 10, + width: 1000, + height: 40, + }, data() { return { itemToDelete: {}, @@ -104,74 +109,81 @@ export default { </gl-empty-state> <template v-else> - <gl-loading-icon v-if="isLoading" size="md" class="prepend-top-16" /> - - <template v-else> - <div v-if="images.length" ref="imagesList"> - <h4>{{ s__('ContainerRegistry|Container Registry') }}</h4> - <p> - <gl-sprintf - :message=" - s__(`ContainerRegistry|With the Docker Container Registry integrated into GitLab, every + <div> + <h4>{{ s__('ContainerRegistry|Container Registry') }}</h4> + <p> + <gl-sprintf + :message=" + s__(`ContainerRegistry|With the Docker Container Registry integrated into GitLab, every project can have its own space to store its Docker images. %{docLinkStart}More Information%{docLinkEnd}`) - " - > - <template #docLink="{content}"> - <gl-link :href="config.helpPagePath" target="_blank"> - {{ content }} - </gl-link> - </template> - </gl-sprintf> - </p> + " + > + <template #docLink="{content}"> + <gl-link :href="config.helpPagePath" target="_blank"> + {{ content }} + </gl-link> + </template> + </gl-sprintf> + </p> + </div> - <div class="d-flex flex-column"> + <div v-if="isLoading" class="mt-2"> + <gl-skeleton-loader + v-for="index in $options.loader.repeat" + :key="index" + :width="$options.loader.width" + :height="$options.loader.height" + preserve-aspect-ratio="xMinYMax meet" + > + <rect width="500" x="10" y="10" height="20" rx="4" /> + <circle cx="525" cy="20" r="10" /> + <rect x="960" y="0" width="40" height="40" rx="4" /> + </gl-skeleton-loader> + </div> + <template v-else> + <div v-if="images.length" ref="imagesList" class="d-flex flex-column"> + <div + v-for="(listItem, index) in images" + :key="index" + ref="rowItem" + :class="{ 'border-top': index === 0 }" + class="d-flex justify-content-between align-items-center py-2 border-bottom" + > + <div> + <router-link + ref="detailsLink" + :to="{ name: 'details', params: { id: encodeListItem(listItem) } }" + > + {{ listItem.path }} + </router-link> + <clipboard-button + v-if="listItem.location" + ref="clipboardButton" + :text="listItem.location" + :title="listItem.location" + css-class="btn-default btn-transparent btn-clipboard" + /> + </div> <div - v-for="(listItem, index) in images" - :key="index" - ref="rowItem" - :class="[ - 'd-flex justify-content-between align-items-center py-2 border-bottom', - { 'border-top': index === 0 }, - ]" + v-gl-tooltip="{ disabled: listItem.destroy_path }" + class="d-none d-sm-block" + :title=" + s__('ContainerRegistry|Missing or insufficient permission, delete button disabled') + " > - <div> - <router-link - ref="detailsLink" - :to="{ name: 'details', params: { id: encodeListItem(listItem) } }" - > - {{ listItem.path }} - </router-link> - <clipboard-button - v-if="listItem.location" - ref="clipboardButton" - :text="listItem.location" - :title="listItem.location" - css-class="btn-default btn-transparent btn-clipboard" - /> - </div> - <div - v-gl-tooltip="{ disabled: listItem.destroy_path }" - class="d-none d-sm-block" - :title=" - s__( - 'ContainerRegistry|Missing or insufficient permission, delete button disabled', - ) - " + <gl-button + ref="deleteImageButton" + v-gl-tooltip + :disabled="!listItem.destroy_path" + :title="s__('ContainerRegistry|Remove repository')" + :aria-label="s__('ContainerRegistry|Remove repository')" + class="btn-inverted" + variant="danger" + @click="deleteImage(listItem)" > - <gl-button - ref="deleteImageButton" - v-gl-tooltip - :disabled="!listItem.destroy_path" - :title="s__('ContainerRegistry|Remove repository')" - :aria-label="s__('ContainerRegistry|Remove repository')" - class="btn-inverted" - variant="danger" - @click="deleteImage(listItem)" - > - <gl-icon name="remove" /> - </gl-button> - </div> + <gl-icon name="remove" /> + </gl-button> </div> </div> <gl-pagination @@ -182,6 +194,7 @@ export default { class="w-100 mt-2" /> </div> + <template v-else> <project-empty-state v-if="!config.isGroupPage" /> <group-empty-state v-else /> diff --git a/app/assets/javascripts/registry/explorer/stores/actions.js b/app/assets/javascripts/registry/explorer/stores/actions.js index 86d00d4fca9..7d8201949f4 100644 --- a/app/assets/javascripts/registry/explorer/stores/actions.js +++ b/app/assets/javascripts/registry/explorer/stores/actions.js @@ -68,31 +68,28 @@ export const requestDeleteTag = ({ commit, dispatch, state }, { tag, params }) = .delete(tag.destroy_path) .then(() => { createFlash(DELETE_TAG_SUCCESS_MESSAGE, 'success'); - dispatch('requestTagsList', { pagination: state.tagsPagination, params }); + return dispatch('requestTagsList', { pagination: state.tagsPagination, params }); }) .catch(() => { createFlash(DELETE_TAG_ERROR_MESSAGE); - }) - .finally(() => { commit(types.SET_MAIN_LOADING, false); }); }; export const requestDeleteTags = ({ commit, dispatch, state }, { ids, params }) => { commit(types.SET_MAIN_LOADING, true); - const { id } = decodeAndParse(params); - const url = `/${state.config.projectPath}/registry/repository/${id}/tags/bulk_destroy`; + const { tags_path } = decodeAndParse(params); + + const url = tags_path.replace('?format=json', '/bulk_destroy'); return axios .delete(url, { params: { ids } }) .then(() => { createFlash(DELETE_TAGS_SUCCESS_MESSAGE, 'success'); - dispatch('requestTagsList', { pagination: state.tagsPagination, params }); + return dispatch('requestTagsList', { pagination: state.tagsPagination, params }); }) .catch(() => { createFlash(DELETE_TAGS_ERROR_MESSAGE); - }) - .finally(() => { commit(types.SET_MAIN_LOADING, false); }); }; diff --git a/app/assets/javascripts/registry/explorer/stores/getters.js b/app/assets/javascripts/registry/explorer/stores/getters.js new file mode 100644 index 00000000000..5619b73d495 --- /dev/null +++ b/app/assets/javascripts/registry/explorer/stores/getters.js @@ -0,0 +1,6 @@ +// eslint-disable-next-line import/prefer-default-export +export const tags = state => { + // to show the loader inside the table we need to pass an empty array to gl-table whenever the table is loading + // this is to take in account isLoading = true and state.tags =[1,2,3] during pagination and delete + return state.isLoading ? [] : state.tags; +}; diff --git a/app/assets/javascripts/registry/explorer/stores/index.js b/app/assets/javascripts/registry/explorer/stores/index.js index 91a35aac149..b3ff2e6e002 100644 --- a/app/assets/javascripts/registry/explorer/stores/index.js +++ b/app/assets/javascripts/registry/explorer/stores/index.js @@ -1,6 +1,7 @@ import Vue from 'vue'; import Vuex from 'vuex'; import * as actions from './actions'; +import * as getters from './getters'; import mutations from './mutations'; import state from './state'; @@ -9,6 +10,7 @@ Vue.use(Vuex); export const createStore = () => new Vuex.Store({ state, + getters, actions, mutations, }); diff --git a/app/assets/stylesheets/utilities.scss b/app/assets/stylesheets/utilities.scss index dabbcf0eac1..2a92b271ed0 100644 --- a/app/assets/stylesheets/utilities.scss +++ b/app/assets/stylesheets/utilities.scss @@ -54,7 +54,7 @@ .mh-50vh { max-height: 50vh; } .font-size-inherit { font-size: inherit; } - +.gl-w-16 { width: px-to-rem($grid-size * 2); } .gl-w-64 { width: px-to-rem($grid-size * 8); } .gl-h-32 { height: px-to-rem($grid-size * 4); } .gl-h-64 { height: px-to-rem($grid-size * 8); } diff --git a/app/controllers/groups/deploy_tokens_controller.rb b/app/controllers/groups/deploy_tokens_controller.rb new file mode 100644 index 00000000000..a765922fc54 --- /dev/null +++ b/app/controllers/groups/deploy_tokens_controller.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class Groups::DeployTokensController < Groups::ApplicationController + before_action :authorize_admin_group! + + def revoke + @token = @group.deploy_tokens.find(params[:id]) + @token.revoke! + + redirect_to group_settings_ci_cd_path(@group, anchor: 'js-deploy-tokens') + end +end diff --git a/app/controllers/groups/settings/ci_cd_controller.rb b/app/controllers/groups/settings/ci_cd_controller.rb index 3c1f020702f..ffa3f2c3364 100644 --- a/app/controllers/groups/settings/ci_cd_controller.rb +++ b/app/controllers/groups/settings/ci_cd_controller.rb @@ -7,11 +7,11 @@ module Groups before_action :authorize_admin_group! before_action :authorize_update_max_artifacts_size!, only: [:update] before_action do - push_frontend_feature_flag(:new_variables_ui, @group, default_enabled: true) + push_frontend_feature_flag(:new_variables_ui, @group) end + before_action :define_variables, only: [:show, :create_deploy_token] def show - define_ci_variables end def update @@ -41,8 +41,23 @@ module Groups redirect_to group_settings_ci_cd_path end + def create_deploy_token + @new_deploy_token = Groups::DeployTokens::CreateService.new(@group, current_user, deploy_token_params).execute + + if @new_deploy_token.persisted? + flash.now[:notice] = s_('DeployTokens|Your new group deploy token has been created.') + end + + render 'show' + end + private + def define_variables + define_ci_variables + define_deploy_token_variables + end + def define_ci_variables @variable = Ci::GroupVariable.new(group: group) .present(current_user: current_user) @@ -50,6 +65,12 @@ module Groups .map { |variable| variable.present(current_user: current_user) } end + def define_deploy_token_variables + @deploy_tokens = @group.deploy_tokens.active + + @new_deploy_token = DeployToken.new + end + def authorize_admin_group! return render_404 unless can?(current_user, :admin_group, group) end @@ -73,6 +94,10 @@ module Groups def update_group_params params.require(:group).permit(:max_artifacts_size) end + + def deploy_token_params + params.require(:deploy_token).permit(:name, :expires_at, :read_repository, :read_registry, :username) + end end end end diff --git a/app/controllers/projects/deploy_tokens_controller.rb b/app/controllers/projects/deploy_tokens_controller.rb index 830b1f4fe4a..4a70424ec01 100644 --- a/app/controllers/projects/deploy_tokens_controller.rb +++ b/app/controllers/projects/deploy_tokens_controller.rb @@ -7,6 +7,6 @@ class Projects::DeployTokensController < Projects::ApplicationController @token = @project.deploy_tokens.find(params[:id]) @token.revoke! - redirect_to project_settings_repository_path(project, anchor: 'js-deploy-tokens') + redirect_to project_settings_ci_cd_path(project, anchor: 'js-deploy-tokens') end end diff --git a/app/controllers/projects/settings/ci_cd_controller.rb b/app/controllers/projects/settings/ci_cd_controller.rb index 5097b6b8c8c..ed42fb55223 100644 --- a/app/controllers/projects/settings/ci_cd_controller.rb +++ b/app/controllers/projects/settings/ci_cd_controller.rb @@ -6,7 +6,7 @@ module Projects before_action :authorize_admin_pipeline! before_action :define_variables before_action do - push_frontend_feature_flag(:new_variables_ui, @project, default_enabled: true) + push_frontend_feature_flag(:new_variables_ui, @project) end def show @@ -46,6 +46,16 @@ module Projects redirect_to namespace_project_settings_ci_cd_path end + def create_deploy_token + @new_deploy_token = Projects::DeployTokens::CreateService.new(@project, current_user, deploy_token_params).execute + + if @new_deploy_token.persisted? + flash.now[:notice] = s_('DeployTokens|Your new project deploy token has been created.') + end + + render 'show' + end + private def update_params @@ -64,6 +74,10 @@ module Projects end end + def deploy_token_params + params.require(:deploy_token).permit(:name, :expires_at, :read_repository, :read_registry, :username) + end + def run_autodevops_pipeline(service) return unless service.run_auto_devops_pipeline? @@ -83,6 +97,7 @@ module Projects def define_variables define_runners_variables define_ci_variables + define_deploy_token_variables define_triggers_variables define_badges_variables define_auto_devops_variables @@ -132,6 +147,12 @@ module Projects def define_auto_devops_variables @auto_devops = @project.auto_devops || ProjectAutoDevops.new end + + def define_deploy_token_variables + @deploy_tokens = @project.deploy_tokens.active + + @new_deploy_token = DeployToken.new + end end end end diff --git a/app/controllers/projects/settings/repository_controller.rb b/app/controllers/projects/settings/repository_controller.rb index 63f5d5073a7..28db3024dc4 100644 --- a/app/controllers/projects/settings/repository_controller.rb +++ b/app/controllers/projects/settings/repository_controller.rb @@ -10,16 +10,6 @@ module Projects render_show end - def create_deploy_token - @new_deploy_token = DeployTokens::CreateService.new(@project, current_user, deploy_token_params).execute - - if @new_deploy_token.persisted? - flash.now[:notice] = s_('DeployTokens|Your new project deploy token has been created.') - end - - render_show - end - def cleanup cleanup_params = params.require(:project).permit(:bfg_object_map) result = Projects::UpdateService.new(project, current_user, cleanup_params).execute @@ -38,9 +28,7 @@ module Projects def render_show @deploy_keys = DeployKeysPresenter.new(@project, current_user: current_user) - @deploy_tokens = @project.deploy_tokens.active - define_deploy_token define_protected_refs remote_mirror @@ -93,14 +81,6 @@ module Projects gon.push(protectable_branches_for_dropdown) gon.push(access_levels_options) end - - def define_deploy_token - @new_deploy_token ||= DeployToken.new - end - - def deploy_token_params - params.require(:deploy_token).permit(:name, :expires_at, :read_repository, :read_registry, :username) - end end end end diff --git a/app/helpers/ci_variables_helper.rb b/app/helpers/ci_variables_helper.rb index fc51f00d052..3f4c04070b5 100644 --- a/app/helpers/ci_variables_helper.rb +++ b/app/helpers/ci_variables_helper.rb @@ -5,6 +5,22 @@ module CiVariablesHelper Gitlab::CurrentSettings.current_application_settings.protected_ci_variables end + def create_deploy_token_path(entity, opts = {}) + if entity.is_a?(Group) + create_deploy_token_group_settings_ci_cd_path(entity, opts) + else + create_deploy_token_project_settings_repository_path(entity, opts) + end + end + + def revoke_deploy_token_path(entity, token) + if entity.is_a?(Group) + revoke_group_deploy_token_path(entity, token) + else + revoke_project_deploy_token_path(entity, token) + end + end + def ci_variable_protected?(variable, only_key_value) if variable && !only_key_value variable.protected diff --git a/app/models/group.rb b/app/models/group.rb index ea5d46e23f4..a5337f19b38 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -59,6 +59,9 @@ class Group < Namespace has_many :import_failures, inverse_of: :group + has_many :group_deploy_tokens + has_many :deploy_tokens, through: :group_deploy_tokens + accepts_nested_attributes_for :variables, allow_destroy: true validate :visibility_level_allowed_by_projects diff --git a/app/models/group_deploy_token.rb b/app/models/group_deploy_token.rb index 221a7d768ae..d4ad29ddabb 100644 --- a/app/models/group_deploy_token.rb +++ b/app/models/group_deploy_token.rb @@ -9,7 +9,7 @@ class GroupDeployToken < ApplicationRecord validates :deploy_token_id, uniqueness: { scope: [:group_id] } def has_access_to?(requested_project) - return false unless Feature.enabled?(:allow_group_deploy_token, default: true) + return false unless Feature.enabled?(:allow_group_deploy_token, default_enabled: true) requested_project_group = requested_project&.group return false unless requested_project_group diff --git a/app/models/project.rb b/app/models/project.rb index 41c56fe6931..5ec43de21fe 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -2343,6 +2343,14 @@ class Project < ApplicationRecord Gitlab::CurrentSettings.self_monitoring_project_id == id end + def deploy_token_create_url(opts = {}) + Gitlab::Routing.url_helpers.create_deploy_token_project_settings_ci_cd_path(self, opts) + end + + def deploy_token_revoke_url_for(token) + Gitlab::Routing.url_helpers.revoke_project_deploy_token_path(self, token) + end + private def closest_namespace_setting(name) diff --git a/app/models/snippet_repository.rb b/app/models/snippet_repository.rb index ba2a061a5f4..10580c51098 100644 --- a/app/models/snippet_repository.rb +++ b/app/models/snippet_repository.rb @@ -3,11 +3,76 @@ class SnippetRepository < ApplicationRecord include Shardable + DEFAULT_EMPTY_FILE_NAME = 'snippetfile' + EMPTY_FILE_PATTERN = /^#{DEFAULT_EMPTY_FILE_NAME}(\d)\.txt$/.freeze + + CommitError = Class.new(StandardError) + belongs_to :snippet, inverse_of: :snippet_repository + delegate :repository, to: :snippet + class << self def find_snippet(disk_path) find_by(disk_path: disk_path)&.snippet end end + + def multi_files_action(user, files = [], **options) + return if files.nil? || files.empty? + + lease_key = "multi_files_action:#{snippet_id}" + + lease = Gitlab::ExclusiveLease.new(lease_key, timeout: 120) + raise CommitError, 'Snippet is already being updated' unless uuid = lease.try_obtain + + options[:actions] = transform_file_entries(files) + + capture_git_error { repository.multi_action(user, **options) } + ensure + Gitlab::ExclusiveLease.cancel(lease_key, uuid) + end + + private + + def capture_git_error(&block) + yield block + rescue Gitlab::Git::Index::IndexError, + Gitlab::Git::CommitError, + Gitlab::Git::PreReceiveError, + Gitlab::Git::CommandError => e + raise CommitError, e.message + end + + def transform_file_entries(files) + last_index = get_last_empty_file_index + + files.each do |file_entry| + file_entry[:action] = infer_action(file_entry) unless file_entry[:action] + + if file_entry[:file_path].blank? + file_entry[:file_path] = build_empty_file_name(last_index) + last_index += 1 + end + end + end + + def infer_action(file_entry) + return :create if file_entry[:previous_path].blank? + + file_entry[:previous_path] != file_entry[:file_path] ? :move : :update + end + + def get_last_empty_file_index + last_file = repository.ls_files(nil) + .map! { |file| file.match(EMPTY_FILE_PATTERN) } + .compact + .max_by { |element| element[1] } + + last_file ? (last_file[1].to_i + 1) : 1 + end + + def build_empty_file_name(index) + "#{DEFAULT_EMPTY_FILE_NAME}#{index}.txt" + end end diff --git a/app/services/concerns/deploy_token_methods.rb b/app/services/concerns/deploy_token_methods.rb new file mode 100644 index 00000000000..c0208b16623 --- /dev/null +++ b/app/services/concerns/deploy_token_methods.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module DeployTokenMethods + def create_deploy_token_for(entity, params) + params[:deploy_token_type] = DeployToken.deploy_token_types["#{entity.class.name.downcase}_type".to_sym] + + entity.deploy_tokens.create(params) do |deploy_token| + deploy_token.username = params[:username].presence + end + end +end diff --git a/app/services/deploy_tokens/create_service.rb b/app/services/deploy_tokens/create_service.rb deleted file mode 100644 index 327a1dbf408..00000000000 --- a/app/services/deploy_tokens/create_service.rb +++ /dev/null @@ -1,11 +0,0 @@ -# frozen_string_literal: true - -module DeployTokens - class CreateService < BaseService - def execute - @project.deploy_tokens.create(params) do |deploy_token| - deploy_token.username = params[:username].presence - end - end - end -end diff --git a/app/services/groups/deploy_tokens/create_service.rb b/app/services/groups/deploy_tokens/create_service.rb new file mode 100644 index 00000000000..8c42b56ebb0 --- /dev/null +++ b/app/services/groups/deploy_tokens/create_service.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Groups + module DeployTokens + class CreateService < BaseService + include DeployTokenMethods + + def execute + create_deploy_token_for(@group, params) + end + end + end +end diff --git a/app/services/projects/deploy_tokens/create_service.rb b/app/services/projects/deploy_tokens/create_service.rb new file mode 100644 index 00000000000..51cb68dfb10 --- /dev/null +++ b/app/services/projects/deploy_tokens/create_service.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Projects + module DeployTokens + class CreateService < BaseService + include DeployTokenMethods + + def execute + create_deploy_token_for(@project, params) + end + end + end +end diff --git a/app/services/snippets/create_service.rb b/app/services/snippets/create_service.rb index 7ded185a6f9..cc645c514b7 100644 --- a/app/services/snippets/create_service.rb +++ b/app/services/snippets/create_service.rb @@ -4,6 +4,8 @@ module Snippets class CreateService < Snippets::BaseService include SpamCheckMethods + CreateRepositoryError = Class.new(StandardError) + def execute filter_spam_check_params @@ -23,13 +25,7 @@ module Snippets spam_check(snippet, current_user) - snippet_saved = snippet.with_transaction_returning_status do - (snippet.save && snippet.store_mentions!).tap do |saved| - create_repository_for(snippet, current_user) if saved - end - end - - if snippet_saved + if save_and_commit(snippet) UserAgentDetailService.new(snippet, @request).create Gitlab::UsageDataCounters::SnippetCounter.count(:create) @@ -41,8 +37,45 @@ module Snippets private - def create_repository_for(snippet, user) - snippet.create_repository if Feature.enabled?(:version_snippets, user) + def save_and_commit(snippet) + snippet.with_transaction_returning_status do + (snippet.save && snippet.store_mentions!).tap do |saved| + break false unless saved + + if Feature.enabled?(:version_snippets, current_user) + create_repository_for(snippet) + create_commit(snippet) + end + end + rescue => e # Rescuing all because we can receive Creation exceptions, GRPC exceptions, Git exceptions, ... + snippet.errors.add(:base, e.message) + + # If the commit action failed we need to remove the repository if exists + if snippet.repository_exists? + Repositories::DestroyService.new(snippet.repository).execute + end + + false + end + end + + def create_repository_for(snippet) + snippet.create_repository + + raise CreateRepositoryError, 'Repository could not be created' unless snippet.repository_exists? + end + + def create_commit(snippet) + commit_attrs = { + branch_name: 'master', + message: 'Initial commit' + } + + snippet.snippet_repository.multi_files_action(current_user, snippet_files, commit_attrs) + end + + def snippet_files + [{ file_path: params[:file_name], content: params[:content] }] end end end diff --git a/app/views/ci/variables/_index.html.haml b/app/views/ci/variables/_index.html.haml index aadb2c62d83..f11c730eba6 100644 --- a/app/views/ci/variables/_index.html.haml +++ b/app/views/ci/variables/_index.html.haml @@ -5,7 +5,7 @@ - link_start = '<a href="%{url}">'.html_safe % { url: help_page_path('ci/variables/README', anchor: 'protected-variables') } = s_('Environment variables are configured by your administrator to be %{link_start}protected%{link_end} by default').html_safe % { link_start: link_start, link_end: '</a>'.html_safe } -- if Feature.enabled?(:new_variables_ui, @project || @group, default_enabled: true) +- if Feature.enabled?(:new_variables_ui, @project || @group) - is_group = !@group.nil? #js-ci-project-variables{ data: { endpoint: save_endpoint, project_id: @project&.id || '', group: is_group.to_s, maskable_regex: ci_variable_maskable_regex} } diff --git a/app/views/groups/settings/ci_cd/show.html.haml b/app/views/groups/settings/ci_cd/show.html.haml index 8c9b859e127..4aef30622cd 100644 --- a/app/views/groups/settings/ci_cd/show.html.haml +++ b/app/views/groups/settings/ci_cd/show.html.haml @@ -3,6 +3,7 @@ - expanded = expanded_by_default? - general_expanded = @group.errors.empty? ? expanded : true +- deploy_token_description = s_('DeployTokens|Group deploy tokens allow read-only access to the repositories and registry images within the group.') -# Given we only have one field in this form which is also admin-only, -# we don't want to show an empty section to non-admin users, @@ -24,6 +25,8 @@ .settings-content = render 'ci/variables/index', save_endpoint: group_variables_path += render "shared/deploy_tokens/index", group_or_project: @group, description: deploy_token_description + %section.settings#runners-settings.no-animate{ class: ('expanded' if expanded) } .settings-header %h4 diff --git a/app/views/import/shared/_new_project_form.html.haml b/app/views/import/shared/_new_project_form.html.haml index 35059229a55..a558b21b461 100644 --- a/app/views/import/shared/_new_project_form.html.haml +++ b/app/views/import/shared/_new_project_form.html.haml @@ -15,7 +15,7 @@ .input-group-prepend.static-namespace.has-tooltip{ title: user_url(current_user.username) + '/' } .input-group-text.border-0 #{user_url(current_user.username)}/ - = hidden_field_tag :namespace_id, value: current_user.namespace_id + = hidden_field_tag :namespace_id, current_user.namespace_id .form-group.col-12.col-sm-6.project-path = label_tag :path, _('Project slug'), class: 'label-bold' = text_field_tag :path, @path, placeholder: "my-awesome-project", class: "js-path-name form-control", tabindex: 2, required: true diff --git a/app/views/projects/deploy_tokens/_index.html.haml b/app/views/projects/deploy_tokens/_index.html.haml deleted file mode 100644 index 4619522cfaf..00000000000 --- a/app/views/projects/deploy_tokens/_index.html.haml +++ /dev/null @@ -1,17 +0,0 @@ -- expanded = expand_deploy_tokens_section?(@new_deploy_token) - -%section.qa-deploy-tokens-settings.settings.no-animate#js-deploy-tokens{ class: ('expanded' if expanded) } - .settings-header - %h4= s_('DeployTokens|Deploy Tokens') - %button.btn.js-settings-toggle.qa-expand-deploy-keys{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' - %p - = s_('DeployTokens|Deploy tokens allow read-only access to your repository and registry images.') - .settings-content - - if @new_deploy_token.persisted? - = render 'projects/deploy_tokens/new_deploy_token', deploy_token: @new_deploy_token - %h5.prepend-top-0 - = s_('DeployTokens|Add a deploy token') - = render 'projects/deploy_tokens/form', project: @project, token: @new_deploy_token, presenter: @deploy_tokens - %hr - = render 'projects/deploy_tokens/table', project: @project, active_tokens: @deploy_tokens diff --git a/app/views/projects/registry/repositories/index.html.haml b/app/views/projects/registry/repositories/index.html.haml index 247cf021cc7..62b744b5095 100644 --- a/app/views/projects/registry/repositories/index.html.haml +++ b/app/views/projects/registry/repositories/index.html.haml @@ -6,7 +6,6 @@ .col-12 - if Feature.enabled?(:vue_container_registry_explorer, @project.group) #js-container-registry{ data: { endpoint: project_container_registry_index_path(@project), - project_path: @project.full_path, "help_page_path" => help_page_path('user/packages/container_registry/index'), "two_factor_auth_help_link" => help_page_path('user/profile/account/two_factor_authentication'), "personal_access_tokens_help_link" => help_page_path('user/profile/personal_access_tokens'), diff --git a/app/views/projects/settings/ci_cd/show.html.haml b/app/views/projects/settings/ci_cd/show.html.haml index 1358077f2b2..9e47e380266 100644 --- a/app/views/projects/settings/ci_cd/show.html.haml +++ b/app/views/projects/settings/ci_cd/show.html.haml @@ -4,6 +4,7 @@ - expanded = expanded_by_default? - general_expanded = @project.errors.empty? ? expanded : true +- deploy_token_description = s_('DeployTokens|Deploy tokens allow read-only access to your repository and registry images.') %section.settings#js-general-pipeline-settings.no-animate{ class: ('expanded' if general_expanded) } .settings-header @@ -51,6 +52,8 @@ .settings-content = render 'ci/variables/index', save_endpoint: project_variables_path(@project) += render "shared/deploy_tokens/index", group_or_project: @project, description: deploy_token_description + %section.settings.no-animate#js-pipeline-triggers{ class: ('expanded' if expanded) } .settings-header %h4 diff --git a/app/views/projects/settings/repository/show.html.haml b/app/views/projects/settings/repository/show.html.haml index ff30cc4f6db..3d1eb85da0d 100644 --- a/app/views/projects/settings/repository/show.html.haml +++ b/app/views/projects/settings/repository/show.html.haml @@ -13,7 +13,6 @@ = render "projects/settings/repository/protected_branches" = render @deploy_keys -= render "projects/deploy_tokens/index" = render "projects/cleanup/show" = render_if_exists 'shared/promotions/promote_repository_features' diff --git a/app/views/projects/deploy_tokens/_form.html.haml b/app/views/shared/deploy_tokens/_form.html.haml index f846dbd3763..99e259ba944 100644 --- a/app/views/projects/deploy_tokens/_form.html.haml +++ b/app/views/shared/deploy_tokens/_form.html.haml @@ -1,7 +1,7 @@ %p.profile-settings-content = s_("DeployTokens|Pick a name for the application, and we'll give you a unique deploy token.") -= form_for token, url: create_deploy_token_namespace_project_settings_repository_path(project.namespace, project, anchor: 'js-deploy-tokens'), method: :post do |f| += form_for token, url: create_deploy_token_path(group_or_project, anchor: 'js-deploy-tokens'), method: :post do |f| = form_errors(token) .form-group @@ -24,7 +24,7 @@ = label_tag ("deploy_token_read_repository"), 'read_repository', class: 'label-bold form-check-label' .text-secondary= s_('DeployTokens|Allows read-only access to the repository') - - if container_registry_enabled?(project) + - if container_registry_enabled?(group_or_project) %fieldset.form-group.form-check = f.check_box :read_registry, class: 'form-check-input qa-deploy-token-read-registry' = label_tag ("deploy_token_read_registry"), 'read_registry', class: 'label-bold form-check-label' diff --git a/app/views/shared/deploy_tokens/_index.html.haml b/app/views/shared/deploy_tokens/_index.html.haml new file mode 100644 index 00000000000..b0c9c72dfaa --- /dev/null +++ b/app/views/shared/deploy_tokens/_index.html.haml @@ -0,0 +1,18 @@ +- expanded = expand_deploy_tokens_section?(@new_deploy_token) + +%section.qa-deploy-tokens-settings.settings.no-animate#js-deploy-tokens{ class: ('expanded' if expanded), data: { qa_selector: 'deploy_tokens_settings' } } + .settings-header + %h4= s_('DeployTokens|Deploy Tokens') + %button.btn.js-settings-toggle.qa-expand-deploy-keys{ type: 'button' } + = expanded ? 'Collapse' : 'Expand' + %p + = description + .settings-content + - if @new_deploy_token.persisted? + = render 'shared/deploy_tokens/new_deploy_token', deploy_token: @new_deploy_token + %h5.prepend-top-0 + = s_('DeployTokens|Add a deploy token') + = render 'shared/deploy_tokens/form', group_or_project: group_or_project, token: @new_deploy_token, presenter: @deploy_tokens + %hr + = render 'shared/deploy_tokens/table', group_or_project: group_or_project, active_tokens: @deploy_tokens + diff --git a/app/views/projects/deploy_tokens/_new_deploy_token.html.haml b/app/views/shared/deploy_tokens/_new_deploy_token.html.haml index f295fa82192..f295fa82192 100644 --- a/app/views/projects/deploy_tokens/_new_deploy_token.html.haml +++ b/app/views/shared/deploy_tokens/_new_deploy_token.html.haml diff --git a/app/views/projects/deploy_tokens/_revoke_modal.html.haml b/app/views/shared/deploy_tokens/_revoke_modal.html.haml index 35eacae2c2e..5a3759ef755 100644 --- a/app/views/projects/deploy_tokens/_revoke_modal.html.haml +++ b/app/views/shared/deploy_tokens/_revoke_modal.html.haml @@ -3,15 +3,13 @@ .modal-content .modal-header %h4.modal-title - = s_('DeployTokens|Revoke') - %b #{token.name}? + = s_('DeployTokens|Revoke %{b_start}%{name}%{b_end}?').html_safe % { b_start: '<b>'.html_safe, name: token.name, b_end: '</b>'.html_safe } %button.close{ type: "button", "data-dismiss": "modal", "aria-label" => _('Close') } %span{ "aria-hidden": true } × .modal-body %p - = s_('DeployTokens|You are about to revoke') - %b #{token.name}. + = s_('DeployTokens|You are about to revoke %{b_start}%{name}%{b_end}.').html_safe % { b_start: '<b>'.html_safe, name: token.name, b_end: '</b>'.html_safe } = s_('DeployTokens|This action cannot be undone.') .modal-footer %a{ href: '#', data: { dismiss: 'modal' }, class: 'btn btn-default' }= _('Cancel') - = link_to s_('DeployTokens|Revoke %{name}') % { name: token.name }, revoke_project_deploy_token_path(project, token), method: :put, class: 'btn btn-danger' + = link_to s_('DeployTokens|Revoke %{name}') % { name: token.name }, revoke_deploy_token_path(group_or_project, token), method: :put, class: 'btn btn-danger' diff --git a/app/views/projects/deploy_tokens/_table.html.haml b/app/views/shared/deploy_tokens/_table.html.haml index 91466a6736b..d4e20805a2a 100644 --- a/app/views/projects/deploy_tokens/_table.html.haml +++ b/app/views/shared/deploy_tokens/_table.html.haml @@ -22,10 +22,10 @@ %span{ class: ('text-warning' if token.expires_soon?) } In #{distance_of_time_in_words_to_now(token.expires_at)} - else - %span.token-never-expires-label Never - %td= token.scopes.present? ? token.scopes.join(", ") : "<no scopes selected>" + %span.token-never-expires-label= _('Never') + %td= token.scopes.present? ? token.scopes.join(", ") : _('<no scopes selected>') %td= link_to s_('DeployTokens|Revoke'), "#", class: "btn btn-danger float-right", data: { toggle: "modal", target: "#revoke-modal-#{token.id}"} - = render 'projects/deploy_tokens/revoke_modal', token: token, project: project + = render 'shared/deploy_tokens/revoke_modal', token: token, group_or_project: group_or_project - else .settings-message.text-center - = s_('DeployTokens|This project has no active Deploy Tokens.') + = s_('DeployTokens|This %{entity_type} has no active Deploy Tokens.') % { entity_type: group_or_project.class.name.downcase } diff --git a/changelogs/unreleased/21765-group-token-refactor.yml b/changelogs/unreleased/21765-group-token-refactor.yml new file mode 100644 index 00000000000..3a93d957c86 --- /dev/null +++ b/changelogs/unreleased/21765-group-token-refactor.yml @@ -0,0 +1,5 @@ +--- +title: Addition of the Group Deploy Token interface +merge_request: 24102 +author: +type: added diff --git a/changelogs/unreleased/cat-fix-namespaceid-import-39078.yml b/changelogs/unreleased/cat-fix-namespaceid-import-39078.yml new file mode 100644 index 00000000000..993662fff97 --- /dev/null +++ b/changelogs/unreleased/cat-fix-namespaceid-import-39078.yml @@ -0,0 +1,5 @@ +--- +title: Fixes project import failures when user is not part of any groups +merge_request: 26038 +author: +type: fixed diff --git a/changelogs/unreleased/fj-39265-create-snippet-repository-content.yml b/changelogs/unreleased/fj-39265-create-snippet-repository-content.yml new file mode 100644 index 00000000000..25a12005e95 --- /dev/null +++ b/changelogs/unreleased/fj-39265-create-snippet-repository-content.yml @@ -0,0 +1,5 @@ +--- +title: Commit file when snippet is created +merge_request: 23953 +author: +type: added diff --git a/changelogs/unreleased/insights-description-for-chart.yml b/changelogs/unreleased/insights-description-for-chart.yml new file mode 100644 index 00000000000..79105e40a6c --- /dev/null +++ b/changelogs/unreleased/insights-description-for-chart.yml @@ -0,0 +1,5 @@ +--- +title: Allow chart descriptions for Insights +merge_request: 25686 +author: +type: added diff --git a/changelogs/unreleased/mwaw-remove_logs_path_for_not_authorised_users.yml b/changelogs/unreleased/mwaw-remove_logs_path_for_not_authorised_users.yml new file mode 100644 index 00000000000..249aa36e9fd --- /dev/null +++ b/changelogs/unreleased/mwaw-remove_logs_path_for_not_authorised_users.yml @@ -0,0 +1,5 @@ +--- +title: Remove unreachable link from embded dashboard context menu +merge_request: 25892 +author: +type: fixed diff --git a/changelogs/unreleased/remove-puma-notices-from-admin-area-banner.yml b/changelogs/unreleased/remove-puma-notices-from-admin-area-banner.yml new file mode 100644 index 00000000000..a5dd6b83f4d --- /dev/null +++ b/changelogs/unreleased/remove-puma-notices-from-admin-area-banner.yml @@ -0,0 +1,5 @@ +--- +title: Remove Puma notices from AdminArea banner +merge_request: 26137 +author: +type: changed diff --git a/config/routes/group.rb b/config/routes/group.rb index 68e239faf6d..1d51b3fb6fe 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -29,6 +29,7 @@ constraints(::Constraints::GroupUrlConstrainer.new) do resource :ci_cd, only: [:show, :update], controller: 'ci_cd' do put :reset_registration_token patch :update_auto_devops + post :create_deploy_token, path: 'deploy_token/create' end end @@ -49,6 +50,12 @@ constraints(::Constraints::GroupUrlConstrainer.new) do end end + resources :deploy_tokens, constraints: { id: /\d+/ }, only: [] do + member do + put :revoke + end + end + resource :avatar, only: [:destroy] concerns :clusterable diff --git a/config/routes/project.rb b/config/routes/project.rb index aa2410d8e00..cff075b260f 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -79,7 +79,9 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do resource :integrations, only: [:show] resource :repository, only: [:show], controller: :repository do - post :create_deploy_token, path: 'deploy_token/create' + # TODO: Move 'create_deploy_token' here to the ':ci_cd' resource above during 12.9. + # More details here: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/24102#note_287572556 + post :create_deploy_token, path: 'deploy_token/create', to: 'ci_cd#create_deploy_token' post :cleanup end end diff --git a/doc/development/sidekiq_style_guide.md b/doc/development/sidekiq_style_guide.md index 70385b666a4..4445efa823a 100644 --- a/doc/development/sidekiq_style_guide.md +++ b/doc/development/sidekiq_style_guide.md @@ -64,6 +64,63 @@ the extra jobs will take resources away from jobs from workers that were already there, if the resources available to the Sidekiq process handling the namespace are not adjusted appropriately. +## Idempotent Jobs + +It's known that a job can fail for multiple reasons, for example, network outages or bugs. +In order to address this, Sidekiq has a built-in retry mechanism that is +used by default by most workers within GitLab. + +It's expected that a job can run again after a failure without major side-effects for the +application or users, which is why Sidekiq encourages +jobs to be [idempotent and transactional](https://github.com/mperham/sidekiq/wiki/Best-Practices#2-make-your-job-idempotent-and-transactional). + +As a general rule, a worker can be considered idempotent if: + +- It can safely run multiple times with the same arguments. +- Application side-effects are expected to happen only once + (or side-effects of a second run are not impactful). + +A good example of that would be a cache expiration worker. + +### Ensuring a worker is idempotent + +Make sure the worker tests pass using the following shared example: + +```ruby +include_examples 'an idempotent worker' do + it 'marks the MR as merged' do + # Using subject inside this block will process the job multiple times + subject + + expect(merge_request.state).to eq('merged') + end +end +``` + +Use the `perform_multiple` method directly instead of `job.perform` (this +helper method is automatically included for workers). + +### Declaring a worker as idempotent + +```ruby +class IdempotentWorker + include ApplicationWorker + + # Declares a worker is idempotent and can + # safely run multiple times. + idempotent! + + # ... +end +``` + +It's encouraged to only have the `idempotent!` call in the top-most worker class, even if +the `perform` method is defined in another class or module. + +NOTE: **Note:** +Note that a cop will fail if the worker class is not marked as idempotent. +Consider skipping the cop if you're not confident your job can safely run multiple times. + ## Latency Sensitive Jobs If a large number of background jobs get scheduled at once, queueing of jobs may diff --git a/doc/user/project/insights/index.md b/doc/user/project/insights/index.md index 4af6f47ce7b..52fcad8dd80 100644 --- a/doc/user/project/insights/index.md +++ b/doc/user/project/insights/index.md @@ -61,6 +61,7 @@ bugsCharts: title: "Charts for bugs" charts: - title: "Monthly bugs created" + description: "Open bugs created per month" type: bar query: issuable_type: issue @@ -77,6 +78,7 @@ For example, here's single chart definition: ```yaml - title: "Monthly bugs created" + description: "Open bugs created per month" type: bar query: issuable_type: issue @@ -96,6 +98,7 @@ The following table lists available parameters for charts: | Keyword | Description | |:---------------------------------------------------|:------------| | [`title`](#title) | The title of the chart. This will displayed on the Insights page. | +| [`description`](#description) | A description for the individual chart. This will be displayed above the relevant chart. | | [`type`](#type) | The type of chart: `bar`, `line` or `stacked-bar`. | | [`query`](#query) | A hash that defines the conditions for issues / merge requests to be part of the chart. | @@ -114,6 +117,17 @@ monthlyBugsCreated: title: "Monthly bugs created" ``` +### `description` + +The `description` text is displayed above the chart, but below the title. It's used +to give extra details regarding the chart, for example: + +```yaml +monthlyBugsCreated: + title: "Monthly bugs created" + description: "Open bugs created per month" +``` + ### `type` `type` is the chart type. @@ -145,6 +159,7 @@ Example: ```yaml monthlyBugsCreated: title: "Monthly bugs created" + description: "Open bugs created per month" type: bar query: issuable_type: issue @@ -283,6 +298,7 @@ a group's insights: ```yaml monthlyBugsCreated: title: "Monthly bugs created" + description: "Open bugs created per month" type: bar query: issuable_type: issue @@ -311,6 +327,7 @@ bugsCharts: title: "Charts for bugs" charts: - title: "Monthly bugs created" + description: "Open bugs created per month" type: bar <<: *projectsOnly query: diff --git a/doc/user/project/merge_requests/accessibility_testing.md b/doc/user/project/merge_requests/accessibility_testing.md index 7fa758b8a51..3d44f342715 100644 --- a/doc/user/project/merge_requests/accessibility_testing.md +++ b/doc/user/project/merge_requests/accessibility_testing.md @@ -33,17 +33,19 @@ defined in that template. Add the following to your `.gitlab-ci.yml` file: ```yaml -include: - template: Verify/Accessibility.gitlab-ci.yml +variables: + a11y_urls: "https://about.gitlab.com" -a11y: - variables: - a11y_urls: https://example.com https://example.com/another-page +include: + - remote: "https://gitlab.com/gitlab-org/gitlab/-/raw/master/lib/gitlab/ci/templates/Verify/Accessibility.gitlab-ci.yml" ``` The example above will create an `a11y` job in your CI/CD pipeline and will run Pa11y against the webpage you defined in `a11y_urls` to build a report. +NOTE: **Note:** +Only one URL may be currently passed into `a11y_urls`. + The full HTML Pa11y report will be saved as an artifact that can be [viewed directly in your browser](../pipelines/job_artifacts.md#browsing-artifacts). NOTE: **Note:** diff --git a/lib/gitlab/config_checker/puma_rugged_checker.rb b/lib/gitlab/config_checker/puma_rugged_checker.rb index a9bbaebaf0b..82c59f3328b 100644 --- a/lib/gitlab/config_checker/puma_rugged_checker.rb +++ b/lib/gitlab/config_checker/puma_rugged_checker.rb @@ -7,36 +7,14 @@ module Gitlab extend Gitlab::Git::RuggedImpl::UseRugged def check - return [] unless Gitlab::Runtime.puma? - notices = [] - link_start = '<a href="https://docs.gitlab.com/ee/administration/operations/puma.html">' - link_end = '</a>' - notices << { - type: 'info', - message: _('You are running Puma, which is currently experimental. '\ - 'More information is available in our '\ - '%{link_start}documentation%{link_end}.') % { link_start: link_start, link_end: link_end } - } - - if running_puma_with_multiple_threads? - link_start = '<a href="https://docs.gitlab.com/ee/administration/operations/puma.html">' - link_end = '</a>' - notices << { - type: 'info', - message: _('Puma is running with a thread count above 1. '\ - 'Information on deprecated GitLab features in this configuration is available in the '\ - '%{link_start}documentation%{link_end}.') % { link_start: link_start, link_end: link_end } - } - end - if running_puma_with_multiple_threads? && rugged_enabled_through_feature_flag? link_start = '<a href="https://docs.gitlab.com/ee/administration/operations/puma.html#performance-caveat-when-using-puma-with-rugged">' link_end = '</a>' notices << { type: 'warning', - message: _('Puma is running with a thread count above 1 and the rugged '\ + message: _('Puma is running with a thread count above 1 and the Rugged '\ 'service is enabled. This may decrease performance in some environments. '\ 'See our %{link_start}documentation%{link_end} '\ 'for details of this issue.') % { link_start: link_start, link_end: link_end } diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 1dfa9d0c95f..aaa81eceed6 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -6507,6 +6507,9 @@ msgstr "" msgid "DeployTokens|Expires" msgstr "" +msgid "DeployTokens|Group deploy tokens allow read-only access to the repositories and registry images within the group." +msgstr "" + msgid "DeployTokens|Name" msgstr "" @@ -6516,16 +6519,19 @@ msgstr "" msgid "DeployTokens|Revoke" msgstr "" +msgid "DeployTokens|Revoke %{b_start}%{name}%{b_end}?" +msgstr "" + msgid "DeployTokens|Revoke %{name}" msgstr "" msgid "DeployTokens|Scopes" msgstr "" -msgid "DeployTokens|This action cannot be undone." +msgid "DeployTokens|This %{entity_type} has no active Deploy Tokens." msgstr "" -msgid "DeployTokens|This project has no active Deploy Tokens." +msgid "DeployTokens|This action cannot be undone." msgstr "" msgid "DeployTokens|Use this token as a password. Make sure you save it - you won't be able to access it again." @@ -6537,12 +6543,15 @@ msgstr "" msgid "DeployTokens|Username" msgstr "" -msgid "DeployTokens|You are about to revoke" +msgid "DeployTokens|You are about to revoke %{b_start}%{name}%{b_end}." msgstr "" msgid "DeployTokens|Your New Deploy Token" msgstr "" +msgid "DeployTokens|Your new group deploy token has been created." +msgstr "" + msgid "DeployTokens|Your new project deploy token has been created." msgstr "" @@ -15735,10 +15744,7 @@ msgstr "" msgid "Pull" msgstr "" -msgid "Puma is running with a thread count above 1 and the rugged service is enabled. This may decrease performance in some environments. See our %{link_start}documentation%{link_end} for details of this issue." -msgstr "" - -msgid "Puma is running with a thread count above 1. Information on deprecated GitLab features in this configuration is available in the %{link_start}documentation%{link_end}." +msgid "Puma is running with a thread count above 1 and the Rugged service is enabled. This may decrease performance in some environments. See our %{link_start}documentation%{link_end} for details of this issue." msgstr "" msgid "Purchase more minutes" @@ -22324,9 +22330,6 @@ msgstr "" msgid "You are receiving this message because you are a GitLab administrator for %{url}." msgstr "" -msgid "You are running Puma, which is currently experimental. More information is available in our %{link_start}documentation%{link_end}." -msgstr "" - msgid "You can %{linkStart}view the blob%{linkEnd} instead." msgstr "" diff --git a/qa/qa/page/project/settings/ci_cd.rb b/qa/qa/page/project/settings/ci_cd.rb index 46f93fad61e..01f7ab43d27 100644 --- a/qa/qa/page/project/settings/ci_cd.rb +++ b/qa/qa/page/project/settings/ci_cd.rb @@ -13,6 +13,16 @@ module QA element :variables_settings_content end + view 'app/views/shared/deploy_tokens/_index.html.haml' do + element :deploy_tokens_settings + end + + def expand_deploy_tokens(&block) + expand_section(:deploy_tokens_settings) do + Settings::DeployTokens.perform(&block) + end + end + def expand_runners_settings(&block) expand_section(:runners_settings_content) do Settings::Runners.perform(&block) diff --git a/qa/qa/page/project/settings/deploy_tokens.rb b/qa/qa/page/project/settings/deploy_tokens.rb index 3173752d40a..3c3ed4f8716 100644 --- a/qa/qa/page/project/settings/deploy_tokens.rb +++ b/qa/qa/page/project/settings/deploy_tokens.rb @@ -5,7 +5,7 @@ module QA module Project module Settings class DeployTokens < Page::Base - view 'app/views/projects/deploy_tokens/_form.html.haml' do + view 'app/views/shared/deploy_tokens/_form.html.haml' do element :deploy_token_name element :deploy_token_expires_at element :deploy_token_read_repository @@ -13,7 +13,7 @@ module QA element :create_deploy_token end - view 'app/views/projects/deploy_tokens/_new_deploy_token.html.haml' do + view 'app/views/shared/deploy_tokens/_new_deploy_token.html.haml' do element :created_deploy_token_section element :deploy_token_user element :deploy_token diff --git a/qa/qa/page/project/settings/repository.rb b/qa/qa/page/project/settings/repository.rb index 58ed37870b7..7875a38dcf3 100644 --- a/qa/qa/page/project/settings/repository.rb +++ b/qa/qa/page/project/settings/repository.rb @@ -31,12 +31,6 @@ module QA end end - def expand_deploy_tokens(&block) - expand_section(:deploy_tokens_settings) do - DeployTokens.perform(&block) - end - end - def expand_mirroring_repositories(&block) expand_section(:mirroring_repositories_settings_section) do MirroringRepositories.perform(&block) diff --git a/qa/qa/resource/deploy_token.rb b/qa/qa/resource/deploy_token.rb index f97e76cc322..b4baaa47349 100644 --- a/qa/qa/resource/deploy_token.rb +++ b/qa/qa/resource/deploy_token.rb @@ -6,16 +6,16 @@ module QA attr_accessor :name, :expires_at attribute :username do - Page::Project::Settings::Repository.perform do |repository_page| - repository_page.expand_deploy_tokens do |token| + Page::Project::Settings::CICD.perform do |cicd_page| + cicd_page.expand_deploy_tokens do |token| token.token_username end end end attribute :password do - Page::Project::Settings::Repository.perform do |repository_page| - repository_page.expand_deploy_tokens do |token| + Page::Project::Settings::CICD.perform do |cicd_page| + cicd_page.expand_deploy_tokens do |token| token.token_password end end @@ -31,12 +31,10 @@ module QA def fabricate! project.visit! - Page::Project::Menu.act do - go_to_repository_settings - end + Page::Project::Menu.perform(&:go_to_ci_cd_settings) - Page::Project::Settings::Repository.perform do |setting| - setting.expand_deploy_tokens do |page| + Page::Project::Settings::CICD.perform do |cicd| + cicd.expand_deploy_tokens do |page| page.fill_token_name(name) page.fill_token_expires_at(expires_at) page.fill_scopes(read_repository: true, read_registry: false) diff --git a/spec/controllers/groups/settings/ci_cd_controller_spec.rb b/spec/controllers/groups/settings/ci_cd_controller_spec.rb index 4e8cb3f94fb..fbf88a01eb3 100644 --- a/spec/controllers/groups/settings/ci_cd_controller_spec.rb +++ b/spec/controllers/groups/settings/ci_cd_controller_spec.rb @@ -210,4 +210,16 @@ describe Groups::Settings::CiCdController do end end end + + describe 'POST create_deploy_token' do + it_behaves_like 'a created deploy token' do + let(:entity) { group } + let(:create_entity_params) { { group_id: group } } + let(:deploy_token_type) { DeployToken.deploy_token_types[:group_type] } + + before do + entity.add_owner(user) + end + end + end end diff --git a/spec/controllers/projects/settings/ci_cd_controller_spec.rb b/spec/controllers/projects/settings/ci_cd_controller_spec.rb index 68260e4e101..a8631389e17 100644 --- a/spec/controllers/projects/settings/ci_cd_controller_spec.rb +++ b/spec/controllers/projects/settings/ci_cd_controller_spec.rb @@ -247,4 +247,12 @@ describe Projects::Settings::CiCdController do end end end + + describe 'POST create_deploy_token' do + it_behaves_like 'a created deploy token' do + let(:entity) { project } + let(:create_entity_params) { { namespace_id: project.namespace, project_id: project } } + let(:deploy_token_type) { DeployToken.deploy_token_types[:project_type] } + end + end end diff --git a/spec/controllers/projects/settings/repository_controller_spec.rb b/spec/controllers/projects/settings/repository_controller_spec.rb index 64f5b8e34ae..67ae9ebda38 100644 --- a/spec/controllers/projects/settings/repository_controller_spec.rb +++ b/spec/controllers/projects/settings/repository_controller_spec.rb @@ -32,24 +32,4 @@ describe Projects::Settings::RepositoryController do expect(RepositoryCleanupWorker).to have_received(:perform_async).once end end - - describe 'POST create_deploy_token' do - let(:deploy_token_params) do - { - name: 'deployer_token', - expires_at: 1.month.from_now.to_date.to_s, - username: 'deployer', - read_repository: '1' - } - end - - subject(:create_deploy_token) { post :create_deploy_token, params: { namespace_id: project.namespace, project_id: project, deploy_token: deploy_token_params } } - - it 'creates deploy token' do - expect { create_deploy_token }.to change { DeployToken.active.count }.by(1) - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:show) - end - end end diff --git a/spec/features/groups/settings/ci_cd_spec.rb b/spec/features/groups/settings/ci_cd_spec.rb index 5b1a9512c55..3fbc7c7a695 100644 --- a/spec/features/groups/settings/ci_cd_spec.rb +++ b/spec/features/groups/settings/ci_cd_spec.rb @@ -37,6 +37,19 @@ describe 'Group CI/CD settings' do end end + context 'Deploy tokens' do + let!(:deploy_token) { create(:deploy_token, :group, groups: [group]) } + + before do + stub_container_registry_config(enabled: true) + visit group_settings_ci_cd_path(group) + end + + it_behaves_like 'a deploy token in ci/cd settings' do + let(:entity_type) { 'group' } + end + end + describe 'Auto DevOps form' do before do stub_application_setting(auto_devops_enabled: true) diff --git a/spec/features/projects/settings/ci_cd_settings_spec.rb b/spec/features/projects/settings/ci_cd_settings_spec.rb new file mode 100644 index 00000000000..e69ee31e582 --- /dev/null +++ b/spec/features/projects/settings/ci_cd_settings_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Projects > Settings > CI/CD settings' do + let(:project) { create(:project_empty_repo) } + let(:user) { create(:user) } + let(:role) { :maintainer } + + context 'Deploy tokens' do + let!(:deploy_token) { create(:deploy_token, projects: [project]) } + + before do + project.add_role(user, role) + sign_in(user) + stub_container_registry_config(enabled: true) + visit project_settings_ci_cd_path(project) + end + + it_behaves_like 'a deploy token in ci/cd settings' do + let(:entity_type) { 'project' } + end + end +end diff --git a/spec/features/projects/settings/repository_settings_spec.rb b/spec/features/projects/settings/repository_settings_spec.rb index 18031a40f15..d750234d9ad 100644 --- a/spec/features/projects/settings/repository_settings_spec.rb +++ b/spec/features/projects/settings/repository_settings_spec.rb @@ -108,39 +108,6 @@ describe 'Projects > Settings > Repository settings' do end end - context 'Deploy tokens' do - let!(:deploy_token) { create(:deploy_token, projects: [project]) } - - before do - stub_container_registry_config(enabled: true) - visit project_settings_repository_path(project) - end - - it 'view deploy tokens' do - within('.deploy-tokens') do - expect(page).to have_content(deploy_token.name) - expect(page).to have_content('read_repository') - expect(page).to have_content('read_registry') - end - end - - it 'add a new deploy token' do - fill_in 'deploy_token_name', with: 'new_deploy_key' - fill_in 'deploy_token_expires_at', with: (Date.today + 1.month).to_s - fill_in 'deploy_token_username', with: 'deployer' - check 'deploy_token_read_repository' - check 'deploy_token_read_registry' - click_button 'Create deploy token' - - expect(page).to have_content('Your new project deploy token has been created') - - within('.created-deploy-token-container') do - expect(page).to have_selector("input[name='deploy-token-user'][value='deployer']") - expect(page).to have_selector("input[name='deploy-token'][readonly='readonly']") - end - end - end - context 'remote mirror settings' do let(:user2) { create(:user) } diff --git a/spec/features/projects/settings/user_sees_revoke_deploy_token_modal_spec.rb b/spec/features/projects/settings/user_sees_revoke_deploy_token_modal_spec.rb index 3e9bfed1e47..a9253c20896 100644 --- a/spec/features/projects/settings/user_sees_revoke_deploy_token_modal_spec.rb +++ b/spec/features/projects/settings/user_sees_revoke_deploy_token_modal_spec.rb @@ -11,7 +11,7 @@ describe 'Repository Settings > User sees revoke deploy token modal', :js do before do project.add_role(user, role) sign_in(user) - visit(project_settings_repository_path(project)) + visit(project_settings_ci_cd_path(project)) click_link('Revoke') end diff --git a/spec/javascripts/behaviors/quick_submit_spec.js b/spec/frontend/behaviors/quick_submit_spec.js index 7af8c984841..2dc2bb198e8 100644 --- a/spec/javascripts/behaviors/quick_submit_spec.js +++ b/spec/frontend/behaviors/quick_submit_spec.js @@ -1,70 +1,72 @@ import $ from 'jquery'; import '~/behaviors/quick_submit'; -describe('Quick Submit behavior', function() { +describe('Quick Submit behavior', () => { + let testContext; + const keydownEvent = (options = { keyCode: 13, metaKey: true }) => $.Event('keydown', options); preloadFixtures('snippets/show.html'); beforeEach(() => { loadFixtures('snippets/show.html'); + + testContext = {}; + + testContext.spies = { + submit: jest.fn(), + }; + $('form').submit(e => { // Prevent a form submit from moving us off the testing page e.preventDefault(); + // Explicitly call the spie to know this function get's not called + testContext.spies.submit(); }); - this.spies = { - submit: spyOnEvent('form', 'submit'), - }; - - this.textarea = $('.js-quick-submit textarea').first(); - }); - - afterEach(() => { - // Undo what we did to the shared <body> - $('body').removeAttr('data-page'); + testContext.textarea = $('.js-quick-submit textarea').first(); }); it('does not respond to other keyCodes', () => { - this.textarea.trigger( + testContext.textarea.trigger( keydownEvent({ keyCode: 32, }), ); - expect(this.spies.submit).not.toHaveBeenTriggered(); + expect(testContext.spies.submit).not.toHaveBeenCalled(); }); it('does not respond to Enter alone', () => { - this.textarea.trigger( + testContext.textarea.trigger( keydownEvent({ ctrlKey: false, metaKey: false, }), ); - expect(this.spies.submit).not.toHaveBeenTriggered(); + expect(testContext.spies.submit).not.toHaveBeenCalled(); }); it('does not respond to repeated events', () => { - this.textarea.trigger( + testContext.textarea.trigger( keydownEvent({ repeat: true, }), ); - expect(this.spies.submit).not.toHaveBeenTriggered(); + expect(testContext.spies.submit).not.toHaveBeenCalled(); }); it('disables input of type submit', () => { const submitButton = $('.js-quick-submit input[type=submit]'); - this.textarea.trigger(keydownEvent()); + testContext.textarea.trigger(keydownEvent()); expect(submitButton).toBeDisabled(); }); it('disables button of type submit', () => { const submitButton = $('.js-quick-submit input[type=submit]'); - this.textarea.trigger(keydownEvent()); + testContext.textarea.trigger(keydownEvent()); expect(submitButton).toBeDisabled(); }); @@ -73,71 +75,79 @@ describe('Quick Submit behavior', function() { const existingSubmit = $('.js-quick-submit input[type=submit]'); // Add an extra submit button const newSubmit = $('<button type="submit">Submit it</button>'); - newSubmit.insertAfter(this.textarea); + newSubmit.insertAfter(testContext.textarea); - const oldClick = spyOnEvent(existingSubmit, 'click'); - const newClick = spyOnEvent(newSubmit, 'click'); + const spies = { + oldClickSpy: jest.fn(), + newClickSpy: jest.fn(), + }; + existingSubmit.on('click', () => { + spies.oldClickSpy(); + }); + newSubmit.on('click', () => { + spies.newClickSpy(); + }); - this.textarea.trigger(keydownEvent()); + testContext.textarea.trigger(keydownEvent()); - expect(oldClick).not.toHaveBeenTriggered(); - expect(newClick).toHaveBeenTriggered(); + expect(spies.oldClickSpy).not.toHaveBeenCalled(); + expect(spies.newClickSpy).toHaveBeenCalled(); }); // We cannot stub `navigator.userAgent` for CI's `rake karma` task, so we'll // only run the tests that apply to the current platform if (navigator.userAgent.match(/Macintosh/)) { describe('In Macintosh', () => { it('responds to Meta+Enter', () => { - this.textarea.trigger(keydownEvent()); + testContext.textarea.trigger(keydownEvent()); - expect(this.spies.submit).toHaveBeenTriggered(); + expect(testContext.spies.submit).toHaveBeenCalled(); }); it('excludes other modifier keys', () => { - this.textarea.trigger( + testContext.textarea.trigger( keydownEvent({ altKey: true, }), ); - this.textarea.trigger( + testContext.textarea.trigger( keydownEvent({ ctrlKey: true, }), ); - this.textarea.trigger( + testContext.textarea.trigger( keydownEvent({ shiftKey: true, }), ); - expect(this.spies.submit).not.toHaveBeenTriggered(); + expect(testContext.spies.submit).not.toHaveBeenCalled(); }); }); } else { it('responds to Ctrl+Enter', () => { - this.textarea.trigger(keydownEvent()); + testContext.textarea.trigger(keydownEvent()); - expect(this.spies.submit).toHaveBeenTriggered(); + expect(testContext.spies.submit).toHaveBeenCalled(); }); it('excludes other modifier keys', () => { - this.textarea.trigger( + testContext.textarea.trigger( keydownEvent({ altKey: true, }), ); - this.textarea.trigger( + testContext.textarea.trigger( keydownEvent({ metaKey: true, }), ); - this.textarea.trigger( + testContext.textarea.trigger( keydownEvent({ shiftKey: true, }), ); - expect(this.spies.submit).not.toHaveBeenTriggered(); + expect(testContext.spies.submit).not.toHaveBeenCalled(); }); } }); diff --git a/spec/frontend/registry/explorer/pages/details_spec.js b/spec/frontend/registry/explorer/pages/details_spec.js index 48f3b0f9b65..2b83f7e7351 100644 --- a/spec/frontend/registry/explorer/pages/details_spec.js +++ b/spec/frontend/registry/explorer/pages/details_spec.js @@ -1,5 +1,5 @@ import { mount } from '@vue/test-utils'; -import { GlTable, GlPagination, GlLoadingIcon } from '@gitlab/ui'; +import { GlTable, GlPagination, GlSkeletonLoader } from '@gitlab/ui'; import Tracking from '~/tracking'; import stubChildren from 'helpers/stub_children'; import component from '~/registry/explorer/pages/details.vue'; @@ -14,8 +14,7 @@ describe('Details Page', () => { const findDeleteModal = () => wrapper.find(GlModal); const findPagination = () => wrapper.find(GlPagination); - const findLoadingIcon = () => wrapper.find(GlLoadingIcon); - const findTagsTable = () => wrapper.find(GlTable); + const findSkeletonLoader = () => wrapper.find(GlSkeletonLoader); const findMainCheckbox = () => wrapper.find({ ref: 'mainCheckbox' }); const findFirstRowItem = ref => wrapper.find({ ref }); const findBulkDeleteButton = () => wrapper.find({ ref: 'bulkDeleteButton' }); @@ -33,7 +32,7 @@ describe('Details Page', () => { ...stubChildren(component), GlModal, GlSprintf: false, - GlTable: false, + GlTable, }, mocks: { $route: { @@ -53,18 +52,19 @@ describe('Details Page', () => { }); describe('when isLoading is true', () => { - beforeAll(() => store.commit(SET_MAIN_LOADING, true)); + beforeEach(() => { + store.dispatch('receiveTagsListSuccess', { ...tagsListResponse, data: [] }); + store.commit(SET_MAIN_LOADING, true); + }); afterAll(() => store.commit(SET_MAIN_LOADING, false)); - it('has a loading icon', () => { - expect(findLoadingIcon().exists()).toBe(true); + it('has a skeleton loader', () => { + expect(findSkeletonLoader().exists()).toBe(true); }); - it('does not have a main content', () => { - expect(findTagsTable().exists()).toBe(false); - expect(findPagination().exists()).toBe(false); - expect(findDeleteModal().exists()).toBe(false); + it('does not have list items', () => { + expect(findFirstRowItem('rowCheckbox').exists()).toBe(false); }); }); diff --git a/spec/frontend/registry/explorer/pages/list_spec.js b/spec/frontend/registry/explorer/pages/list_spec.js index f463dc49035..91c3c242ed4 100644 --- a/spec/frontend/registry/explorer/pages/list_spec.js +++ b/spec/frontend/registry/explorer/pages/list_spec.js @@ -1,6 +1,6 @@ import VueRouter from 'vue-router'; import { shallowMount, createLocalVue } from '@vue/test-utils'; -import { GlPagination, GlLoadingIcon, GlSprintf } from '@gitlab/ui'; +import { GlPagination, GlSkeletonLoader, GlSprintf } from '@gitlab/ui'; import Tracking from '~/tracking'; import component from '~/registry/explorer/pages/list.vue'; import store from '~/registry/explorer/stores/'; @@ -17,7 +17,7 @@ describe('List Page', () => { const findDeleteBtn = () => wrapper.find({ ref: 'deleteImageButton' }); const findDeleteModal = () => wrapper.find(GlModal); - const findLoadingIcon = () => wrapper.find(GlLoadingIcon); + const findSkeletonLoader = () => wrapper.find(GlSkeletonLoader); const findImagesList = () => wrapper.find({ ref: 'imagesList' }); const findRowItems = () => wrapper.findAll({ ref: 'rowItem' }); const findEmptyState = () => wrapper.find(GlEmptyState); @@ -71,7 +71,7 @@ describe('List Page', () => { }); it('should not show the loading or default state', () => { - expect(findLoadingIcon().exists()).toBe(false); + expect(findSkeletonLoader().exists()).toBe(false); expect(findImagesList().exists()).toBe(false); }); }); @@ -81,8 +81,8 @@ describe('List Page', () => { afterAll(() => store.commit(SET_MAIN_LOADING, false)); - it('shows the loading icon', () => { - expect(findLoadingIcon().exists()).toBe(true); + it('shows the skeleton loader', () => { + expect(findSkeletonLoader().exists()).toBe(true); }); it('imagesList is not visible', () => { diff --git a/spec/frontend/registry/explorer/stores/actions_spec.js b/spec/frontend/registry/explorer/stores/actions_spec.js index a3fb29c0eb9..3e22621058e 100644 --- a/spec/frontend/registry/explorer/stores/actions_spec.js +++ b/spec/frontend/registry/explorer/stores/actions_spec.js @@ -180,10 +180,7 @@ describe('Actions RegistryExplorer Store', () => { { tagsPagination: {}, }, - [ - { type: types.SET_MAIN_LOADING, payload: true }, - { type: types.SET_MAIN_LOADING, payload: false }, - ], + [{ type: types.SET_MAIN_LOADING, payload: true }], [ { type: 'requestTagsList', @@ -220,13 +217,11 @@ describe('Actions RegistryExplorer Store', () => { }); describe('request delete multiple tags', () => { - const id = 1; - const params = window.btoa(JSON.stringify({ id })); - const projectPath = 'project-path'; - const url = `${projectPath}/registry/repository/${id}/tags/bulk_destroy`; + const url = `project-path/registry/repository/foo/tags`; + const params = window.btoa(JSON.stringify({ tags_path: `${url}?format=json` })); it('successfully performs the delete request', done => { - mock.onDelete(url).replyOnce(200); + mock.onDelete(`${url}/bulk_destroy`).replyOnce(200); testAction( actions.requestDeleteTags, @@ -235,15 +230,9 @@ describe('Actions RegistryExplorer Store', () => { params, }, { - config: { - projectPath, - }, tagsPagination: {}, }, - [ - { type: types.SET_MAIN_LOADING, payload: true }, - { type: types.SET_MAIN_LOADING, payload: false }, - ], + [{ type: types.SET_MAIN_LOADING, payload: true }], [ { type: 'requestTagsList', @@ -267,9 +256,6 @@ describe('Actions RegistryExplorer Store', () => { params, }, { - config: { - projectPath, - }, tagsPagination: {}, }, [ diff --git a/spec/frontend/registry/explorer/stores/getters_spec.js b/spec/frontend/registry/explorer/stores/getters_spec.js new file mode 100644 index 00000000000..c224f076d30 --- /dev/null +++ b/spec/frontend/registry/explorer/stores/getters_spec.js @@ -0,0 +1,34 @@ +import * as getters from '~/registry/explorer/stores/getters'; + +describe('Getters RegistryExplorer store', () => { + let state; + const tags = ['foo', 'bar']; + + describe('tags', () => { + describe('when isLoading is false', () => { + beforeEach(() => { + state = { + tags, + isLoading: false, + }; + }); + + it('returns tags', () => { + expect(getters.tags(state)).toEqual(state.tags); + }); + }); + + describe('when isLoading is true', () => { + beforeEach(() => { + state = { + tags, + isLoading: true, + }; + }); + + it('returns empty array', () => { + expect(getters.tags(state)).toEqual([]); + }); + }); + }); +}); diff --git a/spec/lib/gitlab/config_checker/puma_rugged_checker_spec.rb b/spec/lib/gitlab/config_checker/puma_rugged_checker_spec.rb index 070887c83c3..badfd56d571 100644 --- a/spec/lib/gitlab/config_checker/puma_rugged_checker_spec.rb +++ b/spec/lib/gitlab/config_checker/puma_rugged_checker_spec.rb @@ -15,26 +15,10 @@ describe Gitlab::ConfigChecker::PumaRuggedChecker do end context 'application is puma' do - let(:notice_running_puma) do - { - type: 'info', - message: 'You are running Puma, which is currently experimental. '\ - 'More information is available in our '\ - '<a href="https://docs.gitlab.com/ee/administration/operations/puma.html">documentation</a>.' - } - end - let(:notice_multi_threaded_puma) do - { - type: 'info', - message: 'Puma is running with a thread count above 1. '\ - 'Information on deprecated GitLab features in this configuration is available in the '\ - '<a href="https://docs.gitlab.com/ee/administration/operations/puma.html">documentation</a>.'\ - } - end let(:notice_multi_threaded_puma_with_rugged) do { type: 'warning', - message: 'Puma is running with a thread count above 1 and the rugged '\ + message: 'Puma is running with a thread count above 1 and the Rugged '\ 'service is enabled. This may decrease performance in some environments. '\ 'See our <a href="https://docs.gitlab.com/ee/administration/operations/puma.html#performance-caveat-when-using-puma-with-rugged">documentation</a> '\ 'for details of this issue.' @@ -51,35 +35,29 @@ describe Gitlab::ConfigChecker::PumaRuggedChecker do let(:multithreaded_puma) { false } let(:rugged_enabled) { true } - it 'report running puma notice' do - is_expected.to contain_exactly(notice_running_puma) - end + it { is_expected.to be_empty } end context 'not multithreaded_puma and rugged API is not enabled' do let(:multithreaded_puma) { false } let(:rugged_enabled) { false } - it 'report running puma notice' do - is_expected.to contain_exactly(notice_running_puma) - end + it { is_expected.to be_empty } end context 'multithreaded_puma and rugged API is not enabled' do let(:multithreaded_puma) { true } let(:rugged_enabled) { false } - it 'report running puma notice and multi-thread puma notice' do - is_expected.to contain_exactly(notice_running_puma, notice_multi_threaded_puma) - end + it { is_expected.to be_empty } end context 'multithreaded_puma and rugged API is enabled' do let(:multithreaded_puma) { true } let(:rugged_enabled) { true } - it 'report puma/multi_threaded_puma/multi_threaded_puma_with_rugged notices' do - is_expected.to contain_exactly(notice_running_puma, notice_multi_threaded_puma, notice_multi_threaded_puma_with_rugged) + it 'report multi_threaded_puma_with_rugged notices' do + is_expected.to contain_exactly(notice_multi_threaded_puma_with_rugged) end end end diff --git a/spec/models/snippet_repository_spec.rb b/spec/models/snippet_repository_spec.rb index 9befbb02b17..5b16b945efe 100644 --- a/spec/models/snippet_repository_spec.rb +++ b/spec/models/snippet_repository_spec.rb @@ -3,6 +3,11 @@ require 'spec_helper' describe SnippetRepository do + let_it_be(:user) { create(:user) } + let(:snippet) { create(:personal_snippet, :repository, author: user) } + let(:snippet_repository) { snippet.snippet_repository } + let(:commit_opts) { { branch_name: 'master', message: 'whatever' } } + describe 'associations' do it { is_expected.to belong_to(:shard) } it { is_expected.to belong_to(:snippet) } @@ -10,7 +15,7 @@ describe SnippetRepository do describe '.find_snippet' do it 'finds snippet by disk path' do - snippet = create(:snippet) + snippet = create(:snippet, author: user) snippet.track_snippet_repository expect(described_class.find_snippet(snippet.disk_path)).to eq(snippet) @@ -20,4 +25,147 @@ describe SnippetRepository do expect(described_class.find_snippet('@@unexisting/path/to/snippet')).to be_nil end end + + describe '#multi_files_action' do + let(:new_file) { { file_path: 'new_file_test', content: 'bar' } } + let(:move_file) { { previous_path: 'CHANGELOG', file_path: 'CHANGELOG_new', content: 'bar' } } + let(:update_file) { { previous_path: 'README', file_path: 'README', content: 'bar' } } + let(:data) { [new_file, move_file, update_file] } + + it 'returns nil when files argument is empty' do + expect(snippet.repository).not_to receive(:multi_action) + + operation = snippet_repository.multi_files_action(user, [], commit_opts) + + expect(operation).to be_nil + end + + it 'returns nil when files argument is nil' do + expect(snippet.repository).not_to receive(:multi_action) + + operation = snippet_repository.multi_files_action(user, nil, commit_opts) + + expect(operation).to be_nil + end + + it 'performs the operation accordingly to the files data' do + new_file_blob = blob_at(snippet, new_file[:file_path]) + move_file_blob = blob_at(snippet, move_file[:previous_path]) + update_file_blob = blob_at(snippet, update_file[:previous_path]) + + aggregate_failures do + expect(new_file_blob).to be_nil + expect(move_file_blob).not_to be_nil + expect(update_file_blob).not_to be_nil + end + + expect do + snippet_repository.multi_files_action(user, data, commit_opts) + end.not_to raise_error + + aggregate_failures do + data.each do |entry| + blob = blob_at(snippet, entry[:file_path]) + + expect(blob).not_to be_nil + expect(blob.path).to eq entry[:file_path] + expect(blob.data).to eq entry[:content] + end + end + end + + it 'tries to obtain an exclusive lease' do + expect(Gitlab::ExclusiveLease).to receive(:new).with("multi_files_action:#{snippet.id}", anything).and_call_original + + snippet_repository.multi_files_action(user, data, commit_opts) + end + + it 'cancels the lease when the method has finished' do + expect(Gitlab::ExclusiveLease).to receive(:cancel).with("multi_files_action:#{snippet.id}", anything).and_call_original + + snippet_repository.multi_files_action(user, data, commit_opts) + end + + it 'raises an error if the lease cannot be obtained' do + allow_next_instance_of(Gitlab::ExclusiveLease) do |instance| + allow(instance).to receive(:try_obtain).and_return false + end + + expect do + snippet_repository.multi_files_action(user, data, commit_opts) + end.to raise_error(described_class::CommitError) + end + + context 'with commit actions' do + let(:result) do + [{ action: :create }.merge(new_file), + { action: :move }.merge(move_file), + { action: :update }.merge(update_file)] + end + let(:repo) { double } + + before do + allow(snippet).to receive(:repository).and_return(repo) + allow(repo).to receive(:ls_files).and_return([]) + end + + it 'infers the commit action based on the parameters if not present' do + expect(repo).to receive(:multi_action).with(user, hash_including(actions: result)) + + snippet_repository.multi_files_action(user, data, commit_opts) + end + + context 'when commit actions are present' do + let(:file_action) { { file_path: 'foo.txt', content: 'foo', action: :foobar } } + let(:data) { [file_action] } + + it 'does not change commit action' do + expect(repo).to( + receive(:multi_action).with( + user, + hash_including(actions: array_including(hash_including(action: :foobar))))) + + snippet_repository.multi_files_action(user, data, commit_opts) + end + end + end + + context 'when files are not named' do + let(:data) do + [ + { + file_path: '', + content: 'foo', + action: :create + }, + { + file_path: '', + content: 'bar', + action: :create + }, + { + file_path: 'foo.txt', + content: 'bar', + action: :create + } + ] + end + + it 'sets a name for non named files' do + expect do + snippet_repository.multi_files_action(user, data, commit_opts) + end.not_to raise_error + + expect(snippet.repository.ls_files(nil)).to include('snippetfile1.txt', 'snippetfile2.txt', 'foo.txt') + end + end + end + + def blob_at(snippet, path) + snippet.repository.blob_at('master', path) + end + + def first_blob(snippet) + snippet.repository.blob_at('master', snippet.repository.ls_files(nil).first) + end end diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb index 7efc426c81d..16903d9d6d0 100644 --- a/spec/requests/api/project_snippets_spec.rb +++ b/spec/requests/api/project_snippets_spec.rb @@ -98,6 +98,36 @@ describe API::ProjectSnippets do } end + shared_examples 'project snippet repository actions' do + let(:snippet) { ProjectSnippet.find(json_response['id']) } + + it 'creates repository' do + subject + + expect(snippet.repository.exists?).to be_truthy + end + + it 'commit the files to the repository' do + subject + + blob = snippet.repository.blob_at('master', params[:file_name]) + + expect(blob.data).to eq params[:code] + end + + context 'when feature flag :version_snippets is disabled' do + it 'does not create snippet repository' do + stub_feature_flags(version_snippets: false) + + expect do + subject + end.to change { ProjectSnippet.count }.by(1) + + expect(snippet.repository_exists?).to be_falsey + end + end + end + context 'with a regular user' do let(:user) { create(:user) } @@ -118,6 +148,10 @@ describe API::ProjectSnippets do expect(snippet.file_name).to eq(params[:file_name]) expect(snippet.visibility_level).to eq(Snippet::INTERNAL) end + + it_behaves_like 'project snippet repository actions' do + subject { post api("/projects/#{project.id}/snippets/", user), params: params } + end end it 'creates a new snippet' do @@ -132,6 +166,10 @@ describe API::ProjectSnippets do expect(snippet.visibility_level).to eq(Snippet::PUBLIC) end + it_behaves_like 'project snippet repository actions' do + subject { post api("/projects/#{project.id}/snippets/", admin), params: params } + end + it 'creates a new snippet with content parameter' do params[:content] = params.delete(:code) diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index 21565265b99..cb2a0adc092 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -199,9 +199,13 @@ describe API::Snippets do end shared_examples 'snippet creation' do + let(:snippet) { Snippet.find(json_response["id"]) } + + subject { post api("/snippets/", user), params: params } + it 'creates a new snippet' do expect do - post api("/snippets/", user), params: params + subject end.to change { PersonalSnippet.count }.by(1) expect(response).to have_gitlab_http_status(201) @@ -210,6 +214,32 @@ describe API::Snippets do expect(json_response['file_name']).to eq(params[:file_name]) expect(json_response['visibility']).to eq(params[:visibility]) end + + it 'creates repository' do + subject + + expect(snippet.repository.exists?).to be_truthy + end + + it 'commit the files to the repository' do + subject + + blob = snippet.repository.blob_at('master', params[:file_name]) + + expect(blob.data).to eq params[:content] + end + + context 'when feature flag :version_snippets is disabled' do + it 'does not create snippet repository' do + stub_feature_flags(version_snippets: false) + + expect do + subject + end.to change { PersonalSnippet.count }.by(1) + + expect(snippet.repository_exists?).to be_falsey + end + end end context 'with restricted visibility settings' do diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index e503f1a4231..449c4c07b08 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -798,6 +798,11 @@ describe 'project routing' do end it_behaves_like 'redirecting a legacy project path', "/gitlab/gitlabhq/settings/repository", "/gitlab/gitlabhq/-/settings/repository" + + # TODO: remove this test as part of https://gitlab.com/gitlab-org/gitlab/issues/207079 (12.9) + it 'to ci_cd#create_deploy_token' do + expect(post('gitlab/gitlabhq/-/settings/repository/deploy_token/create')).to route_to('projects/settings/ci_cd#create_deploy_token', namespace_id: 'gitlab', project_id: 'gitlabhq') + end end describe Projects::TemplatesController, 'routing' do diff --git a/spec/services/groups/deploy_tokens/create_service_spec.rb b/spec/services/groups/deploy_tokens/create_service_spec.rb new file mode 100644 index 00000000000..20c609bc828 --- /dev/null +++ b/spec/services/groups/deploy_tokens/create_service_spec.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Groups::DeployTokens::CreateService do + it_behaves_like 'a deploy token creation service' do + let(:entity) { create(:group) } + let(:deploy_token_class) { GroupDeployToken } + end +end diff --git a/spec/services/projects/deploy_tokens/create_service_spec.rb b/spec/services/projects/deploy_tokens/create_service_spec.rb new file mode 100644 index 00000000000..5c3ada8af4e --- /dev/null +++ b/spec/services/projects/deploy_tokens/create_service_spec.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Projects::DeployTokens::CreateService do + it_behaves_like 'a deploy token creation service' do + let(:entity) { create(:project) } + let(:deploy_token_class) { ProjectDeployToken } + end +end diff --git a/spec/services/snippets/create_service_spec.rb b/spec/services/snippets/create_service_spec.rb index 37b203c2341..5c853f8b7d7 100644 --- a/spec/services/snippets/create_service_spec.rb +++ b/spec/services/snippets/create_service_spec.rb @@ -143,37 +143,102 @@ describe Snippets::CreateService do end end - shared_examples 'creates repository' do - it do + shared_examples 'creates repository and files' do + it 'creates repository' do subject - expect(snippet.repository_exists?).to be_truthy + expect(snippet.repository.exists?).to be_truthy + end + + it 'commit the files to the repository' do + subject + + blob = snippet.repository.blob_at('master', base_opts[:file_name]) + + expect(blob.data).to eq base_opts[:content] + end + + context 'when repository creation action fails' do + before do + allow_next_instance_of(Snippet) do |instance| + allow(instance).to receive(:create_repository).and_return(nil) + end + end + + it 'does not create the snippet' do + expect { subject }.not_to change { Snippet.count } + end + + it 'returns the error' do + expect(snippet.errors.full_messages).to include('Repository could not be created') + end + end + + context 'when the commit action fails' do + before do + allow_next_instance_of(SnippetRepository) do |instance| + allow(instance).to receive(:multi_files_action).and_raise(SnippetRepository::CommitError.new('foobar')) + end + end + + it 'does not create the snippet' do + expect { subject }.not_to change { Snippet.count } + end + + it 'does not create the repository' do + expect(snippet.repository_exists?).to be_falsey + end + + it 'destroys the existing repository' do + expect(Repositories::DestroyService).to receive(:new).and_call_original + + subject + end + + it 'returns the error' do + response = subject + + expect(response).to be_error + expect(response.payload[:snippet].errors.full_messages).to eq ['foobar'] + end end context 'when snippet creation fails' do let(:extra_opts) { { content: nil } } it 'does not create repository' do - subject + expect do + subject + end.not_to change(Snippet, :count) expect(snippet.repository_exists?).to be_falsey end end context 'when feature flag :version_snippets is disabled' do - it 'does not create snippet repository' do + before do stub_feature_flags(version_snippets: false) + end + it 'does not create snippet repository' do expect do subject end.to change(Snippet, :count).by(1) expect(snippet.repository_exists?).to be_falsey end + + it 'does not try to commit files' do + expect_next_instance_of(described_class) do |instance| + expect(instance).not_to receive(:create_commit) + end + + subject + end end end - context 'when Project Snippet' do + context 'when ProjectSnippet' do let_it_be(:project) { create(:project) } before do @@ -185,7 +250,7 @@ describe Snippets::CreateService do it_behaves_like 'spam check is performed' it_behaves_like 'snippet create data is tracked' it_behaves_like 'an error service response when save fails' - it_behaves_like 'creates repository' + it_behaves_like 'creates repository and files' end context 'when PersonalSnippet' do @@ -196,7 +261,7 @@ describe Snippets::CreateService do it_behaves_like 'spam check is performed' it_behaves_like 'snippet create data is tracked' it_behaves_like 'an error service response when save fails' - it_behaves_like 'creates repository' + it_behaves_like 'creates repository and files' end end end diff --git a/spec/services/deploy_tokens/create_service_spec.rb b/spec/support/services/deploy_token_shared_examples.rb index fbb66fe4cb7..b49f4743f7d 100644 --- a/spec/services/deploy_tokens/create_service_spec.rb +++ b/spec/support/services/deploy_token_shared_examples.rb @@ -1,14 +1,11 @@ # frozen_string_literal: true -require 'spec_helper' - -describe DeployTokens::CreateService do - let(:project) { create(:project) } +RSpec.shared_examples 'a deploy token creation service' do let(:user) { create(:user) } let(:deploy_token_params) { attributes_for(:deploy_token) } describe '#execute' do - subject { described_class.new(project, user, deploy_token_params).execute } + subject { described_class.new(entity, user, deploy_token_params).execute } context 'when the deploy token is valid' do it 'creates a new DeployToken' do @@ -16,7 +13,7 @@ describe DeployTokens::CreateService do end it 'creates a new ProjectDeployToken' do - expect { subject }.to change { ProjectDeployToken.count }.by(1) + expect { subject }.to change { deploy_token_class.count }.by(1) end it 'returns a DeployToken' do @@ -56,7 +53,7 @@ describe DeployTokens::CreateService do end it 'does not create a new ProjectDeployToken' do - expect { subject }.not_to change { ProjectDeployToken.count } + expect { subject }.not_to change { deploy_token_class.count } end end end diff --git a/spec/support/shared_examples/controllers/deploy_token_shared_examples.rb b/spec/support/shared_examples/controllers/deploy_token_shared_examples.rb new file mode 100644 index 00000000000..791eb0b68e0 --- /dev/null +++ b/spec/support/shared_examples/controllers/deploy_token_shared_examples.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'a created deploy token' do + let(:deploy_token_params) do + { + name: 'deployer_token', + expires_at: 1.month.from_now.to_date.to_s, + username: 'deployer', + read_repository: '1', + deploy_token_type: deploy_token_type + } + end + + subject(:create_deploy_token) { post :create_deploy_token, params: create_entity_params.merge({ deploy_token: deploy_token_params }) } + + it 'creates deploy token' do + expect { create_deploy_token }.to change { DeployToken.active.count }.by(1) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:show) + end +end diff --git a/spec/support/shared_examples/features/deploy_token_shared_examples.rb b/spec/support/shared_examples/features/deploy_token_shared_examples.rb new file mode 100644 index 00000000000..f358615ee9e --- /dev/null +++ b/spec/support/shared_examples/features/deploy_token_shared_examples.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'a deploy token in ci/cd settings' do + it 'view deploy tokens' do + within('.deploy-tokens') do + expect(page).to have_content(deploy_token.name) + expect(page).to have_content('read_repository') + expect(page).to have_content('read_registry') + end + end + + it 'add a new deploy token' do + fill_in 'deploy_token_name', with: 'new_deploy_key' + fill_in 'deploy_token_expires_at', with: (Date.today + 1.month).to_s + fill_in 'deploy_token_username', with: 'deployer' + check 'deploy_token_read_repository' + check 'deploy_token_read_registry' + click_button 'Create deploy token' + + expect(page).to have_content("Your new #{entity_type} deploy token has been created") + + within('.created-deploy-token-container') do + expect(page).to have_selector("input[name='deploy-token-user'][value='deployer']") + expect(page).to have_selector("input[name='deploy-token'][readonly='readonly']") + end + end +end diff --git a/spec/views/import/gitlab_projects/new.html.haml_spec.rb b/spec/views/import/gitlab_projects/new.html.haml_spec.rb index 953fcc6dc51..17636c99cbc 100644 --- a/spec/views/import/gitlab_projects/new.html.haml_spec.rb +++ b/spec/views/import/gitlab_projects/new.html.haml_spec.rb @@ -5,17 +5,18 @@ require 'spec_helper' describe 'import/gitlab_projects/new.html.haml' do include Devise::Test::ControllerHelpers - let(:user) { build_stubbed(:user, namespace: build_stubbed(:namespace)) } + let(:namespace) { build_stubbed(:namespace) } + let(:user) { build_stubbed(:user, namespace: namespace) } before do allow(view).to receive(:current_user).and_return(user) end context 'when the user has no other namespaces' do - it 'shows a namespace_id hidden field tag' do + it 'adds a namespace_id hidden field tag with the namespace id as value' do render - expect(rendered).to have_css('input[name="namespace_id"]', count: 1, visible: false) + expect(rendered).to have_css("input[name='namespace_id'][value='#{namespace.id}']", count: 1, visible: false) end end |