diff options
47 files changed, 1161 insertions, 93 deletions
diff --git a/.gitlab/issue_templates/Test plan.md b/.gitlab/issue_templates/Test plan.md new file mode 100644 index 00000000000..580fab206b3 --- /dev/null +++ b/.gitlab/issue_templates/Test plan.md @@ -0,0 +1,96 @@ +# Test Plan + +<!-- This issue outlines testing activities related to a particular issue or epic. + +[Here is an example test plan](https://gitlab.com/gitlab-org/gitlab-ce/issues/50353) + +This and other comments should be removed as you write the plan --> + +## Introduction + +<!-- Briefly outline what is being tested + +Mention the issue(s) this test plan is related to --> + +## Scope + +<!-- State any limits on aspects of the feature being tested +Outline the types of data to be included +Outline the types of tests to be performed (functional, security, performance, +database, automated, etc) --> + +## ACC Matrix + +<!-- Use the matrix below as a template to identify the Attributes, Components, and +Capabilities relevant to the scope of this test plan. Add or remove Attributes +and Components as required and list Capabilities in the next section + +Attributes (columns) are adverbs or adjectives that describe (at a high level) +the qualities testing is meant to ensure Components have. + +Components (rows) are nouns that define major parts of the product being tested. + +Capabilities link Attributes and Components. They are what your product needs to +do to make sure a Component fulfills an Attribute + +For more information see the [Google Testing Blog article about the 10 minute +test plan](https://testing.googleblog.com/2011/09/10-minute-test-plan.html) and +[this wiki page from an open-source tool that implements the ACC +model](https://code.google.com/archive/p/test-analytics/wikis/AccExplained.wiki). --> + +| | Simple | Secure | Responsive | Obvious | Stable | +|------------|:------:|:------:|:----------:|:-------:|:------:| +| Admin | | | | | | +| Groups | | | | | | +| Project | | | | | | +| Repository | | | | | | +| Issues | | | | | | +| MRs | | | | | | +| CI/CD | | | | | | +| Ops | | | | | | +| Registry | | | | | | +| Wiki | | | | | | +| Snippets | | | | | | +| Settings | | | | | | +| Tracking | | | | | | +| API | | | | | | + +## Capabilities + +<!-- Use the ACC matrix above to help you identify Capabilities at each relevant +intersection of Components and Attributes. + +Some features might be simple enough that they only involve one Component, while +more complex features could involve multiple or even all. + +Example (from https://gitlab.com/gitlab-org/gitlab-ce/issues/50353): +* Respository is + * Simple + * It's easy to select the desired file template + * It doesn't require unnecessary actions to save the change + * It's easy to undo the change after selecting a template + * Responsive + * The list of templates can be restricted to allow a user to find a specific template among many + * Once a template is selected the file content updates quickly and smoothly +--> + +## Test Plan + +<!-- If the scope is small enough you may not need to write a list of tests to +perform. It might be enough to use the Capabilities to guide your testing. + +If the feature is more complex, especially if it involves multiple Components, +briefly outline a set of tests here. When identifying tests to perform be sure +to consider risk. Note inherent/known levels of risk so that testing can focus +on high risk areas first. + +New end-to-end and integration tests (Selenium and API) should be added to the +[Test Coverage sheet](https://docs.google.com/spreadsheets/d/1RlLfXGboJmNVIPP9jgFV5sXIACGfdcFq1tKd7xnlb74/) + +Please note if automated tests already exist. + +When adding new automated tests, please keep [testing levels](https://docs.gitlab.com/ce/development/testing_guide/testing_levels.html) +in mind. +--> + +/label ~Quality
\ No newline at end of file diff --git a/app/assets/javascripts/vue_merge_request_widget/components/deployment.vue b/app/assets/javascripts/vue_merge_request_widget/components/deployment.vue index 21f21232596..d530ab2767b 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/deployment.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/deployment.vue @@ -1,5 +1,6 @@ <script> import Icon from '~/vue_shared/components/icon.vue'; +import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate.vue'; import timeagoMixin from '../../vue_shared/mixins/timeago'; import tooltip from '../../vue_shared/directives/tooltip'; import LoadingButton from '../../vue_shared/components/loading_button.vue'; @@ -16,6 +17,7 @@ export default { MemoryUsage, StatusIcon, Icon, + TooltipOnTruncate, }, directives: { tooltip, @@ -88,14 +90,20 @@ export default { <span> Deployed to </span> - <a - :href="deployment.url" - target="_blank" - rel="noopener noreferrer nofollow" - class="deploy-link js-deploy-meta" + <tooltip-on-truncate + :title="deployment.name" + truncate-target="child" + class="deploy-link label-truncate" > - {{ deployment.name }} - </a> + <a + :href="deployment.url" + target="_blank" + rel="noopener noreferrer nofollow" + class="js-deploy-meta" + > + {{ deployment.name }} + </a> + </tooltip-on-truncate> </template> <span v-tooltip diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.vue b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.vue index a4c2289c590..72bd28ae03f 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.vue @@ -1,18 +1,17 @@ <script> -import tooltip from '~/vue_shared/directives/tooltip'; -import { n__ } from '~/locale'; +import _ from 'underscore'; +import { n__, s__, sprintf } from '~/locale'; import { mergeUrlParams, webIDEUrl } from '~/lib/utils/url_utility'; import Icon from '~/vue_shared/components/icon.vue'; import clipboardButton from '~/vue_shared/components/clipboard_button.vue'; +import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate.vue'; export default { name: 'MRWidgetHeader', - directives: { - tooltip, - }, components: { Icon, clipboardButton, + TooltipOnTruncate, }, props: { mr: { @@ -24,8 +23,12 @@ export default { shouldShowCommitsBehindText() { return this.mr.divergedCommitsCount > 0; }, - commitsText() { - return n__('%d commit behind', '%d commits behind', this.mr.divergedCommitsCount); + commitsBehindText() { + return sprintf(s__('mrWidget|The source branch is %{commitsBehindLinkStart}%{commitsBehind}%{commitsBehindLinkEnd} the target branch'), { + commitsBehindLinkStart: `<a href="${_.escape(this.mr.targetBranchPath)}">`, + commitsBehind: n__('%d commit behind', '%d commits behind', this.mr.divergedCommitsCount), + commitsBehindLinkEnd: '</a>', + }, false); }, branchNameClipboardData() { // This supports code in app/assets/javascripts/copy_to_clipboard.js that @@ -36,12 +39,6 @@ export default { gfm: `\`${this.mr.sourceBranch}\``, }); }, - isSourceBranchLong() { - return this.isBranchTitleLong(this.mr.sourceBranch); - }, - isTargetBranchLong() { - return this.isBranchTitleLong(this.mr.targetBranch); - }, webIdePath() { return mergeUrlParams({ target_project: this.mr.sourceProjectFullPath !== this.mr.targetProjectFullPath ? @@ -49,11 +46,6 @@ export default { }, webIDEUrl(`/${this.mr.sourceProjectFullPath}/merge_requests/${this.mr.iid}`)); }, }, - methods: { - isBranchTitleLong(branchTitle) { - return branchTitle.length > 32; - }, - }, }; </script> <template> @@ -65,30 +57,21 @@ export default { <div class="normal"> <strong> {{ s__("mrWidget|Request to merge") }} - <span - :class="{ 'label-truncated': isSourceBranchLong }" - :title="isSourceBranchLong ? mr.sourceBranch : ''" - :v-tooltip="isSourceBranchLong" - class="label-branch js-source-branch" - data-placement="bottom" + <tooltip-on-truncate + :title="mr.sourceBranch" + truncate-target="child" + class="label-branch label-truncate js-source-branch" v-html="mr.sourceBranchLink" - > - </span> - - <clipboard-button + /><clipboard-button :text="branchNameClipboardData" :title="__('Copy branch name to clipboard')" css-class="btn-default btn-transparent btn-clipboard" /> - {{ s__("mrWidget|into") }} - - <span - :v-tooltip="isTargetBranchLong" - :class="{ 'label-truncatedtooltip': isTargetBranchLong }" - :title="isTargetBranchLong ? mr.targetBranch : ''" - class="label-branch" - data-placement="bottom" + <tooltip-on-truncate + :title="mr.targetBranch" + truncate-target="child" + class="label-branch label-truncate" > <a :href="mr.targetBranchTreePath" @@ -96,15 +79,13 @@ export default { > {{ mr.targetBranch }} </a> - </span> + </tooltip-on-truncate> </strong> <div v-if="shouldShowCommitsBehindText" class="diverged-commits-count" + v-html="commitsBehindText" > - <span class="monospace">{{ mr.sourceBranch }}</span> - is {{ commitsText }} - <span class="monospace">{{ mr.targetBranch }}</span> </div> </div> diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue index 4a3fd01fa39..fee41b239e8 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue @@ -3,6 +3,7 @@ import PipelineStage from '~/pipelines/components/stage.vue'; import CiIcon from '~/vue_shared/components/ci_icon.vue'; import Icon from '~/vue_shared/components/icon.vue'; +import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate.vue'; export default { name: 'MRWidgetPipeline', @@ -10,6 +11,7 @@ export default { PipelineStage, CiIcon, Icon, + TooltipOnTruncate, }, props: { pipeline: { @@ -30,6 +32,10 @@ export default { type: String, required: false, }, + sourceBranch: { + type: String, + required: false, + }, }, computed: { hasPipeline() { @@ -107,11 +113,12 @@ export default { > {{ pipeline.commit.short_id }}</a> on - <span - class="label-branch" + <tooltip-on-truncate + :title="sourceBranch" + truncate-target="child" + class="label-branch label-truncate" v-html="sourceBranchLink" - > - </span> + /> </template> </div> <div diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue index 80593d1f34a..dc6be025f11 100644 --- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue +++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue @@ -254,6 +254,7 @@ export default { :pipeline="mr.pipeline" :ci-status="mr.ciStatus" :has-ci="mr.hasCI" + :source-branch="mr.sourceBranch" :source-branch-link="mr.sourceBranchLink" /> <deployment diff --git a/app/assets/javascripts/vue_shared/components/stacked_progress_bar.vue b/app/assets/javascripts/vue_shared/components/stacked_progress_bar.vue index f44d361c47e..78fde463507 100644 --- a/app/assets/javascripts/vue_shared/components/stacked_progress_bar.vue +++ b/app/assets/javascripts/vue_shared/components/stacked_progress_bar.vue @@ -71,7 +71,11 @@ export default { }, methods: { getPercent(count) { - return roundOffFloat((count / this.totalCount) * 100, 1); + const percent = roundOffFloat((count / this.totalCount) * 100, 1); + if (percent > 0 && percent < 1) { + return '< 1'; + } + return percent; }, barStyle(percent) { return `width: ${percent}%;`; diff --git a/app/assets/javascripts/vue_shared/components/tooltip_on_truncate.vue b/app/assets/javascripts/vue_shared/components/tooltip_on_truncate.vue new file mode 100644 index 00000000000..125826da6c3 --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/tooltip_on_truncate.vue @@ -0,0 +1,67 @@ +<script> +import _ from 'underscore'; +import tooltip from '../directives/tooltip'; + +export default { + directives: { + tooltip, + }, + props: { + title: { + type: String, + required: false, + default: '', + }, + placement: { + type: String, + required: false, + default: 'top', + }, + truncateTarget: { + type: [String, Function], + required: false, + default: '', + }, + }, + data() { + return { + showTooltip: false, + }; + }, + mounted() { + const target = this.selectTarget(); + + if (target && target.scrollWidth > target.offsetWidth) { + this.showTooltip = true; + } + }, + methods: { + selectTarget() { + if (_.isFunction(this.truncateTarget)) { + return this.truncateTarget(this.$el); + } else if (this.truncateTarget === 'child') { + return this.$el.childNodes[0]; + } + + return this.$el; + }, + }, +}; +</script> + +<template> + <span + v-tooltip + v-if="showTooltip" + :title="title" + :data-placement="placement" + class="js-show-tooltip" + > + <slot></slot> + </span> + <span + v-else + > + <slot></slot> + </span> +</template> diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 5631d943984..7b8cad254c7 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -195,6 +195,7 @@ .ci-widget-content { display: flex; align-items: center; + flex: 1; } } @@ -222,6 +223,7 @@ .normal { flex: 1; + flex-basis: auto; } .capitalize { @@ -235,22 +237,23 @@ font-weight: normal; overflow: hidden; word-break: break-all; + } - &.label-truncated { - position: relative; - display: inline-block; - width: 250px; - margin-bottom: -3px; - white-space: nowrap; - text-overflow: clip; - line-height: 14px; - - &::after { - position: absolute; - content: '...'; - right: 0; - font-family: $regular-font; - background-color: $gray-light; + .deploy-link, + .label-branch { + &.label-truncate { + // NOTE: This selector targets its children because some of the HTML comes from + // 'source_branch_link'. Once this external HTML is no longer used, we could + // simplify this. + > a, + > span { + display: inline-block; + max-width: 12.5em; + margin-bottom: -3px; + white-space: nowrap; + text-overflow: ellipsis; + line-height: 14px; + overflow: hidden; } } } @@ -582,7 +585,7 @@ @include media-breakpoint-down(md) { flex-direction: column; - align-items: flex-start; + align-items: stretch; .branch-actions { margin-top: 16px; @@ -593,13 +596,13 @@ .branch-actions { align-self: center; margin-left: $gl-padding; + white-space: nowrap; } } } .diverged-commits-count { color: $gl-text-color-secondary; - font-size: 12px; } } @@ -918,7 +921,7 @@ flex: 1; flex-direction: row; - @include media-breakpoint-down(md) { + @include media-breakpoint-down(sm) { flex-direction: column; .stage-cell .stage-container { diff --git a/app/helpers/import_helper.rb b/app/helpers/import_helper.rb index 4664b1728c4..c65f1565425 100644 --- a/app/helpers/import_helper.rb +++ b/app/helpers/import_helper.rb @@ -5,6 +5,10 @@ module ImportHelper false end + def sanitize_project_name(name) + name.gsub(/[^\w\-]/, '-') + end + def import_project_target(owner, name) namespace = current_user.can_create_group? ? owner : current_user.namespace_path "#{namespace}/#{name}" diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 6b4079b4113..18b3badda8d 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -447,7 +447,7 @@ module ProjectsHelper end def project_permissions_panel_data(project) - data = { + { currentSettings: project_permissions_settings(project), canChangeVisibilityLevel: can_change_visibility_level?(project, current_user), allowedVisibilityOptions: project_allowed_visibility_levels(project), @@ -457,8 +457,10 @@ module ProjectsHelper lfsAvailable: Gitlab.config.lfs.enabled, lfsHelpPath: help_page_path('workflow/lfs/manage_large_binaries_with_git_lfs') } + end - data.to_json.html_safe + def project_permissions_panel_data_json(project) + project_permissions_panel_data(project).to_json.html_safe end def project_allowed_visibility_levels(project) diff --git a/app/models/project.rb b/app/models/project.rb index 8f631d7f0ed..178d70757a1 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -2072,13 +2072,19 @@ class Project < ActiveRecord::Base private def rename_or_migrate_repository! - if Gitlab::CurrentSettings.hashed_storage_enabled? && storage_version != LATEST_STORAGE_VERSION + if Gitlab::CurrentSettings.hashed_storage_enabled? && + storage_upgradable? && + Feature.disabled?(:skip_hashed_storage_upgrade) # kill switch in case we need to disable upgrade behavior ::Projects::HashedStorageMigrationService.new(self, full_path_was).execute else storage.rename_repo end end + def storage_upgradable? + storage_version != LATEST_STORAGE_VERSION + end + def after_rename_repository(full_path_before, path_before) execute_rename_repository_hooks!(full_path_before) diff --git a/app/models/protected_tag.rb b/app/models/protected_tag.rb index a36f0d36262..94746141945 100644 --- a/app/models/protected_tag.rb +++ b/app/models/protected_tag.rb @@ -4,6 +4,8 @@ class ProtectedTag < ActiveRecord::Base include Gitlab::ShellAdapter include ProtectedRef + validates :name, uniqueness: { scope: :project_id } + protected_ref_access_levels :create def self.protected?(project, ref_name) diff --git a/app/views/import/_githubish_status.html.haml b/app/views/import/_githubish_status.html.haml index f0d1e837317..f4a29ed18dc 100644 --- a/app/views/import/_githubish_status.html.haml +++ b/app/views/import/_githubish_status.html.haml @@ -45,7 +45,7 @@ = text_field_tag :path, current_user.namespace_path, class: "input-group-text input-large form-control", tabindex: 1, disabled: true %span.input-group-prepend .input-group-text / - = text_field_tag :path, repo.name, class: "input-mini form-control", tabindex: 2, autofocus: true, required: true + = text_field_tag :path, sanitize_project_name(repo.name), class: "input-mini form-control", tabindex: 2, autofocus: true, required: true %td.import-actions.job-status = button_tag class: "btn btn-import js-add-to-import" do = has_ci_cd_only_params? ? _('Connect') : _('Import') diff --git a/app/views/import/bitbucket/status.html.haml b/app/views/import/bitbucket/status.html.haml index a75b7aa9dd2..3b1b5e55302 100644 --- a/app/views/import/bitbucket/status.html.haml +++ b/app/views/import/bitbucket/status.html.haml @@ -63,7 +63,7 @@ = text_field_tag :path, current_user.namespace_path, class: "input-group-text input-large form-control", tabindex: 1, disabled: true %span.input-group-prepend .input-group-text / - = text_field_tag :path, repo.name, class: "input-mini form-control", tabindex: 2, autofocus: true, required: true + = text_field_tag :path, sanitize_project_name(repo.slug), class: "input-mini form-control", tabindex: 2, autofocus: true, required: true %td.import-actions.job-status = button_tag class: 'btn btn-import js-add-to-import' do = _('Import') diff --git a/app/views/import/bitbucket_server/status.html.haml b/app/views/import/bitbucket_server/status.html.haml index 3d05a5e696f..ae09e0dfa18 100644 --- a/app/views/import/bitbucket_server/status.html.haml +++ b/app/views/import/bitbucket_server/status.html.haml @@ -61,7 +61,7 @@ = text_field_tag :path, current_user.namespace_path, class: "input-group-text input-large form-control", tabindex: 1, disabled: true %span.input-group-prepend .input-group-text / - = text_field_tag :path, repo.name, class: "input-mini form-control", tabindex: 2, autofocus: true, required: true + = text_field_tag :path, sanitize_project_name(repo.slug), class: "input-mini form-control", tabindex: 2, autofocus: true, required: true %td.import-actions.job-status = button_tag class: 'btn btn-import js-add-to-import' do Import diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml index 30544dde451..e37a444c1c9 100644 --- a/app/views/projects/edit.html.haml +++ b/app/views/projects/edit.html.haml @@ -78,7 +78,7 @@ = form_for [@project.namespace.becomes(Namespace), @project], remote: true, html: { multipart: true, class: "sharing-permissions-form" }, authenticity_token: true do |f| %input{ name: 'update_section', type: 'hidden', value: 'js-shared-permissions' } -# haml-lint:disable InlineJavaScript - %script.js-project-permissions-form-data{ type: "application/json" }= project_permissions_panel_data(@project) + %script.js-project-permissions-form-data{ type: "application/json" }= project_permissions_panel_data_json(@project) .js-project-permissions-form = f.submit 'Save changes', class: "btn btn-save" diff --git a/changelogs/unreleased/49110-update-mr-widget-styles.yml b/changelogs/unreleased/49110-update-mr-widget-styles.yml new file mode 100644 index 00000000000..e54866a0908 --- /dev/null +++ b/changelogs/unreleased/49110-update-mr-widget-styles.yml @@ -0,0 +1,5 @@ +--- +title: Truncate branch names and update "commits behind" text in MR page +merge_request: 21206 +author: +type: changed diff --git a/changelogs/unreleased/50345-hashed-storage-feature-flag.yml b/changelogs/unreleased/50345-hashed-storage-feature-flag.yml new file mode 100644 index 00000000000..4c5182b843b --- /dev/null +++ b/changelogs/unreleased/50345-hashed-storage-feature-flag.yml @@ -0,0 +1,5 @@ +--- +title: Feature flag to disable Hashed Storage migration when renaming a repository +merge_request: 21291 +author: +type: added diff --git a/changelogs/unreleased/6028-show-generic-percent-stacked-progress-bar.yml b/changelogs/unreleased/6028-show-generic-percent-stacked-progress-bar.yml new file mode 100644 index 00000000000..94098dd0144 --- /dev/null +++ b/changelogs/unreleased/6028-show-generic-percent-stacked-progress-bar.yml @@ -0,0 +1,6 @@ +--- +title: Show '< 1%' when percent value evaluated is less than 1 on Stacked Progress + Bar +merge_request: 21306 +author: +type: fixed diff --git a/changelogs/unreleased/api-protected-tags.yml b/changelogs/unreleased/api-protected-tags.yml new file mode 100644 index 00000000000..6e7ecf24b6e --- /dev/null +++ b/changelogs/unreleased/api-protected-tags.yml @@ -0,0 +1,5 @@ +--- +title: 'API: Protected tags' +merge_request: 14986 +author: Robert Schilling +type: added diff --git a/changelogs/unreleased/sh-sanitize-project-import-names.yml b/changelogs/unreleased/sh-sanitize-project-import-names.yml new file mode 100644 index 00000000000..6e0284bda08 --- /dev/null +++ b/changelogs/unreleased/sh-sanitize-project-import-names.yml @@ -0,0 +1,5 @@ +--- +title: Use slugs for default project path and sanitize names before import +merge_request: 21367 +author: +type: fixed diff --git a/danger/changelog/Dangerfile b/danger/changelog/Dangerfile index a1f94dc6004..713ed95a04c 100644 --- a/danger/changelog/Dangerfile +++ b/danger/changelog/Dangerfile @@ -53,9 +53,11 @@ end changelog_needed = (gitlab.mr_labels & NO_CHANGELOG_LABELS).empty? changelog_found = git.added_files.find { |path| path =~ %r{\A(ee/)?(changelogs/unreleased)(-ee)?/} } +mr_title = gitlab.mr_json["title"].gsub(/^WIP: */, '') + if git.modified_files.include?("CHANGELOG.md") fail "**CHANGELOG.md was edited.** Please remove the additions and create a CHANGELOG entry.\n\n" + - format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: gitlab.mr_json["title"], labels: presented_no_changelog_labels) + format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: mr_title, labels: presented_no_changelog_labels) end if changelog_needed @@ -63,6 +65,6 @@ if changelog_needed check_changelog(changelog_found) else warn "**[CHANGELOG missing](https://docs.gitlab.com/ce/development/changelog.html).**\n\n" + - format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: gitlab.mr_json["title"], labels: presented_no_changelog_labels) + format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: mr_title, labels: presented_no_changelog_labels) end end diff --git a/db/migrate/20180711103851_drop_duplicate_protected_tags.rb b/db/migrate/20180711103851_drop_duplicate_protected_tags.rb new file mode 100644 index 00000000000..8fa2137551e --- /dev/null +++ b/db/migrate/20180711103851_drop_duplicate_protected_tags.rb @@ -0,0 +1,45 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class DropDuplicateProtectedTags < ActiveRecord::Migration + DOWNTIME = false + + disable_ddl_transaction! + + BATCH_SIZE = 1000 + + class Project < ActiveRecord::Base + self.table_name = 'projects' + + include ::EachBatch + end + + class ProtectedTag < ActiveRecord::Base + self.table_name = 'protected_tags' + end + + def up + Project.each_batch(of: BATCH_SIZE) do |projects| + ids = ProtectedTag + .where(project_id: projects) + .group(:name, :project_id) + .select('max(id)') + + tags = ProtectedTag + .where(project_id: projects) + .where.not(id: ids) + + if Gitlab::Database.postgresql? + tags.delete_all + else + # Workaround needed for MySQL + sql = "SELECT id FROM (#{tags.to_sql}) protected_tags" + + ProtectedTag.where("id IN (#{sql})").delete_all # rubocop:disable GitlabSecurity/SqlInjection + end + end + end + + def down + end +end diff --git a/db/migrate/20180711103922_add_protected_tags_index.rb b/db/migrate/20180711103922_add_protected_tags_index.rb new file mode 100644 index 00000000000..7ed2258ebaf --- /dev/null +++ b/db/migrate/20180711103922_add_protected_tags_index.rb @@ -0,0 +1,18 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddProtectedTagsIndex < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :protected_tags, [:project_id, :name], unique: true + end + + def down + remove_concurrent_index :protected_tags, [:project_id, :name] + end +end diff --git a/db/schema.rb b/db/schema.rb index f5ce7df60e8..380d4e49ddf 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1741,6 +1741,7 @@ ActiveRecord::Schema.define(version: 20180816193530) do t.datetime "updated_at", null: false end + add_index "protected_tags", ["project_id", "name"], name: "index_protected_tags_on_project_id_and_name", unique: true, using: :btree add_index "protected_tags", ["project_id"], name: "index_protected_tags_on_project_id", using: :btree create_table "push_event_payloads", id: false, force: :cascade do |t| diff --git a/doc/api/README.md b/doc/api/README.md index 45e926d3b6b..e2a6e87a2c3 100644 --- a/doc/api/README.md +++ b/doc/api/README.md @@ -53,6 +53,7 @@ following locations: - [Project Members](members.md) - [Project Snippets](project_snippets.md) - [Protected Branches](protected_branches.md) +- [Protected Tags](protected_tags.md) - [Repositories](repositories.md) - [Repository Files](repository_files.md) - [Runners](runners.md) diff --git a/doc/api/protected_tags.md b/doc/api/protected_tags.md new file mode 100644 index 00000000000..aa750e467f8 --- /dev/null +++ b/doc/api/protected_tags.md @@ -0,0 +1,128 @@ +# Protected tags API + +>**Note:** This feature was introduced in GitLab 11.3 + +**Valid access levels** + +Currently, these levels are recognized: +``` +0 => No access +30 => Developer access +40 => Maintainer access +``` + +## List protected tags + +Gets a list of protected tags from a project. +This function takes pagination parameters `page` and `per_page` to restrict the list of protected tags. + +``` +GET /projects/:id/protected_tags +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | + +```bash +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" 'https://gitlab.example.com/api/v4/projects/5/protected_tags' +``` + +Example response: + +```json +[ + { + "name": "release-1-0", + "create_access_levels": [ + { + "access_level": 40, + "access_level_description": "Maintainers" + } + ] + }, + ... +] +``` + +## Get a single protected tag or wildcard protected tag + +Gets a single protected tag or wildcard protected tag. +The pagination parameters `page` and `per_page` can be used to restrict the list of protected tags. + +``` +GET /projects/:id/protected_tags/:name +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | +| `name` | string | yes | The name of the tag or wildcard | + +```bash +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" 'https://gitlab.example.com/api/v4/projects/5/protected_tags/release-1-0' +``` + +Example response: + +```json +{ + "name": "release-1-0", + "create_access_levels": [ + { + "access_level": 40, + "access_level_description": "Maintainers" + } + ] +} +``` + +## Protect repository tags + +Protects a single repository tag or several project repository +tags using a wildcard protected tag. + +``` +POST /projects/:id/protected_tags +``` + +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" 'https://gitlab.example.com/api/v4/projects/5/protected_tags?name=*-stable&create_access_level=30' +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | +| `name` | string | yes | The name of the tag or wildcard | +| `create_access_level` | string | no | Access levels allowed to create (defaults: `40`, maintainer access level) | + +Example response: + +```json +{ + "name": "*-stable", + "create_access_levels": [ + { + "access_level": 30, + "access_level_description": "Developers + Maintainers" + } + ] +} +``` + +## Unprotect repository tags + +Unprotects the given protected tag or wildcard protected tag. + +``` +DELETE /projects/:id/protected_tags/:name +``` + +```bash +curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" 'https://gitlab.example.com/api/v4/projects/5/protected_tags/*-stable' +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | +| `name` | string | yes | The name of the tag | diff --git a/doc/development/feature_flags.md b/doc/development/feature_flags.md index 09ea8c05be6..702caacc74f 100644 --- a/doc/development/feature_flags.md +++ b/doc/development/feature_flags.md @@ -57,3 +57,15 @@ end Features that are developed and are intended to be merged behind a feature flag should not include a changelog entry. The entry should be added in the merge request removing the feature flags. + +### Specs + +In the test environment `Feature.enabled?` is stubbed to always respond to `true`, +so we make sure behavior under feature flag doesn't go untested in some non-specific +contexts. + +If you need to test the feature flag in a different state, you need to stub it with: + +```ruby +stub_feature_flags(my_feature_flag: false) +``` diff --git a/lib/api/api.rb b/lib/api/api.rb index e2ad3c5f4e3..c000666d992 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -99,12 +99,13 @@ module API mount ::API::Features mount ::API::Files mount ::API::GroupBoards - mount ::API::Groups mount ::API::GroupMilestones + mount ::API::Groups + mount ::API::GroupVariables mount ::API::Internal mount ::API::Issues - mount ::API::Jobs mount ::API::JobArtifacts + mount ::API::Jobs mount ::API::Keys mount ::API::Labels mount ::API::Lint @@ -122,11 +123,12 @@ module API mount ::API::ProjectExport mount ::API::ProjectImport mount ::API::ProjectHooks - mount ::API::Projects mount ::API::ProjectMilestones + mount ::API::Projects mount ::API::ProjectSnapshots mount ::API::ProjectSnippets mount ::API::ProtectedBranches + mount ::API::ProtectedTags mount ::API::Repositories mount ::API::Runner mount ::API::Runners @@ -143,7 +145,6 @@ module API mount ::API::Triggers mount ::API::Users mount ::API::Variables - mount ::API::GroupVariables mount ::API::Version mount ::API::Wikis diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 06262f0f991..95b25d7351a 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -429,6 +429,11 @@ module API expose :merge_access_levels, using: Entities::ProtectedRefAccess end + class ProtectedTag < Grape::Entity + expose :name + expose :create_access_levels, using: Entities::ProtectedRefAccess + end + class Milestone < Grape::Entity expose :id, :iid expose :project_id, if: -> (entity, options) { entity&.project_id } diff --git a/lib/api/protected_tags.rb b/lib/api/protected_tags.rb new file mode 100644 index 00000000000..bf0a7184e1c --- /dev/null +++ b/lib/api/protected_tags.rb @@ -0,0 +1,79 @@ +module API + class ProtectedTags < Grape::API + include PaginationParams + + TAG_ENDPOINT_REQUIREMENTS = API::PROJECT_ENDPOINT_REQUIREMENTS.merge(name: API::NO_SLASH_URL_PART_REGEX) + + before { authorize_admin_project } + + params do + requires :id, type: String, desc: 'The ID of a project' + end + resource :projects, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do + desc "Get a project's protected tags" do + detail 'This feature was introduced in GitLab 11.3.' + success Entities::ProtectedTag + end + params do + use :pagination + end + get ':id/protected_tags' do + protected_tags = user_project.protected_tags.preload(:create_access_levels) + + present paginate(protected_tags), with: Entities::ProtectedTag, project: user_project + end + + desc 'Get a single protected tag' do + detail 'This feature was introduced in GitLab 11.3.' + success Entities::ProtectedTag + end + params do + requires :name, type: String, desc: 'The name of the tag or wildcard' + end + get ':id/protected_tags/:name', requirements: TAG_ENDPOINT_REQUIREMENTS do + protected_tag = user_project.protected_tags.find_by!(name: params[:name]) + + present protected_tag, with: Entities::ProtectedTag, project: user_project + end + + desc 'Protect a single tag or wildcard' do + detail 'This feature was introduced in GitLab 11.3.' + success Entities::ProtectedTag + end + params do + requires :name, type: String, desc: 'The name of the protected tag' + optional :create_access_level, type: Integer, default: Gitlab::Access::MAINTAINER, + values: ProtectedRefAccess::ALLOWED_ACCESS_LEVELS, + desc: 'Access levels allowed to create (defaults: `40`, maintainer access level)' + end + post ':id/protected_tags' do + protected_tags_params = { + name: params[:name], + create_access_levels_attributes: [{ access_level: params[:create_access_level] }] + } + + protected_tag = ::ProtectedTags::CreateService.new(user_project, + current_user, + protected_tags_params).execute + + if protected_tag.persisted? + present protected_tag, with: Entities::ProtectedTag, project: user_project + else + render_api_error!(protected_tag.errors.full_messages, 422) + end + end + + desc 'Unprotect a single tag' do + detail 'This feature was introduced in GitLab 11.3.' + end + params do + requires :name, type: String, desc: 'The name of the protected tag' + end + delete ':id/protected_tags/:name', requirements: TAG_ENDPOINT_REQUIREMENTS do + protected_tag = user_project.protected_tags.find_by!(name: params[:name]) + + destroy_conditionally!(protected_tag) + end + end + end +end diff --git a/lib/feature.rb b/lib/feature.rb index 09c5ef3ad94..24dbcb32fc0 100644 --- a/lib/feature.rb +++ b/lib/feature.rb @@ -47,7 +47,8 @@ class Feature end def disabled?(key, thing = nil) - !enabled?(key, thing) + # we need to make different method calls to make it easy to mock / define expectations in test mode + thing.nil? ? !enabled?(key) : !enabled?(key, thing) end def enable(key, thing = true) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index f348f66ae17..0bf552d6084 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -6706,6 +6706,9 @@ msgstr "" msgid "mrWidget|The source branch has been removed" msgstr "" +msgid "mrWidget|The source branch is %{commitsBehindLinkStart}%{commitsBehind}%{commitsBehindLinkEnd} the target branch" +msgstr "" + msgid "mrWidget|The source branch is being removed" msgstr "" @@ -83,6 +83,7 @@ module QA # Test scenario entrypoints. # module Test + autoload :Instance, 'qa/scenario/test/instance' module Instance autoload :All, 'qa/scenario/test/instance/all' autoload :Smoke, 'qa/scenario/test/instance/smoke' diff --git a/qa/qa/page/project/import/github.rb b/qa/qa/page/project/import/github.rb index 36567927194..1a410a0f8a5 100644 --- a/qa/qa/page/project/import/github.rb +++ b/qa/qa/page/project/import/github.rb @@ -14,7 +14,7 @@ module QA element :project_import_row, 'data: { qa: { repo_path: repo.full_name } }' element :project_namespace_select element :project_namespace_field, 'select_tag :namespace_id' - element :project_path_field, 'text_field_tag :path, repo.name' + element :project_path_field, 'text_field_tag :path, sanitize_project_name(repo.name)' element :import_button, "_('Import')" end diff --git a/qa/qa/scenario/test/instance.rb b/qa/qa/scenario/test/instance.rb new file mode 100644 index 00000000000..b9a73d3ffb8 --- /dev/null +++ b/qa/qa/scenario/test/instance.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module QA + module Scenario + module Test + # This class exists for back-compatibility so that gitlab-qa can continue + # to call Test::Instance instead of Test::Instance::All until at least + # the current latest GitLab version has the Test::Instance::All class. + # As of Aug, 22nd 2018. Only GitLab >= 11.3 has this class. + module Instance + include Bootable + + def self.perform(*args) + self.tap do |scenario| + yield scenario if block_given? + break scenario.do_perform(*args) + end + end + + def self.do_perform(address, *rspec_options) + Runtime::Scenario.define(:gitlab_address, address) + + Specs::Runner.perform do |specs| + specs.tty = true + specs.options = + if rspec_options.any? + rspec_options + else + ::File.expand_path('../specs/features', __dir__) + end + end + end + end + end + end +end diff --git a/spec/helpers/import_helper_spec.rb b/spec/helpers/import_helper_spec.rb index 033155617c6..cb0ea4e26ba 100644 --- a/spec/helpers/import_helper_spec.rb +++ b/spec/helpers/import_helper_spec.rb @@ -1,6 +1,16 @@ require 'rails_helper' describe ImportHelper do + describe '#sanitize_project_name' do + it 'removes whitespace' do + expect(helper.sanitize_project_name('my test repo')).to eq('my-test-repo') + end + + it 'removes disallowed characters' do + expect(helper.sanitize_project_name('Test&me$over*h_ere')).to eq('Test-me-over-h_ere') + end + end + describe '#import_project_target' do let(:user) { create(:user) } diff --git a/spec/javascripts/helpers/vue_mount_component_helper.js b/spec/javascripts/helpers/vue_mount_component_helper.js index 1057f0aca3e..6848c95d95d 100644 --- a/spec/javascripts/helpers/vue_mount_component_helper.js +++ b/spec/javascripts/helpers/vue_mount_component_helper.js @@ -1,3 +1,5 @@ +import Vue from 'vue'; + const mountComponent = (Component, props = {}, el = null) => new Component({ propsData: props, @@ -25,4 +27,12 @@ export const mountComponentWithSlots = (Component, { props, slots }) => { return component.$mount(); }; +/** + * Mount a component with the given render method. + * + * This helps with inserting slots that need to be compiled. + */ +export const mountComponentWithRender = (render, el = null) => + mountComponent(Vue.extend({ render }), {}, el); + export default mountComponent; diff --git a/spec/javascripts/vue_mr_widget/components/mr_widget_header_spec.js b/spec/javascripts/vue_mr_widget/components/mr_widget_header_spec.js index 8ac2f26979b..0e42537faed 100644 --- a/spec/javascripts/vue_mr_widget/components/mr_widget_header_spec.js +++ b/spec/javascripts/vue_mr_widget/components/mr_widget_header_spec.js @@ -45,7 +45,7 @@ describe('MRWidgetHeader', () => { }); }); - describe('commitsText', () => { + describe('commitsBehindText', () => { it('returns singular when there is one commit', () => { vm = mountComponent(Component, { mr: { @@ -53,11 +53,12 @@ describe('MRWidgetHeader', () => { sourceBranch: 'mr-widget-refactor', sourceBranchLink: '<a href="/foo/bar/mr-widget-refactor">Link</a>', targetBranch: 'master', + targetBranchPath: '/foo/bar/master', statusPath: 'abc', }, }); - expect(vm.commitsText).toEqual('1 commit behind'); + expect(vm.commitsBehindText).toEqual('The source branch is <a href="/foo/bar/master">1 commit behind</a> the target branch'); }); it('returns plural when there is more than one commit', () => { @@ -67,11 +68,12 @@ describe('MRWidgetHeader', () => { sourceBranch: 'mr-widget-refactor', sourceBranchLink: '<a href="/foo/bar/mr-widget-refactor">Link</a>', targetBranch: 'master', + targetBranchPath: '/foo/bar/master', statusPath: 'abc', }, }); - expect(vm.commitsText).toEqual('2 commits behind'); + expect(vm.commitsBehindText).toEqual('The source branch is <a href="/foo/bar/master">2 commits behind</a> the target branch'); }); }); }); @@ -280,9 +282,9 @@ describe('MRWidgetHeader', () => { }); it('renders diverged commits info', () => { - expect(vm.$el.querySelector('.diverged-commits-count').textContent).toMatch( - /(mr-widget-refactor[\s\S]+?is 12 commits behind[\s\S]+?master)/, - ); + expect(vm.$el.querySelector('.diverged-commits-count').textContent).toEqual('The source branch is 12 commits behind the target branch'); + expect(vm.$el.querySelector('.diverged-commits-count a').textContent).toEqual('12 commits behind'); + expect(vm.$el.querySelector('.diverged-commits-count a')).toHaveAttr('href', vm.mr.targetBranchPath); }); }); }); diff --git a/spec/javascripts/vue_shared/components/stacked_progress_bar_spec.js b/spec/javascripts/vue_shared/components/stacked_progress_bar_spec.js index 076d940961d..f1fe2e996fc 100644 --- a/spec/javascripts/vue_shared/components/stacked_progress_bar_spec.js +++ b/spec/javascripts/vue_shared/components/stacked_progress_bar_spec.js @@ -44,7 +44,11 @@ describe('StackedProgressBarComponent', () => { }); it('returns percentage with decimal place from provided count based on `totalCount`', () => { - expect(vm.getPercent(10)).toBe(0.2); + expect(vm.getPercent(67)).toBe(1.3); + }); + + it('returns percentage as `< 1` from provided count based on `totalCount` when evaluated value is less than 1', () => { + expect(vm.getPercent(10)).toBe('< 1'); }); }); diff --git a/spec/javascripts/vue_shared/components/tooltip_on_truncate_spec.js b/spec/javascripts/vue_shared/components/tooltip_on_truncate_spec.js new file mode 100644 index 00000000000..8465757deb6 --- /dev/null +++ b/spec/javascripts/vue_shared/components/tooltip_on_truncate_spec.js @@ -0,0 +1,162 @@ +import { mountComponentWithRender } from 'spec/helpers/vue_mount_component_helper'; +import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate.vue'; + +const TEST_TITLE = 'lorem-ipsum-dolar-sit-amit-consectur-adipiscing-elit-sed-do'; +const CLASS_SHOW_TOOLTIP = 'js-show-tooltip'; +const STYLE_TRUNCATED = { + display: 'inline-block', + 'max-width': '20px', +}; +const STYLE_NORMAL = { + display: 'inline-block', + 'max-width': '1000px', +}; + +function mountTooltipOnTruncate(options, createChildren) { + return mountComponentWithRender(h => h(TooltipOnTruncate, options, createChildren(h)), '#app'); +} + +describe('TooltipOnTruncate component', () => { + let vm; + + beforeEach(() => { + const el = document.createElement('div'); + el.id = 'app'; + document.body.appendChild(el); + }); + + afterEach(() => { + vm.$destroy(); + }); + + describe('with default target', () => { + it('renders tooltip if truncated', done => { + const options = { + style: STYLE_TRUNCATED, + props: { + title: TEST_TITLE, + }, + }; + + vm = mountTooltipOnTruncate(options, () => [TEST_TITLE]); + + vm.$nextTick() + .then(() => { + expect(vm.$el).toHaveClass(CLASS_SHOW_TOOLTIP); + expect(vm.$el).toHaveData('original-title', TEST_TITLE); + expect(vm.$el).toHaveData('placement', 'top'); + }) + .then(done) + .catch(done.fail); + }); + + it('does not render tooltip if normal', done => { + const options = { + style: STYLE_NORMAL, + props: { + title: TEST_TITLE, + }, + }; + + vm = mountTooltipOnTruncate(options, () => [TEST_TITLE]); + + vm.$nextTick() + .then(() => { + expect(vm.$el).not.toHaveClass(CLASS_SHOW_TOOLTIP); + }) + .then(done) + .catch(done.fail); + }); + }); + + describe('with child target', () => { + it('renders tooltip if truncated', done => { + const options = { + style: STYLE_NORMAL, + props: { + title: TEST_TITLE, + truncateTarget: 'child', + }, + }; + + vm = mountTooltipOnTruncate(options, (h) => [ + h('a', { style: STYLE_TRUNCATED }, TEST_TITLE), + ]); + + vm.$nextTick() + .then(() => { + expect(vm.$el).toHaveClass(CLASS_SHOW_TOOLTIP); + }) + .then(done) + .catch(done.fail); + }); + + it('does not render tooltip if normal', done => { + const options = { + props: { + title: TEST_TITLE, + truncateTarget: 'child', + }, + }; + + vm = mountTooltipOnTruncate(options, (h) => [ + h('a', { style: STYLE_NORMAL }, TEST_TITLE), + ]); + + vm.$nextTick() + .then(() => { + expect(vm.$el).not.toHaveClass(CLASS_SHOW_TOOLTIP); + }) + .then(done) + .catch(done.fail); + }); + }); + + describe('with fn target', () => { + it('renders tooltip if truncated', done => { + const options = { + style: STYLE_NORMAL, + props: { + title: TEST_TITLE, + truncateTarget: (el) => el.childNodes[1], + }, + }; + + vm = mountTooltipOnTruncate(options, (h) => [ + h('a', { style: STYLE_NORMAL }, TEST_TITLE), + h('span', { style: STYLE_TRUNCATED }, TEST_TITLE), + ]); + + vm.$nextTick() + .then(() => { + expect(vm.$el).toHaveClass(CLASS_SHOW_TOOLTIP); + }) + .then(done) + .catch(done.fail); + }); + }); + + describe('placement', () => { + it('sets data-placement when tooltip is rendered', done => { + const options = { + props: { + title: TEST_TITLE, + truncateTarget: 'child', + placement: 'bottom', + }, + }; + + vm = mountTooltipOnTruncate(options, (h) => [ + h('a', { style: STYLE_TRUNCATED }, TEST_TITLE), + ]); + + vm.$nextTick() + .then(() => { + expect(vm.$el).toHaveClass(CLASS_SHOW_TOOLTIP); + expect(vm.$el).toHaveData('placement', options.props.placement); + }) + .then(done) + .catch(done.fail); + }); + }); +}); diff --git a/spec/lib/feature_spec.rb b/spec/lib/feature_spec.rb index f313e675654..8bb5a843484 100644 --- a/spec/lib/feature_spec.rb +++ b/spec/lib/feature_spec.rb @@ -1,6 +1,13 @@ require 'spec_helper' describe Feature do + before do + # We mock all calls to .enabled? to return true in order to force all + # specs to run the feature flag gated behavior, but here we need a clean + # behavior from the class + allow(described_class).to receive(:enabled?).and_call_original + end + describe '.get' do let(:feature) { double(:feature) } let(:key) { 'my_feature' } @@ -106,4 +113,63 @@ describe Feature do it_behaves_like 'a memoized Flipper instance' end end + + describe '.enabled?' do + it 'returns false for undefined feature' do + expect(described_class.enabled?(:some_random_feature_flag)).to be_falsey + end + + it 'returns false for existing disabled feature in the database' do + described_class.disable(:disabled_feature_flag) + + expect(described_class.enabled?(:disabled_feature_flag)).to be_falsey + end + + it 'returns true for existing enabled feature in the database' do + described_class.enable(:enabled_feature_flag) + + expect(described_class.enabled?(:enabled_feature_flag)).to be_truthy + end + + context 'with an individual actor' do + CustomActor = Struct.new(:flipper_id) + + let(:actor) { CustomActor.new(flipper_id: 'CustomActor:5') } + let(:another_actor) { CustomActor.new(flipper_id: 'CustomActor:10') } + + before do + described_class.enable(:enabled_feature_flag, actor) + end + + it 'returns true when same actor is informed' do + expect(described_class.enabled?(:enabled_feature_flag, actor)).to be_truthy + end + + it 'returns false when different actor is informed' do + expect(described_class.enabled?(:enabled_feature_flag, another_actor)).to be_falsey + end + + it 'returns false when no actor is informed' do + expect(described_class.enabled?(:enabled_feature_flag)).to be_falsey + end + end + end + + describe '.disable?' do + it 'returns true for undefined feature' do + expect(described_class.disabled?(:some_random_feature_flag)).to be_truthy + end + + it 'returns true for existing disabled feature in the database' do + described_class.disable(:disabled_feature_flag) + + expect(described_class.disabled?(:disabled_feature_flag)).to be_truthy + end + + it 'returns false for existing enabled feature in the database' do + described_class.enable(:enabled_feature_flag) + + expect(described_class.disabled?(:enabled_feature_flag)).to be_falsey + end + end end diff --git a/spec/migrations/drop_duplicate_protected_tags_spec.rb b/spec/migrations/drop_duplicate_protected_tags_spec.rb new file mode 100644 index 00000000000..acfb6850722 --- /dev/null +++ b/spec/migrations/drop_duplicate_protected_tags_spec.rb @@ -0,0 +1,40 @@ +require 'spec_helper' +require Rails.root.join('db', 'migrate', '20180711103851_drop_duplicate_protected_tags.rb') + +describe DropDuplicateProtectedTags, :migration do + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:protected_tags) { table(:protected_tags) } + + before do + stub_const("#{described_class}::BATCH_SIZE", 2) + + namespaces.create(id: 1, name: 'gitlab-org', path: 'gitlab-org') + projects.create!(id: 1, namespace_id: 1, name: 'gitlab1', path: 'gitlab1') + projects.create!(id: 2, namespace_id: 1, name: 'gitlab2', path: 'gitlab2') + end + + it 'removes duplicated protected tags' do + protected_tags.create!(id: 1, project_id: 1, name: 'foo') + tag2 = protected_tags.create!(id: 2, project_id: 1, name: 'foo1') + protected_tags.create!(id: 3, project_id: 1, name: 'foo') + tag4 = protected_tags.create!(id: 4, project_id: 1, name: 'foo') + tag5 = protected_tags.create!(id: 5, project_id: 2, name: 'foo') + + migrate! + + expect(protected_tags.all.count).to eq 3 + expect(protected_tags.all.pluck(:id)).to contain_exactly(tag2.id, tag4.id, tag5.id) + end + + it 'does not remove unique protected tags' do + tag1 = protected_tags.create!(id: 1, project_id: 1, name: 'foo1') + tag2 = protected_tags.create!(id: 2, project_id: 1, name: 'foo2') + tag3 = protected_tags.create!(id: 3, project_id: 1, name: 'foo3') + + migrate! + + expect(protected_tags.all.count).to eq 3 + expect(protected_tags.all.pluck(:id)).to contain_exactly(tag1.id, tag2.id, tag3.id) + end +end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 8cb706b54b0..7cfffbde42f 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2989,6 +2989,7 @@ describe Project do # call. This makes testing a bit easier. allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) allow(project).to receive(:previous_changes).and_return('path' => ['foo']) + stub_feature_flags(skip_hashed_storage_upgrade: false) end it 'renames a repository' do @@ -3160,6 +3161,7 @@ describe Project do # call. This makes testing a bit easier. allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) allow(project).to receive(:previous_changes).and_return('path' => ['foo']) + stub_feature_flags(skip_hashed_storage_upgrade: false) end context 'migration to hashed storage' do diff --git a/spec/requests/api/protected_tags_spec.rb b/spec/requests/api/protected_tags_spec.rb new file mode 100644 index 00000000000..f4f3ef31bc3 --- /dev/null +++ b/spec/requests/api/protected_tags_spec.rb @@ -0,0 +1,202 @@ +require 'spec_helper' + +describe API::ProtectedTags do + let(:user) { create(:user) } + let!(:project) { create(:project, :repository) } + let(:project2) { create(:project, path: 'project2', namespace: user.namespace) } + let(:protected_name) { 'feature' } + let(:tag_name) { protected_name } + let!(:protected_tag) do + create(:protected_tag, project: project, name: protected_name) + end + + describe 'GET /projects/:id/protected_tags' do + let(:route) { "/projects/#{project.id}/protected_tags" } + + shared_examples_for 'protected tags' do + it 'returns the protected tags' do + get api(route, user), per_page: 100 + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + + protected_tag_names = json_response.map { |x| x['name'] } + expected_tags_names = project.protected_tags.map { |x| x['name'] } + expect(protected_tag_names).to match_array(expected_tags_names) + end + end + + context 'when authenticated as a maintainer' do + before do + project.add_maintainer(user) + end + + it_behaves_like 'protected tags' + end + + context 'when authenticated as a guest' do + before do + project.add_guest(user) + end + + it_behaves_like '403 response' do + let(:request) { get api(route, user) } + end + end + end + + describe 'GET /projects/:id/protected_tags/:tag' do + let(:route) { "/projects/#{project.id}/protected_tags/#{tag_name}" } + + shared_examples_for 'protected tag' do + it 'returns the protected tag' do + get api(route, user) + + expect(response).to have_gitlab_http_status(200) + expect(json_response['name']).to eq(tag_name) + expect(json_response['create_access_levels'][0]['access_level']).to eq(::Gitlab::Access::MAINTAINER) + end + + context 'when protected tag does not exist' do + let(:tag_name) { 'unknown' } + + it_behaves_like '404 response' do + let(:request) { get api(route, user) } + let(:message) { '404 Not found' } + end + end + end + + context 'when authenticated as a maintainer' do + before do + project.add_maintainer(user) + end + + it_behaves_like 'protected tag' + + context 'when protected tag contains a wildcard' do + let(:protected_name) { 'feature*' } + + it_behaves_like 'protected tag' + end + end + + context 'when authenticated as a guest' do + before do + project.add_guest(user) + end + + it_behaves_like '403 response' do + let(:request) { get api(route, user) } + end + end + end + + describe 'POST /projects/:id/protected_tags' do + let(:tag_name) { 'new_tag' } + + context 'when authenticated as a maintainer' do + before do + project.add_maintainer(user) + end + + it 'protects a single tag with maintainers can create tags' do + post api("/projects/#{project.id}/protected_tags", user), name: tag_name + + expect(response).to have_gitlab_http_status(201) + expect(json_response['name']).to eq(tag_name) + expect(json_response['create_access_levels'][0]['access_level']).to eq(Gitlab::Access::MAINTAINER) + end + + it 'protects a single tag with developers can create tags' do + post api("/projects/#{project.id}/protected_tags", user), + name: tag_name, create_access_level: 30 + + expect(response).to have_gitlab_http_status(201) + expect(json_response['name']).to eq(tag_name) + expect(json_response['create_access_levels'][0]['access_level']).to eq(Gitlab::Access::DEVELOPER) + end + + it 'protects a single tag with no one can create tags' do + post api("/projects/#{project.id}/protected_tags", user), + name: tag_name, create_access_level: 0 + + expect(response).to have_gitlab_http_status(201) + expect(json_response['name']).to eq(tag_name) + expect(json_response['create_access_levels'][0]['access_level']).to eq(Gitlab::Access::NO_ACCESS) + end + + it 'returns a 422 error if the same tag is protected twice' do + post api("/projects/#{project.id}/protected_tags", user), name: protected_name + + expect(response).to have_gitlab_http_status(422) + expect(json_response['message'][0]).to eq('Name has already been taken') + end + + it 'returns 201 if the same tag is proteted on different projects' do + post api("/projects/#{project.id}/protected_tags", user), name: protected_name + post api("/projects/#{project2.id}/protected_tags", user), name: protected_name + + expect(response).to have_gitlab_http_status(201) + expect(json_response['name']).to eq(protected_name) + end + + context 'when tag has a wildcard in its name' do + let(:tag_name) { 'feature/*' } + + it 'protects multiple tags with a wildcard in the name' do + post api("/projects/#{project.id}/protected_tags", user), name: tag_name + + expect(response).to have_gitlab_http_status(201) + expect(json_response['name']).to eq(tag_name) + expect(json_response['create_access_levels'][0]['access_level']).to eq(Gitlab::Access::MAINTAINER) + end + end + end + + context 'when authenticated as a guest' do + before do + project.add_guest(user) + end + + it 'returns a 403 error if guest' do + post api("/projects/#{project.id}/protected_tags/", user), name: tag_name + + expect(response).to have_gitlab_http_status(403) + end + end + end + + describe 'DELETE /projects/:id/protected_tags/unprotect/:tag' do + before do + project.add_maintainer(user) + end + + it 'unprotects a single tag' do + delete api("/projects/#{project.id}/protected_tags/#{tag_name}", user) + + expect(response).to have_gitlab_http_status(204) + end + + it_behaves_like '412 response' do + let(:request) { api("/projects/#{project.id}/protected_tags/#{tag_name}", user) } + end + + it "returns 404 if tag does not exist" do + delete api("/projects/#{project.id}/protected_tags/barfoo", user) + + expect(response).to have_gitlab_http_status(404) + end + + context 'when tag has a wildcard in its name' do + let(:protected_name) { 'feature*' } + + it 'unprotects a wildcard tag' do + delete api("/projects/#{project.id}/protected_tags/#{tag_name}", user) + + expect(response).to have_gitlab_http_status(204) + end + end + end +end diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 9572b4110d5..695b9980548 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -249,9 +249,20 @@ describe Projects::UpdateService do expect(project.errors.messages[:base]).to include('There is already a repository with that name on disk') end - context 'when hashed storage enabled' do + it 'renames the project without upgrading it' do + result = update_project(project, admin, path: 'new-path') + + expect(result).not_to include(status: :error) + expect(project).to be_valid + expect(project.errors).to be_empty + expect(project.disk_path).to include('new-path') + expect(project.reload.hashed_storage?(:repository)).to be_falsey + end + + context 'when hashed storage is enabled' do before do stub_application_setting(hashed_storage_enabled: true) + stub_feature_flags(skip_hashed_storage_upgrade: false) end it 'migrates project to a hashed storage instead of renaming the repo to another legacy name' do @@ -262,6 +273,22 @@ describe Projects::UpdateService do expect(project.errors).to be_empty expect(project.reload.hashed_storage?(:repository)).to be_truthy end + + context 'when skip_hashed_storage_upgrade feature flag is enabled' do + before do + stub_feature_flags(skip_hashed_storage_upgrade: true) + end + + it 'renames the project without upgrading it' do + result = update_project(project, admin, path: 'new-path') + + expect(result).not_to include(status: :error) + expect(project).to be_valid + expect(project.errors).to be_empty + expect(project.disk_path).to include('new-path') + expect(project.reload.hashed_storage?(:repository)).to be_falsey + end + end end end diff --git a/spec/support/helpers/stub_feature_flags.rb b/spec/support/helpers/stub_feature_flags.rb index b96338bf548..c54a871b157 100644 --- a/spec/support/helpers/stub_feature_flags.rb +++ b/spec/support/helpers/stub_feature_flags.rb @@ -1,4 +1,7 @@ module StubFeatureFlags + # Stub Feature flags with `flag_name: true/false` + # + # @param [Hash] features where key is feature name and value is boolean whether enabled or not def stub_feature_flags(features) features.each do |feature_name, enabled| allow(Feature).to receive(:enabled?).with(feature_name) { enabled } |