diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-02-26 15:08:56 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-02-26 15:08:56 +0000 |
| commit | 17ab40ca089e1aef61a83f77ab6df62a72f6ce06 (patch) | |
| tree | 8eb149293eee90ec2750b6ac5e46a111a806424e | |
| parent | 66d4203791a01fdedf668a78818a229ea2c07aad (diff) | |
| download | gitlab-ce-17ab40ca089e1aef61a83f77ab6df62a72f6ce06.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
61 files changed, 1078 insertions, 442 deletions
diff --git a/.gitlab/issue_templates/Security developer workflow.md b/.gitlab/issue_templates/Security developer workflow.md index 039157bf845..56be0453b33 100644 --- a/.gitlab/issue_templates/Security developer workflow.md +++ b/.gitlab/issue_templates/Security developer workflow.md @@ -50,10 +50,6 @@ After your merge request has being approved according to our [approval guideline | -------- | -------- | | Issue on [GitLab](https://gitlab.com/gitlab-org/gitlab/issues) | #TODO | | Security Release tracking issue | #TODO | -| `master` MR | !TODO | -| `Backport X.Y` MR | !TODO | -| `Backport X.Y` MR | !TODO | -| `Backport X.Y` MR | !TODO | ### Details diff --git a/.gitlab/merge_request_templates/Security Release.md b/.gitlab/merge_request_templates/Security Release.md index cccfafe397e..02cb4c59fd1 100644 --- a/.gitlab/merge_request_templates/Security Release.md +++ b/.gitlab/merge_request_templates/Security Release.md @@ -8,11 +8,11 @@ See [the general developer security release guidelines](https://gitlab.com/gitla ## Related issues -<!-- Mention the issue(s) this MR is related to --> +<!-- Mention the GitLab Security issue this MR is related to --> ## Developer checklist -- [ ] Link this MR in the `links` section of the related issue on [GitLab Security]. +- [ ] **Make sure this merge request mentions the [GitLab Security] issue it belongs to (i.e. `Related to <issue_id>`).** - [ ] Merge request targets `master`, or `X-Y-stable` for backports. - [ ] Milestone is set for the version this merge request applies to. A closed milestone can be assigned via [quick actions]. - [ ] Title of this merge request is the same as for all backports. diff --git a/.rubocop.yml b/.rubocop.yml index 08cb7f08316..927ee81a3ab 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -349,8 +349,8 @@ RSpec/HaveGitlabHttpStatus: - 'ee/spec/requests/{groups,projects,repositories}/**/*' - 'spec/requests/api/*/**/*.rb' - 'ee/spec/requests/api/*/**/*.rb' - - 'spec/requests/api/[a-l]*.rb' - - 'ee/spec/requests/api/[a-l]*.rb' + - 'spec/requests/api/[a-o]*.rb' + - 'ee/spec/requests/api/[a-o]*.rb' Style/MultilineWhenThen: Enabled: false diff --git a/app/assets/javascripts/due_date_select.js b/app/assets/javascripts/due_date_select.js index 218bf41cd58..6ebbeecae1d 100644 --- a/app/assets/javascripts/due_date_select.js +++ b/app/assets/javascripts/due_date_select.js @@ -44,6 +44,7 @@ class DueDateSelect { this.$selectbox.hide(); this.$value.css('display', ''); }, + shouldPropagate: false, }); } diff --git a/app/assets/javascripts/repository/components/breadcrumbs.vue b/app/assets/javascripts/repository/components/breadcrumbs.vue index 751565ad542..be70bfc7399 100644 --- a/app/assets/javascripts/repository/components/breadcrumbs.vue +++ b/app/assets/javascripts/repository/components/breadcrumbs.vue @@ -1,5 +1,6 @@ <script> import { GlDropdown, GlDropdownDivider, GlDropdownHeader, GlDropdownItem } from '@gitlab/ui'; +import { joinPaths } from '~/lib/utils/url_utility'; import { __ } from '../../locale'; import Icon from '../../vue_shared/components/icon.vue'; import getRefMixin from '../mixins/get_ref'; @@ -102,12 +103,12 @@ export default { .filter(p => p !== '') .reduce( (acc, name, i) => { - const path = `${i > 0 ? acc[i].path : ''}/${name}`; + const path = joinPaths(i > 0 ? acc[i].path : '', encodeURIComponent(name)); return acc.concat({ name, path, - to: `/-/tree/${escape(this.ref)}${escape(path)}`, + to: `/-/tree/${joinPaths(escape(this.ref), path)}`, }); }, [ diff --git a/app/assets/javascripts/repository/components/table/row.vue b/app/assets/javascripts/repository/components/table/row.vue index c905c39bbba..b81e6a38b4c 100644 --- a/app/assets/javascripts/repository/components/table/row.vue +++ b/app/assets/javascripts/repository/components/table/row.vue @@ -91,7 +91,9 @@ export default { }, computed: { routerLinkTo() { - return this.isFolder ? { path: `/-/tree/${escape(this.ref)}/${escape(this.path)}` } : null; + return this.isFolder + ? { path: `/-/tree/${escape(this.ref)}/${encodeURIComponent(this.path)}` } + : null; }, iconName() { return `fa-${getIconName(this.type, this.path)}`; @@ -141,6 +143,7 @@ export default { <i v-else :aria-label="type" role="img" :class="iconName" class="fa fa-fw"></i> <component :is="linkComponent" + ref="link" :to="routerLinkTo" :href="url" class="str-truncated" diff --git a/app/assets/javascripts/repository/log_tree.js b/app/assets/javascripts/repository/log_tree.js index 192e410b36f..ade92cc92e0 100644 --- a/app/assets/javascripts/repository/log_tree.js +++ b/app/assets/javascripts/repository/log_tree.js @@ -27,7 +27,7 @@ export function fetchLogsTree(client, path, offset, resolver = null) { fetchpromise = axios .get( - `${gon.relative_url_root}/${projectPath}/-/refs/${escape(ref)}/logs_tree/${escape( + `${gon.relative_url_root}/${projectPath}/-/refs/${escape(ref)}/logs_tree/${encodeURIComponent( path.replace(/^\//, ''), )}`, { diff --git a/app/assets/javascripts/vue_shared/components/pagination/constants.js b/app/assets/javascripts/vue_shared/components/pagination/constants.js index 229132c0e33..748ad178c70 100644 --- a/app/assets/javascripts/vue_shared/components/pagination/constants.js +++ b/app/assets/javascripts/vue_shared/components/pagination/constants.js @@ -3,8 +3,8 @@ import { s__ } from '~/locale'; export const PAGINATION_UI_BUTTON_LIMIT = 4; export const UI_LIMIT = 6; export const SPREAD = '...'; -export const PREV = s__('Pagination|‹ Prev'); -export const NEXT = s__('Pagination|Next ›'); +export const PREV = s__('Pagination|Prev'); +export const NEXT = s__('Pagination|Next'); export const FIRST = s__('Pagination|« First'); export const LAST = s__('Pagination|Last »'); export const LABEL_FIRST_PAGE = s__('Pagination|Go to first page'); diff --git a/app/controllers/projects/performance_monitoring/dashboards_controller.rb b/app/controllers/projects/performance_monitoring/dashboards_controller.rb index 2d872b78096..ec5a33f5dd6 100644 --- a/app/controllers/projects/performance_monitoring/dashboards_controller.rb +++ b/app/controllers/projects/performance_monitoring/dashboards_controller.rb @@ -22,6 +22,16 @@ module Projects end end + def update + result = ::Metrics::Dashboard::UpdateDashboardService.new(project, current_user, dashboard_params.merge(file_content_params)).execute + + if result[:status] == :success + respond_update_success(result) + else + respond_error(result) + end + end + private def respond_success(result) @@ -43,6 +53,19 @@ module Projects flash[:notice] = message.html_safe end + def respond_update_success(result) + set_web_ide_link_update_notice(result.dig(:dashboard, :path)) + respond_to do |format| + format.json { render status: result.delete(:http_status), json: result } + end + end + + def set_web_ide_link_update_notice(new_dashboard_path) + web_ide_link_start = "<a href=\"#{ide_edit_path(project, redirect_safe_branch_name, new_dashboard_path)}\">" + message = _("Your dashboard has been updated. You can %{web_ide_link_start}edit it here%{web_ide_link_end}.") % { web_ide_link_start: web_ide_link_start, web_ide_link_end: "</a>" } + flash[:notice] = message.html_safe + end + def validate_required_params! params.require(%i(branch file_name dashboard commit_message)) end @@ -54,6 +77,31 @@ module Projects def dashboard_params params.permit(%i(branch file_name dashboard commit_message)).to_h end + + def file_content_params + params.permit( + file_content: [ + :dashboard, + panel_groups: [ + :group, + :priority, + panels: [ + :type, + :title, + :y_label, + :weight, + metrics: [ + :id, + :unit, + :label, + :query, + :query_range + ] + ] + ] + ] + ) + end end end end diff --git a/app/models/ci/processable.rb b/app/models/ci/processable.rb index 6c080582cae..55518f32316 100644 --- a/app/models/ci/processable.rb +++ b/app/models/ci/processable.rb @@ -90,6 +90,12 @@ module Ci end end + def needs_attributes + strong_memoize(:needs_attributes) do + needs.map { |need| need.attributes.except('id', 'build_id') } + end + end + private def validate_scheduling_type? diff --git a/app/models/concerns/reactive_caching.rb b/app/models/concerns/reactive_caching.rb index 010e0018414..4b472cfdf45 100644 --- a/app/models/concerns/reactive_caching.rb +++ b/app/models/concerns/reactive_caching.rb @@ -52,6 +52,13 @@ module ReactiveCaching end end + def with_reactive_cache_set(resource, opts, &blk) + data = with_reactive_cache(resource, opts, &blk) + save_keys_in_set(resource, opts) if data + + data + end + # This method is used for debugging purposes and should not be used otherwise. def without_reactive_cache(*args, &blk) return with_reactive_cache(*args, &blk) unless Rails.env.development? @@ -65,6 +72,12 @@ module ReactiveCaching Rails.cache.delete(alive_reactive_cache_key(*args)) end + def clear_reactive_cache_set!(*args) + cache_key = full_reactive_cache_key(args) + + reactive_set_cache.clear_cache!(cache_key) + end + def exclusively_update_reactive_cache!(*args) locking_reactive_cache(*args) do key = full_reactive_cache_key(*args) @@ -86,6 +99,16 @@ module ReactiveCaching private + def save_keys_in_set(resource, opts) + cache_key = full_reactive_cache_key(resource) + + reactive_set_cache.write(cache_key, "#{cache_key}:#{opts}") + end + + def reactive_set_cache + Gitlab::ReactiveCacheSetCache.new(expires_in: reactive_cache_lifetime) + end + def refresh_reactive_cache!(*args) clear_reactive_cache!(*args) keep_alive_reactive_cache!(*args) diff --git a/app/models/error_tracking/project_error_tracking_setting.rb b/app/models/error_tracking/project_error_tracking_setting.rb index d328a609439..133850b6ab6 100644 --- a/app/models/error_tracking/project_error_tracking_setting.rb +++ b/app/models/error_tracking/project_error_tracking_setting.rb @@ -85,7 +85,7 @@ module ErrorTracking end def list_sentry_issues(opts = {}) - with_reactive_cache('list_issues', opts.stringify_keys) do |result| + with_reactive_cache_set('list_issues', opts.stringify_keys) do |result| result end end @@ -130,6 +130,10 @@ module ErrorTracking end end + def expire_issues_cache + clear_reactive_cache_set!('list_issues') + end + # http://HOST/api/0/projects/ORG/PROJECT # -> # http://HOST/ORG/PROJECT diff --git a/app/services/ci/create_job_artifacts_service.rb b/app/services/ci/create_job_artifacts_service.rb index e633dc7f633..3aa2b16bc73 100644 --- a/app/services/ci/create_job_artifacts_service.rb +++ b/app/services/ci/create_job_artifacts_service.rb @@ -1,8 +1,13 @@ # frozen_string_literal: true module Ci - class CreateJobArtifactsService + class CreateJobArtifactsService < ::BaseService ArtifactsExistError = Class.new(StandardError) + OBJECT_STORAGE_ERRORS = [ + Errno::EIO, + Google::Apis::ServerError, + Signet::RemoteServerError + ].freeze def execute(job, artifacts_file, params, metadata_file: nil) expire_in = params['expire_in'] || @@ -26,18 +31,20 @@ module Ci expire_in: expire_in) end - job.update(artifacts_expire_in: expire_in) - rescue ActiveRecord::RecordNotUnique => error - return true if sha256_matches_existing_artifact?(job, params['artifact_type'], artifacts_file) + if job.update(artifacts_expire_in: expire_in) + success + else + error(job.errors.messages, :bad_request) + end - Gitlab::ErrorTracking.track_exception(error, - job_id: job.id, - project_id: job.project_id, - uploading_type: params['artifact_type'] - ) + rescue ActiveRecord::RecordNotUnique => error + return success if sha256_matches_existing_artifact?(job, params['artifact_type'], artifacts_file) - job.errors.add(:base, 'another artifact of the same type already exists') - false + track_exception(error, job, params) + error('another artifact of the same type already exists', :bad_request) + rescue *OBJECT_STORAGE_ERRORS => error + track_exception(error, job, params) + error(error.message, :service_unavailable) end private @@ -48,5 +55,13 @@ module Ci existing_artifact.file_sha256 == artifacts_file.sha256 end + + def track_exception(error, job, params) + Gitlab::ErrorTracking.track_exception(error, + job_id: job.id, + project_id: job.project_id, + uploading_type: params['artifact_type'] + ) + end end end diff --git a/app/services/ci/retry_build_service.rb b/app/services/ci/retry_build_service.rb index 838ed789155..87f818ad497 100644 --- a/app/services/ci/retry_build_service.rb +++ b/app/services/ci/retry_build_service.rb @@ -5,7 +5,8 @@ module Ci CLONE_ACCESSORS = %i[pipeline project ref tag options name allow_failure stage stage_id stage_idx trigger_request yaml_variables when environment coverage_regex - description tag_list protected needs resource_group scheduling_type].freeze + description tag_list protected needs_attributes + resource_group scheduling_type].freeze def execute(build) reprocess!(build).tap do |new_build| diff --git a/app/services/error_tracking/issue_update_service.rb b/app/services/error_tracking/issue_update_service.rb index e516ac95138..b8235678d1d 100644 --- a/app/services/error_tracking/issue_update_service.rb +++ b/app/services/error_tracking/issue_update_service.rb @@ -11,6 +11,7 @@ module ErrorTracking ) compose_response(response) do + project_error_tracking_setting.expire_issues_cache response[:closed_issue_iid] = update_related_issue&.iid end end diff --git a/app/services/metrics/dashboard/update_dashboard_service.rb b/app/services/metrics/dashboard/update_dashboard_service.rb new file mode 100644 index 00000000000..316563ecbe7 --- /dev/null +++ b/app/services/metrics/dashboard/update_dashboard_service.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +# Updates the content of a specified dashboard in .yml file inside `.gitlab/dashboards` +module Metrics + module Dashboard + class UpdateDashboardService < ::BaseService + ALLOWED_FILE_TYPE = '.yml' + USER_DASHBOARDS_DIR = ::Metrics::Dashboard::ProjectDashboardService::DASHBOARD_ROOT + + def execute + catch(:error) do + throw(:error, error(_(%q(You can't commit to this project)), :forbidden)) unless push_authorized? + + result = ::Files::UpdateService.new(project, current_user, dashboard_attrs).execute + throw(:error, result.merge(http_status: :bad_request)) unless result[:status] == :success + + success(result.merge(http_status: :created, dashboard: dashboard_details)) + end + end + + private + + def dashboard_attrs + { + commit_message: params[:commit_message], + file_path: update_dashboard_path, + file_content: update_dashboard_content, + encoding: 'text', + branch_name: branch, + start_branch: repository.branch_exists?(branch) ? branch : project.default_branch + } + end + + def dashboard_details + { + path: update_dashboard_path, + display_name: ::Metrics::Dashboard::ProjectDashboardService.name_for_path(update_dashboard_path), + default: false, + system_dashboard: false + } + end + + def push_authorized? + Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(branch) + end + + def branch + @branch ||= begin + throw(:error, error(_('There was an error updating the dashboard, branch name is invalid.'), :bad_request)) unless valid_branch_name? + throw(:error, error(_('There was an error updating the dashboard, branch named: %{branch} already exists.') % { branch: params[:branch] }, :bad_request)) unless new_or_default_branch? + + params[:branch] + end + end + + def new_or_default_branch? + !repository.branch_exists?(params[:branch]) || project.default_branch == params[:branch] + end + + def valid_branch_name? + Gitlab::GitRefValidator.validate(params[:branch]) + end + + def update_dashboard_path + File.join(USER_DASHBOARDS_DIR, file_name) + end + + def target_file_type_valid? + File.extname(params[:file_name]) == ALLOWED_FILE_TYPE + end + + def file_name + @file_name ||= begin + throw(:error, error(_('The file name should have a .yml extension'), :bad_request)) unless target_file_type_valid? + + File.basename(CGI.unescape(params[:file_name])) + end + end + + def update_dashboard_content + ::PerformanceMonitoring::PrometheusDashboard.from_json(params[:file_content]).to_yaml + end + + def repository + @repository ||= project.repository + end + end + end +end diff --git a/changelogs/unreleased/201427-issue-board-due-date-picker-unable-to-select-custom-year-or-month.yml b/changelogs/unreleased/201427-issue-board-due-date-picker-unable-to-select-custom-year-or-month.yml new file mode 100644 index 00000000000..d831a674efe --- /dev/null +++ b/changelogs/unreleased/201427-issue-board-due-date-picker-unable-to-select-custom-year-or-month.yml @@ -0,0 +1,6 @@ +--- +title: Added the multiSelect option to stop event propagation when clicking on the + dropdown +merge_request: 24611 +author: Gwen_ +type: fixed diff --git a/changelogs/unreleased/207087-blocked-status-issue.yml b/changelogs/unreleased/207087-blocked-status-issue.yml new file mode 100644 index 00000000000..e708a8a635e --- /dev/null +++ b/changelogs/unreleased/207087-blocked-status-issue.yml @@ -0,0 +1,5 @@ +--- +title: Don't show issue as blocked on the issue board if blocking issue is closed +merge_request: 25817 +author: +type: fixed diff --git a/changelogs/unreleased/207536-retried-jobs-are-not-able-to-find-knapsack-report.yml b/changelogs/unreleased/207536-retried-jobs-are-not-able-to-find-knapsack-report.yml new file mode 100644 index 00000000000..7c40b21769c --- /dev/null +++ b/changelogs/unreleased/207536-retried-jobs-are-not-able-to-find-knapsack-report.yml @@ -0,0 +1,5 @@ +--- +title: Keep needs association on the retried build +merge_request: 25888 +author: +type: fixed diff --git a/changelogs/unreleased/34525-update-custom-dashboard.yml b/changelogs/unreleased/34525-update-custom-dashboard.yml new file mode 100644 index 00000000000..acc03e927b0 --- /dev/null +++ b/changelogs/unreleased/34525-update-custom-dashboard.yml @@ -0,0 +1,5 @@ +--- +title: Update file content of an existing custom dashboard +merge_request: 25024 +author: +type: added diff --git a/changelogs/unreleased/handle-object-storage-errors.yml b/changelogs/unreleased/handle-object-storage-errors.yml new file mode 100644 index 00000000000..8140a30fa78 --- /dev/null +++ b/changelogs/unreleased/handle-object-storage-errors.yml @@ -0,0 +1,5 @@ +--- +title: Return 503 to the Runner when the object storage is unavailable +merge_request: 25822 +author: +type: fixed diff --git a/changelogs/unreleased/ph-p207499-fixNonAsciiChars.yml b/changelogs/unreleased/ph-p207499-fixNonAsciiChars.yml new file mode 100644 index 00000000000..806f4372f89 --- /dev/null +++ b/changelogs/unreleased/ph-p207499-fixNonAsciiChars.yml @@ -0,0 +1,5 @@ +--- +title: Fixed repository browsing for folders with non-ascii characters +merge_request: 25877 +author: +type: fixed diff --git a/changelogs/unreleased/upgrade-gitlab-ui.yml b/changelogs/unreleased/upgrade-gitlab-ui.yml new file mode 100644 index 00000000000..b881a643a72 --- /dev/null +++ b/changelogs/unreleased/upgrade-gitlab-ui.yml @@ -0,0 +1,5 @@ +--- +title: Remove special chars from previous and next items in pagination +merge_request: 25891 +author: +type: other diff --git a/config/routes/project.rb b/config/routes/project.rb index f153082f118..aa2410d8e00 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -252,7 +252,11 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do end namespace :performance_monitoring do - resources :dashboards, only: [:create] + resources :dashboards, only: [:create] do + collection do + put '/:file_name', to: 'dashboards#update', constraints: { file_name: /.+\.yml/ } + end + end end namespace :error_tracking do diff --git a/doc/ci/merge_request_pipelines/pipelines_for_merged_results/index.md b/doc/ci/merge_request_pipelines/pipelines_for_merged_results/index.md index 379644beacd..e5a77c1f0d6 100644 --- a/doc/ci/merge_request_pipelines/pipelines_for_merged_results/index.md +++ b/doc/ci/merge_request_pipelines/pipelines_for_merged_results/index.md @@ -85,13 +85,27 @@ the following feature flags are enabled on your GitLab instance: - `:ci_use_merge_request_ref` - `:merge_ref_auto_sync` -To check these feature flag values, please ask administrator to execute the following commands: +To check and set these feature flag values, please ask an administrator to: -```shell -> sudo gitlab-rails console # Login to Rails console of GitLab instance. -> Feature.enabled?(:ci_use_merge_request_ref) # Check if it's enabled or not. -> Feature.enable(:ci_use_merge_request_ref) # Enable the feature flag. -``` +1. Log into the Rails console of the GitLab instance: + + ```shell + sudo gitlab-rails console + ``` + +1. Check if the flags are enabled or not: + + ```ruby + Feature.enabled?(:ci_use_merge_request_ref) + Feature.enabled?(:merge_ref_auto_sync) + ``` + +1. If needed, enable the feature flags: + + ```ruby + Feature.enable(:ci_use_merge_request_ref) + Feature.enable(:merge_ref_auto_sync) + ``` ### Intermittently pipelines fail by `fatal: reference is not a tree:` error diff --git a/lib/api/runner.rb b/lib/api/runner.rb index e1c79aa8efe..0b6bad6708b 100644 --- a/lib/api/runner.rb +++ b/lib/api/runner.rb @@ -283,10 +283,12 @@ module API bad_request!('Missing artifacts file!') unless artifacts file_too_large! unless artifacts.size < max_artifacts_size(job) - if Ci::CreateJobArtifactsService.new.execute(job, artifacts, params, metadata_file: metadata) + result = Ci::CreateJobArtifactsService.new(job.project).execute(job, artifacts, params, metadata_file: metadata) + + if result[:status] == :success status :created else - render_validation_error!(job) + render_api_error!(result[:message], result[:http_status]) end end diff --git a/lib/gitlab/markdown_cache.rb b/lib/gitlab/markdown_cache.rb index 473c7203ead..d7a0a9b6518 100644 --- a/lib/gitlab/markdown_cache.rb +++ b/lib/gitlab/markdown_cache.rb @@ -3,7 +3,7 @@ module Gitlab module MarkdownCache # Increment this number every time the renderer changes its output - CACHE_COMMONMARK_VERSION = 19 + CACHE_COMMONMARK_VERSION = 20 CACHE_COMMONMARK_VERSION_START = 10 BaseError = Class.new(StandardError) diff --git a/lib/gitlab/reactive_cache_set_cache.rb b/lib/gitlab/reactive_cache_set_cache.rb index 14d008c3dfb..609087d8137 100644 --- a/lib/gitlab/reactive_cache_set_cache.rb +++ b/lib/gitlab/reactive_cache_set_cache.rb @@ -10,9 +10,13 @@ module Gitlab @expires_in = expires_in end + def cache_key(key) + "#{cache_type}:#{key}:set" + end + def clear_cache!(key) with do |redis| - keys = read(key).map { |value| "#{cache_type}#{value}" } + keys = read(key).map { |value| "#{cache_type}:#{value}" } keys << cache_key(key) redis.pipelined do @@ -24,7 +28,7 @@ module Gitlab private def cache_type - "#{Gitlab::Redis::Cache::CACHE_NAMESPACE}:" + Gitlab::Redis::Cache::CACHE_NAMESPACE end end end diff --git a/lib/gitlab/tree_summary.rb b/lib/gitlab/tree_summary.rb index 76018cb23c4..5df53b5adde 100644 --- a/lib/gitlab/tree_summary.rb +++ b/lib/gitlab/tree_summary.rb @@ -69,7 +69,7 @@ module Gitlab end def entry_path(entry) - File.join(*[path, entry[:file_name]].compact) + File.join(*[path, entry[:file_name]].compact).force_encoding(Encoding::ASCII_8BIT) end def build_entry(entry) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 6af3f15f31b..7fb27e961ac 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -13658,18 +13658,12 @@ msgstr "" msgid "Pagination|Next" msgstr "" -msgid "Pagination|Next ›" -msgstr "" - msgid "Pagination|Prev" msgstr "" msgid "Pagination|« First" msgstr "" -msgid "Pagination|‹ Prev" -msgstr "" - msgid "Parameter" msgstr "" @@ -18615,6 +18609,9 @@ msgstr "" msgid "Subscription deletion failed." msgstr "" +msgid "Subscription successfully applied to \"%{group_name}\"" +msgstr "" + msgid "Subscription successfully created." msgstr "" @@ -19633,6 +19630,12 @@ msgstr "" msgid "There was an error trying to validate your query" msgstr "" +msgid "There was an error updating the dashboard, branch name is invalid." +msgstr "" + +msgid "There was an error updating the dashboard, branch named: %{branch} already exists." +msgstr "" + msgid "There was an error when reseting email token." msgstr "" @@ -22692,6 +22695,9 @@ msgstr "" msgid "Your dashboard has been copied. You can %{web_ide_link_start}edit it here%{web_ide_link_end}." msgstr "" +msgid "Your dashboard has been updated. You can %{web_ide_link_start}edit it here%{web_ide_link_end}." +msgstr "" + msgid "Your deployment services will be broken, you will need to manually fix the services after renaming." msgstr "" diff --git a/spec/controllers/projects/performance_monitoring/dashboards_controller_spec.rb b/spec/controllers/projects/performance_monitoring/dashboards_controller_spec.rb index 1c29b68dc24..6a53e8f3dbf 100644 --- a/spec/controllers/projects/performance_monitoring/dashboards_controller_spec.rb +++ b/spec/controllers/projects/performance_monitoring/dashboards_controller_spec.rb @@ -129,4 +129,130 @@ describe Projects::PerformanceMonitoring::DashboardsController do end end end + + describe 'PUT #update' do + context 'authenticated user' do + before do + sign_in(user) + end + + let(:file_content) do + { + "dashboard" => "Dashboard Title", + "panel_groups" => [{ + "group" => "Group Title", + "panels" => [{ + "type" => "area-chart", + "title" => "Chart Title", + "y_label" => "Y-Axis", + "metrics" => [{ + "id" => "metric_of_ages", + "unit" => "count", + "label" => "Metric of Ages", + "query_range" => "http_requests_total" + }] + }] + }] + } + end + + let(:params) do + { + namespace_id: namespace, + project_id: project, + dashboard: dashboard, + file_name: file_name, + file_content: file_content, + commit_message: commit_message, + branch: branch_name, + format: :json + } + end + + context 'project with repository feature' do + context 'with rights to push to the repository' do + before do + project.add_maintainer(user) + end + + context 'valid parameters' do + context 'request format json' do + let(:update_dashboard_service_params) { params.except(:namespace_id, :project_id, :format) } + + let(:update_dashboard_service_results) do + { + status: :success, + http_status: :created, + dashboard: { + path: ".gitlab/dashboards/custom_dashboard.yml", + display_name: "custom_dashboard.yml", + default: false, + system_dashboard: false + } + } + end + + let(:update_dashboard_service) { instance_double(::Metrics::Dashboard::UpdateDashboardService, execute: update_dashboard_service_results) } + + it 'returns path to new file' do + allow(controller).to receive(:repository).and_return(repository) + allow(repository).to receive(:find_branch).and_return(branch) + allow(::Metrics::Dashboard::UpdateDashboardService).to receive(:new).with(project, user, update_dashboard_service_params).and_return(update_dashboard_service) + + put :update, params: params + + expect(response).to have_gitlab_http_status :created + expect(response).to set_flash[:notice].to eq("Your dashboard has been updated. You can <a href=\"/-/ide/project/#{namespace.path}/#{project.name}/edit/#{branch_name}/-/.gitlab/dashboards/#{file_name}\">edit it here</a>.") + expect(json_response).to eq('status' => 'success', 'dashboard' => { 'default' => false, 'display_name' => "custom_dashboard.yml", 'path' => ".gitlab/dashboards/#{file_name}", 'system_dashboard' => false }) + end + + context 'UpdateDashboardService failure' do + it 'returns json with failure message' do + allow(::Metrics::Dashboard::UpdateDashboardService).to receive(:new).and_return(double(execute: { status: :error, message: 'something went wrong', http_status: :bad_request })) + + put :update, params: params + + expect(response).to have_gitlab_http_status :bad_request + expect(json_response).to eq('error' => 'something went wrong') + end + end + end + end + + context 'missing branch' do + let(:branch_name) { nil } + + it 'raises responds with :bad_request status code and error message' do + put :update, params: params + + expect(response).to have_gitlab_http_status :bad_request + expect(json_response).to eq('error' => "Request parameter branch is missing.") + end + end + end + + context 'without rights to push to repository' do + before do + project.add_guest(user) + end + + it 'responds with :forbidden status code' do + put :update, params: params + + expect(response).to have_gitlab_http_status :forbidden + end + end + end + + context 'project without repository feature' do + let!(:project) { create(:project, name: 'dashboard-project', namespace: namespace) } + + it 'responds with :not_found status code' do + put :update, params: params + + expect(response).to have_gitlab_http_status :not_found + end + end + end + end end diff --git a/spec/features/discussion_comments/commit_spec.rb b/spec/features/discussion_comments/commit_spec.rb index 0362ddbae82..f594a30165b 100644 --- a/spec/features/discussion_comments/commit_spec.rb +++ b/spec/features/discussion_comments/commit_spec.rb @@ -24,7 +24,7 @@ describe 'Thread Comments Commit', :js do expect(page).to have_css('.js-note-emoji') end - it 'adds award to the correct note' do + it 'adds award to the correct note', quarantine: 'https://gitlab.com/gitlab-org/gitlab/issues/207973' do find("#note_#{commit_discussion_note2.id} .js-note-emoji").click first('.emoji-menu .js-emoji-btn').click diff --git a/spec/features/projects/tree/tree_show_spec.rb b/spec/features/projects/tree/tree_show_spec.rb index 23b13858096..2407a9e6ea3 100644 --- a/spec/features/projects/tree/tree_show_spec.rb +++ b/spec/features/projects/tree/tree_show_spec.rb @@ -37,6 +37,16 @@ describe 'Projects tree', :js do expect(page).not_to have_selector('.flash-alert') end + it 'renders tree table with non-ASCII filenames without errors' do + visit project_tree_path(project, File.join(test_sha, 'encoding')) + wait_for_requests + + expect(page).to have_selector('.tree-item') + expect(page).to have_content('Files, encoding and much more') + expect(page).to have_content('テスト.txt') + expect(page).not_to have_selector('.flash-alert') + end + context 'gravatar disabled' do let(:gravatar_enabled) { false } diff --git a/spec/frontend/create_cluster/gke_cluster/components/gke_zone_dropdown_spec.js b/spec/frontend/create_cluster/gke_cluster/components/gke_zone_dropdown_spec.js new file mode 100644 index 00000000000..c07e3f81964 --- /dev/null +++ b/spec/frontend/create_cluster/gke_cluster/components/gke_zone_dropdown_spec.js @@ -0,0 +1,101 @@ +import { shallowMount } from '@vue/test-utils'; +import GkeZoneDropdown from '~/create_cluster/gke_cluster/components/gke_zone_dropdown.vue'; +import DropdownHiddenInput from '~/vue_shared/components/dropdown/dropdown_hidden_input.vue'; +import DropdownButton from '~/vue_shared/components/dropdown/dropdown_button.vue'; +import { createStore } from '~/create_cluster/gke_cluster/store'; +import { + SET_PROJECT, + SET_ZONES, + SET_PROJECT_BILLING_STATUS, +} from '~/create_cluster/gke_cluster/store/mutation_types'; +import { selectedZoneMock, selectedProjectMock, gapiZonesResponseMock } from '../mock_data'; + +const propsData = { + fieldId: 'cluster_provider_gcp_attributes_gcp_zone', + fieldName: 'cluster[provider_gcp_attributes][gcp_zone]', +}; + +const LABELS = { + LOADING: 'Fetching zones', + DISABLED: 'Select project to choose zone', + DEFAULT: 'Select zone', +}; + +describe('GkeZoneDropdown', () => { + let store; + let wrapper; + + beforeEach(() => { + store = createStore(); + wrapper = shallowMount(GkeZoneDropdown, { propsData, store }); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + describe('toggleText', () => { + let dropdownButton; + + beforeEach(() => { + dropdownButton = wrapper.find(DropdownButton); + }); + + it('returns disabled state toggle text', () => { + expect(dropdownButton.props('toggleText')).toBe(LABELS.DISABLED); + }); + + describe('isLoading', () => { + beforeEach(() => { + wrapper.setData({ isLoading: true }); + return wrapper.vm.$nextTick(); + }); + + it('returns loading toggle text', () => { + expect(dropdownButton.props('toggleText')).toBe(LABELS.LOADING); + }); + }); + + describe('project is set', () => { + beforeEach(() => { + wrapper.vm.$store.commit(SET_PROJECT, selectedProjectMock); + wrapper.vm.$store.commit(SET_PROJECT_BILLING_STATUS, true); + return wrapper.vm.$nextTick(); + }); + + it('returns default toggle text', () => { + expect(dropdownButton.props('toggleText')).toBe(LABELS.DEFAULT); + }); + }); + + describe('project is selected', () => { + beforeEach(() => { + wrapper.vm.setItem(selectedZoneMock); + return wrapper.vm.$nextTick(); + }); + + it('returns project name if project selected', () => { + expect(dropdownButton.props('toggleText')).toBe(selectedZoneMock); + }); + }); + }); + + describe('selectItem', () => { + beforeEach(() => { + wrapper.vm.$store.commit(SET_ZONES, gapiZonesResponseMock.items); + return wrapper.vm.$nextTick(); + }); + + it('reflects new value when dropdown item is clicked', () => { + const dropdown = wrapper.find(DropdownHiddenInput); + + expect(dropdown.attributes('value')).toBe(''); + + wrapper.find('.dropdown-content button').trigger('click'); + + return wrapper.vm.$nextTick().then(() => { + expect(dropdown.attributes('value')).toBe(selectedZoneMock); + }); + }); + }); +}); diff --git a/spec/frontend/repository/components/breadcrumbs_spec.js b/spec/frontend/repository/components/breadcrumbs_spec.js index 0271db25468..38e5c9aaca5 100644 --- a/spec/frontend/repository/components/breadcrumbs_spec.js +++ b/spec/frontend/repository/components/breadcrumbs_spec.js @@ -41,7 +41,7 @@ describe('Repository breadcrumbs component', () => { .findAll(RouterLinkStub) .at(3) .props('to'), - ).toEqual('/-/tree//app/assets/javascripts%23'); + ).toEqual('/-/tree/app/assets/javascripts%23'); }); it('renders last link as active', () => { diff --git a/spec/frontend/repository/components/table/row_spec.js b/spec/frontend/repository/components/table/row_spec.js index fec9ba3aa2e..a51846023ac 100644 --- a/spec/frontend/repository/components/table/row_spec.js +++ b/spec/frontend/repository/components/table/row_spec.js @@ -109,6 +109,26 @@ describe('Repository table row component', () => { }); }); + it.each` + path + ${'test#'} + ${'Änderungen'} + `('renders link for $path', ({ path }) => { + factory({ + id: '1', + sha: '123', + path, + type: 'tree', + currentPath: '/', + }); + + return vm.vm.$nextTick().then(() => { + expect(vm.find({ ref: 'link' }).props('to')).toEqual({ + path: `/-/tree/master/${encodeURIComponent(path)}`, + }); + }); + }); + it('pushes new route for directory with hash', () => { factory({ id: '1', diff --git a/spec/javascripts/create_cluster/.eslintrc.yml b/spec/javascripts/create_cluster/.eslintrc.yml deleted file mode 100644 index 14e318a2f3e..00000000000 --- a/spec/javascripts/create_cluster/.eslintrc.yml +++ /dev/null @@ -1,3 +0,0 @@ -rules: - # https://gitlab.com/gitlab-org/gitlab/issues/33025 - promise/no-nesting: off diff --git a/spec/javascripts/create_cluster/gke_cluster/components/gke_zone_dropdown_spec.js b/spec/javascripts/create_cluster/gke_cluster/components/gke_zone_dropdown_spec.js deleted file mode 100644 index b2a7443422a..00000000000 --- a/spec/javascripts/create_cluster/gke_cluster/components/gke_zone_dropdown_spec.js +++ /dev/null @@ -1,94 +0,0 @@ -import Vue from 'vue'; -import { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; -import GkeZoneDropdown from '~/create_cluster/gke_cluster/components/gke_zone_dropdown.vue'; -import { createStore } from '~/create_cluster/gke_cluster/store'; -import { - SET_PROJECT, - SET_ZONES, - SET_PROJECT_BILLING_STATUS, -} from '~/create_cluster/gke_cluster/store/mutation_types'; -import { selectedZoneMock, selectedProjectMock, gapiZonesResponseMock } from '../mock_data'; - -const componentConfig = { - fieldId: 'cluster_provider_gcp_attributes_gcp_zone', - fieldName: 'cluster[provider_gcp_attributes][gcp_zone]', -}; - -const LABELS = { - LOADING: 'Fetching zones', - DISABLED: 'Select project to choose zone', - DEFAULT: 'Select zone', -}; - -const createComponent = (store, props = componentConfig) => { - const Component = Vue.extend(GkeZoneDropdown); - - return mountComponentWithStore(Component, { - el: null, - props, - store, - }); -}; - -describe('GkeZoneDropdown', () => { - let vm; - let store; - - beforeEach(() => { - store = createStore(); - vm = createComponent(store); - }); - - afterEach(() => { - vm.$destroy(); - }); - - describe('toggleText', () => { - it('returns disabled state toggle text', () => { - expect(vm.toggleText).toBe(LABELS.DISABLED); - }); - - it('returns loading toggle text', () => { - vm.isLoading = true; - - expect(vm.toggleText).toBe(LABELS.LOADING); - }); - - it('returns default toggle text', () => { - expect(vm.toggleText).toBe(LABELS.DISABLED); - - vm.$store.commit(SET_PROJECT, selectedProjectMock); - vm.$store.commit(SET_PROJECT_BILLING_STATUS, true); - - expect(vm.toggleText).toBe(LABELS.DEFAULT); - }); - - it('returns project name if project selected', () => { - vm.setItem(selectedZoneMock); - - expect(vm.toggleText).toBe(selectedZoneMock); - }); - }); - - describe('selectItem', () => { - it('reflects new value when dropdown item is clicked', done => { - expect(vm.$el.querySelector('input').value).toBe(''); - vm.$store.commit(SET_ZONES, gapiZonesResponseMock.items); - - return vm - .$nextTick() - .then(() => { - vm.$el.querySelector('.dropdown-content button').click(); - - return vm - .$nextTick() - .then(() => { - expect(vm.$el.querySelector('input').value).toBe(selectedZoneMock); - done(); - }) - .catch(done.fail); - }) - .catch(done.fail); - }); - }); -}); diff --git a/spec/javascripts/create_cluster/gke_cluster/mock_data.js b/spec/javascripts/create_cluster/gke_cluster/mock_data.js deleted file mode 100644 index d9f5dbc636f..00000000000 --- a/spec/javascripts/create_cluster/gke_cluster/mock_data.js +++ /dev/null @@ -1,75 +0,0 @@ -export const emptyProjectMock = { - projectId: '', - name: '', -}; - -export const selectedProjectMock = { - projectId: 'gcp-project-123', - name: 'gcp-project', -}; - -export const selectedZoneMock = 'us-central1-a'; - -export const selectedMachineTypeMock = 'n1-standard-2'; - -export const gapiProjectsResponseMock = { - projects: [ - { - projectNumber: '1234', - projectId: 'gcp-project-123', - lifecycleState: 'ACTIVE', - name: 'gcp-project', - createTime: '2017-12-16T01:48:29.129Z', - parent: { - type: 'organization', - id: '12345', - }, - }, - ], -}; - -export const gapiZonesResponseMock = { - kind: 'compute#zoneList', - id: 'projects/gitlab-internal-153318/zones', - items: [ - { - kind: 'compute#zone', - id: '2000', - creationTimestamp: '1969-12-31T16:00:00.000-08:00', - name: 'us-central1-a', - description: 'us-central1-a', - status: 'UP', - region: - 'https://www.googleapis.com/compute/v1/projects/gitlab-internal-153318/regions/us-central1', - selfLink: - 'https://www.googleapis.com/compute/v1/projects/gitlab-internal-153318/zones/us-central1-a', - availableCpuPlatforms: ['Intel Skylake', 'Intel Broadwell', 'Intel Sandy Bridge'], - }, - ], - selfLink: 'https://www.googleapis.com/compute/v1/projects/gitlab-internal-153318/zones', -}; - -export const gapiMachineTypesResponseMock = { - kind: 'compute#machineTypeList', - id: 'projects/gitlab-internal-153318/zones/us-central1-a/machineTypes', - items: [ - { - kind: 'compute#machineType', - id: '3002', - creationTimestamp: '1969-12-31T16:00:00.000-08:00', - name: 'n1-standard-2', - description: '2 vCPUs, 7.5 GB RAM', - guestCpus: 2, - memoryMb: 7680, - imageSpaceGb: 10, - maximumPersistentDisks: 64, - maximumPersistentDisksSizeGb: '65536', - zone: 'us-central1-a', - selfLink: - 'https://www.googleapis.com/compute/v1/projects/gitlab-internal-153318/zones/us-central1-a/machineTypes/n1-standard-2', - isSharedCpu: false, - }, - ], - selfLink: - 'https://www.googleapis.com/compute/v1/projects/gitlab-internal-153318/zones/us-central1-a/machineTypes', -}; diff --git a/spec/javascripts/diffs/components/commit_item_spec.js b/spec/javascripts/diffs/components/commit_item_spec.js index dc3fb16eb40..9fe31a5e727 100644 --- a/spec/javascripts/diffs/components/commit_item_spec.js +++ b/spec/javascripts/diffs/components/commit_item_spec.js @@ -41,7 +41,9 @@ describe('diffs/components/commit_item', () => { expect(titleElement).toHaveText(commit.title_html); }); - it('renders commit description', () => { + // https://gitlab.com/gitlab-org/gitlab/issues/197139 + // eslint-disable-next-line jasmine/no-disabled-tests + xit('renders commit description', () => { const descElement = getDescElement(vm); const descExpandElement = getDescExpandElement(vm); diff --git a/spec/javascripts/vue_shared/components/issue/related_issuable_mock_data.js b/spec/javascripts/vue_shared/components/issue/related_issuable_mock_data.js index 92080cb9bd5..d3dc169ddab 100644 --- a/spec/javascripts/vue_shared/components/issue/related_issuable_mock_data.js +++ b/spec/javascripts/vue_shared/components/issue/related_issuable_mock_data.js @@ -14,6 +14,8 @@ export const issuable1 = { path: '/foo/bar/issues/123', state: 'opened', linkType: 'relates_to', + dueDate: '2010-11-22', + weight: 5, }; export const issuable2 = { diff --git a/spec/lib/gitlab/profiler_spec.rb b/spec/lib/gitlab/profiler_spec.rb index 0186d48fd1b..1e53faf7275 100644 --- a/spec/lib/gitlab/profiler_spec.rb +++ b/spec/lib/gitlab/profiler_spec.rb @@ -212,7 +212,7 @@ describe Gitlab::Profiler do stub_const('STDOUT', stdout) end - it 'prints a profile result sorted by total time' do + it 'prints a profile result sorted by total time', quarantine: 'https://gitlab.com/gitlab-org/gitlab/issues/206907' do described_class.print_by_total_time(result) total_times = diff --git a/spec/lib/gitlab/reactive_cache_set_cache_spec.rb b/spec/lib/gitlab/reactive_cache_set_cache_spec.rb index 58c8fd0c1c3..a0d8f15ba1b 100644 --- a/spec/lib/gitlab/reactive_cache_set_cache_spec.rb +++ b/spec/lib/gitlab/reactive_cache_set_cache_spec.rb @@ -12,7 +12,7 @@ describe Gitlab::ReactiveCacheSetCache, :clean_gitlab_redis_cache do subject { cache.cache_key(cache_prefix) } it 'includes the suffix' do - expect(subject).to eq "#{cache_prefix}:set" + expect(subject).to eq "#{Gitlab::Redis::Cache::CACHE_NAMESPACE}:#{cache_prefix}:set" end end diff --git a/spec/lib/gitlab/tree_summary_spec.rb b/spec/lib/gitlab/tree_summary_spec.rb index e15463ed0eb..d64b826ba9b 100644 --- a/spec/lib/gitlab/tree_summary_spec.rb +++ b/spec/lib/gitlab/tree_summary_spec.rb @@ -129,6 +129,17 @@ describe Gitlab::TreeSummary do expect(commits).to satisfy_one { |c| c.id == whitespace_commit_sha } end end + + context 'in a subdirectory with non-ASCII filenames' do + let(:path) { 'encoding' } + + it 'returns commits for entries in the subdirectory' do + entry = entries.find { |x| x[:file_name] == 'テスト.txt' } + + expect(entry).to be_a(Hash) + expect(entry).to include(:commit) + end + end end describe '#more?' do diff --git a/spec/models/ci/processable_spec.rb b/spec/models/ci/processable_spec.rb index 1e0544c14c5..e03f54aa728 100644 --- a/spec/models/ci/processable_spec.rb +++ b/spec/models/ci/processable_spec.rb @@ -145,4 +145,28 @@ describe Ci::Processable do expect(another_build.reload.scheduling_type).to be_nil end end + + describe '#needs_attributes' do + let(:build) { create(:ci_build, :created, project: project, pipeline: pipeline) } + + context 'with needs' do + before do + create(:ci_build_need, build: build, name: 'test1') + create(:ci_build_need, build: build, name: 'test2') + end + + it 'returns all needs attributes' do + expect(build.needs_attributes).to contain_exactly( + { 'artifacts' => true, 'name' => 'test1' }, + { 'artifacts' => true, 'name' => 'test2' } + ) + end + end + + context 'without needs' do + it 'returns all needs attributes' do + expect(build.needs_attributes).to be_empty + end + end + end end diff --git a/spec/models/concerns/reactive_caching_spec.rb b/spec/models/concerns/reactive_caching_spec.rb index 6d4eeae641f..96a9c317fb8 100644 --- a/spec/models/concerns/reactive_caching_spec.rb +++ b/spec/models/concerns/reactive_caching_spec.rb @@ -112,6 +112,43 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do end end + describe '#with_reactive_cache_set', :use_clean_rails_redis_caching do + subject(:go!) do + instance.with_reactive_cache_set('resource', {}) do |data| + data + end + end + + it 'calls with_reactive_cache' do + expect(instance) + .to receive(:with_reactive_cache) + + go! + end + + context 'data returned' do + let(:resource) { 'resource' } + let(:set_key) { "#{cache_key}:#{resource}" } + let(:set_cache) { Gitlab::ReactiveCacheSetCache.new } + + before do + stub_reactive_cache(instance, true, resource, {}) + end + + it 'saves keys in set' do + expect(set_cache.read(set_key)).to be_empty + + go! + + expect(set_cache.read(set_key)).not_to be_empty + end + + it 'returns the data' do + expect(go!).to eq(true) + end + end + end + describe '.reactive_cache_worker_finder' do context 'with default reactive_cache_worker_finder' do let(:args) { %w(other args) } diff --git a/spec/models/error_tracking/project_error_tracking_setting_spec.rb b/spec/models/error_tracking/project_error_tracking_setting_spec.rb index e81480ab88f..ef3679da71f 100644 --- a/spec/models/error_tracking/project_error_tracking_setting_spec.rb +++ b/spec/models/error_tracking/project_error_tracking_setting_spec.rb @@ -8,7 +8,7 @@ describe ErrorTracking::ProjectErrorTrackingSetting do let_it_be(:project) { create(:project) } - subject { create(:project_error_tracking_setting, project: project) } + subject(:setting) { create(:project_error_tracking_setting, project: project) } describe 'Associations' do it { is_expected.to belong_to(:project) } @@ -453,4 +453,23 @@ describe ErrorTracking::ProjectErrorTrackingSetting do end end end + + describe '#expire_issues_cache', :use_clean_rails_redis_caching do + let(:issues) { [:some, :issues] } + let(:opt) { 'list_issues' } + let(:params) { { issue_status: 'unresolved', limit: 20, sort: 'last_seen' } } + + before do + start_reactive_cache_lifetime(subject, opt, params.stringify_keys) + stub_reactive_cache(subject, issues, opt, params.stringify_keys) + end + + it 'clears the cache' do + expect(subject.list_sentry_issues(params)).to eq(issues) + + subject.expire_issues_cache + + expect(subject.list_sentry_issues(params)).to eq(nil) + end + end end diff --git a/spec/requests/api/markdown_spec.rb b/spec/requests/api/markdown_spec.rb index 8a1e1b05c9a..9b787e76740 100644 --- a/spec/requests/api/markdown_spec.rb +++ b/spec/requests/api/markdown_spec.rb @@ -16,7 +16,7 @@ describe API::Markdown do shared_examples "rendered markdown text without GFM" do it "renders markdown text" do - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(:created) expect(response.headers["Content-Type"]).to eq("application/json") expect(json_response).to be_a(Hash) expect(json_response["html"]).to eq("<p>#{text}</p>") @@ -25,7 +25,7 @@ describe API::Markdown do shared_examples "404 Project Not Found" do it "responses with 404 Not Found" do - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) expect(response.headers["Content-Type"]).to eq("application/json") expect(json_response).to be_a(Hash) expect(json_response["message"]).to eq("404 Project Not Found") @@ -37,7 +37,7 @@ describe API::Markdown do let(:params) { {} } it "responses with 400 Bad Request" do - expect(response).to have_http_status(400) + expect(response).to have_gitlab_http_status(:bad_request) expect(response.headers["Content-Type"]).to eq("application/json") expect(json_response).to be_a(Hash) expect(json_response["error"]).to eq("text is missing") @@ -83,7 +83,7 @@ describe API::Markdown do let(:params) { { text: text, gfm: true } } it "renders markdown text" do - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(:created) expect(response.headers["Content-Type"]).to eq("application/json") expect(json_response).to be_a(Hash) expect(json_response["html"]).to include("Hello world!") @@ -100,7 +100,7 @@ describe API::Markdown do let(:user) { project.owner } it "renders markdown text" do - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(:created) expect(response.headers["Content-Type"]).to eq("application/json") expect(json_response).to be_a(Hash) expect(json_response["html"]).to include("Hello world!") @@ -120,7 +120,7 @@ describe API::Markdown do shared_examples 'user without proper access' do it 'does not render the title or link' do - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(:created) expect(json_response["html"]).not_to include('Confidential title') expect(json_response["html"]).not_to include('<a href=') expect(json_response["html"]).to include('Hello world!') @@ -146,7 +146,7 @@ describe API::Markdown do let(:user) { confidential_issue.author } it 'renders the title or link' do - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(:created) expect(json_response["html"]).to include('Confidential title') expect(json_response["html"]).to include('Hello world!') .and include('data-name="tada"') diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index f2942020e16..0b504df1f51 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -43,7 +43,7 @@ describe API::Members do get api(members_url, user) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.size).to eq(2) @@ -72,7 +72,7 @@ describe API::Members do get api(members_url, developer) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.size).to eq(2) @@ -82,7 +82,7 @@ describe API::Members do it 'finds members with query string' do get api(members_url, developer), params: { query: maintainer.username } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.count).to eq(1) @@ -92,7 +92,7 @@ describe API::Members do it 'finds members with the given user_ids' do get api(members_url, developer), params: { user_ids: [maintainer.id, developer.id, stranger.id] } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.map { |u| u['id'] }).to contain_exactly(maintainer.id, developer.id) @@ -101,7 +101,7 @@ describe API::Members do it 'finds all members with no query specified' do get api(members_url, developer), params: { query: '' } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.count).to eq(2) @@ -137,7 +137,7 @@ describe API::Members do it 'finds all project members including inherited members' do get api("/projects/#{project.id}/members/all", developer) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.map { |u| u['id'] }).to match_array [maintainer.id, developer.id, nested_user.id, project_user.id, linked_group_user.id] @@ -148,7 +148,7 @@ describe API::Members do get api("/projects/#{project.id}/members/all", developer) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array @@ -165,7 +165,7 @@ describe API::Members do it 'finds all group members including inherited members' do get api("/groups/#{nested_group.id}/members/all", developer) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.map { |u| u['id'] }).to match_array [maintainer.id, developer.id, nested_user.id] @@ -185,7 +185,7 @@ describe API::Members do user = public_send(type) get api("/#{source_type.pluralize}/#{source.id}/members/#{all ? 'all/' : ''}#{developer.id}", user) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) # User attributes expect(json_response['id']).to eq(developer.id) expect(json_response['name']).to eq(developer.name) @@ -220,7 +220,7 @@ describe API::Members do post api("/#{source_type.pluralize}/#{source.id}/members", user), params: { user_id: access_requester.id, access_level: Member::MAINTAINER } - expect(response).to have_gitlab_http_status(403) + expect(response).to have_gitlab_http_status(:forbidden) end end end @@ -233,7 +233,7 @@ describe API::Members do post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), params: { user_id: access_requester.id, access_level: Member::MAINTAINER } - expect(response).to have_gitlab_http_status(201) + expect(response).to have_gitlab_http_status(:created) end.to change { source.members.count }.by(1) expect(source.requesters.count).to eq(0) expect(json_response['id']).to eq(access_requester.id) @@ -246,7 +246,7 @@ describe API::Members do post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), params: { user_id: stranger.id, access_level: Member::DEVELOPER, expires_at: '2016-08-05' } - expect(response).to have_gitlab_http_status(201) + expect(response).to have_gitlab_http_status(:created) end.to change { source.members.count }.by(1) expect(json_response['id']).to eq(stranger.id) expect(json_response['access_level']).to eq(Member::DEVELOPER) @@ -265,7 +265,7 @@ describe API::Members do post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), params: { user_id: stranger.id, access_level: Member::REPORTER } - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(:bad_request) expect(json_response['message']['access_level']).to eq(["should be greater than or equal to Developer inherited membership from group #{parent.name}"]) end @@ -279,7 +279,7 @@ describe API::Members do post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), params: { user_id: stranger.id, access_level: Member::MAINTAINER } - expect(response).to have_gitlab_http_status(201) + expect(response).to have_gitlab_http_status(:created) expect(json_response['id']).to eq(stranger.id) expect(json_response['access_level']).to eq(Member::MAINTAINER) end @@ -289,14 +289,14 @@ describe API::Members do post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), params: { user_id: maintainer.id, access_level: Member::MAINTAINER } - expect(response).to have_gitlab_http_status(409) + expect(response).to have_gitlab_http_status(:conflict) end it 'returns 404 when the user_id is not valid' do post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), params: { user_id: 0, access_level: Member::MAINTAINER } - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 User Not Found') end @@ -304,21 +304,21 @@ describe API::Members do post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), params: { access_level: Member::MAINTAINER } - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(:bad_request) end it 'returns 400 when access_level is not given' do post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), params: { user_id: stranger.id } - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(:bad_request) end it 'returns 400 when access_level is not valid' do post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), params: { user_id: stranger.id, access_level: 1234 } - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(:bad_request) end end end @@ -340,7 +340,7 @@ describe API::Members do put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", user), params: { access_level: Member::MAINTAINER } - expect(response).to have_gitlab_http_status(403) + expect(response).to have_gitlab_http_status(:forbidden) end end end @@ -351,7 +351,7 @@ describe API::Members do put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", maintainer), params: { access_level: Member::MAINTAINER, expires_at: '2016-08-05' } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['id']).to eq(developer.id) expect(json_response['access_level']).to eq(Member::MAINTAINER) expect(json_response['expires_at']).to eq('2016-08-05') @@ -362,20 +362,20 @@ describe API::Members do put api("/#{source_type.pluralize}/#{source.id}/members/123", maintainer), params: { access_level: Member::MAINTAINER } - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end it 'returns 400 when access_level is not given' do put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", maintainer) - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(:bad_request) end it 'returns 400 when access level is not valid' do put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", maintainer), params: { access_level: 1234 } - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(:bad_request) end end end @@ -393,7 +393,7 @@ describe API::Members do user = public_send(type) delete api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", user) - expect(response).to have_gitlab_http_status(403) + expect(response).to have_gitlab_http_status(:forbidden) end end end @@ -404,7 +404,7 @@ describe API::Members do expect do delete api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", developer) - expect(response).to have_gitlab_http_status(204) + expect(response).to have_gitlab_http_status(:no_content) end.to change { source.members.count }.by(-1) end end @@ -415,7 +415,7 @@ describe API::Members do expect do delete api("/#{source_type.pluralize}/#{source.id}/members/#{access_requester.id}", maintainer) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end.not_to change { source.requesters.count } end end @@ -424,7 +424,7 @@ describe API::Members do expect do delete api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", maintainer) - expect(response).to have_gitlab_http_status(204) + expect(response).to have_gitlab_http_status(:no_content) end.to change { source.members.count }.by(-1) end @@ -436,7 +436,7 @@ describe API::Members do it 'returns 404 if member does not exist' do delete api("/#{source_type.pluralize}/#{source.id}/members/123", maintainer) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end end end @@ -491,7 +491,7 @@ describe API::Members do post api("/projects/#{project.id}/members", maintainer), params: { user_id: stranger.id, access_level: Member::OWNER } - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(:bad_request) end.to change { project.members.count }.by(0) end end diff --git a/spec/requests/api/merge_request_diffs_spec.rb b/spec/requests/api/merge_request_diffs_spec.rb index 9de76c2fe50..2e74022ae07 100644 --- a/spec/requests/api/merge_request_diffs_spec.rb +++ b/spec/requests/api/merge_request_diffs_spec.rb @@ -28,12 +28,12 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs' do it 'returns a 404 when merge_request id is used instead of the iid' do get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions", user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end it 'returns a 404 when merge_request_iid not found' do get api("/projects/#{project.id}/merge_requests/0/versions", user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end end @@ -51,17 +51,17 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs' do it 'returns a 404 when merge_request id is used instead of the iid' do get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions/#{merge_request_diff.id}", user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end it 'returns a 404 when merge_request version_id is not found' do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/versions/0", user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end it 'returns a 404 when merge_request_iid is not found' do get api("/projects/#{project.id}/merge_requests/12345/versions/#{merge_request_diff.id}", user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 862afd11b86..f0ab2f26900 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -194,7 +194,7 @@ describe API::MergeRequests do it 'matches V4 response schema' do get api(endpoint_path, user) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('public_api/v4/merge_requests') end @@ -454,25 +454,25 @@ describe API::MergeRequests do it "returns authentication error without any scope" do get api("/merge_requests") - expect(response).to have_gitlab_http_status(401) + expect(response).to have_gitlab_http_status(:unauthorized) end it "returns authentication error when scope is assigned-to-me" do get api("/merge_requests"), params: { scope: 'assigned-to-me' } - expect(response).to have_gitlab_http_status(401) + expect(response).to have_gitlab_http_status(:unauthorized) end it "returns authentication error when scope is assigned_to_me" do get api("/merge_requests"), params: { scope: 'assigned_to_me' } - expect(response).to have_gitlab_http_status(401) + expect(response).to have_gitlab_http_status(:unauthorized) end it "returns authentication error when scope is created-by-me" do get api("/merge_requests"), params: { scope: 'created-by-me' } - expect(response).to have_gitlab_http_status(401) + expect(response).to have_gitlab_http_status(:unauthorized) end end @@ -705,7 +705,7 @@ describe API::MergeRequests do get api("/projects/#{project.id}/merge_requests") - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end it "returns an array of no merge_requests when wip=yes" do @@ -818,7 +818,7 @@ describe API::MergeRequests do merge_request = create(:merge_request, :with_test_reports, milestone: milestone1, author: user, assignees: [user], source_project: project, target_project: project, title: "Test", created_at: base_time) get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('public_api/v4/merge_request') end @@ -828,7 +828,7 @@ describe API::MergeRequests do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['id']).to eq(merge_request.id) expect(json_response['iid']).to eq(merge_request.iid) expect(json_response['project_id']).to eq(merge_request.project.id) @@ -866,7 +866,7 @@ describe API::MergeRequests do it 'exposes description and title html when render_html is true' do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: { render_html: true } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response).to include('title_html', 'description_html') end @@ -874,7 +874,7 @@ describe API::MergeRequests do it 'exposes rebase_in_progress when include_rebase_in_progress is true' do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: { include_rebase_in_progress: true } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response).to include('rebase_in_progress') end @@ -950,19 +950,19 @@ describe API::MergeRequests do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: { include_diverged_commits_count: true } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['diverged_commits_count']).to eq(1) end it "returns a 404 error if merge_request_iid not found" do get api("/projects/#{project.id}/merge_requests/0", user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end it "returns a 404 error if merge_request `id` is used instead of iid" do get api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end context 'Work in Progress' do @@ -971,7 +971,7 @@ describe API::MergeRequests do it "returns merge request" do get api("/projects/#{project.id}/merge_requests/#{merge_request_wip.iid}", user) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['work_in_progress']).to eq(true) end end @@ -990,7 +990,7 @@ describe API::MergeRequests do get api("/projects/#{project.id}/merge_requests/#{merge_request_overflow.iid}", user) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['changes_count']).to eq('5+') end end @@ -1059,13 +1059,13 @@ describe API::MergeRequests do it 'returns a 404 when merge_request_iid not found' do get api("/projects/#{project.id}/merge_requests/0/commits", user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end it 'returns a 404 when merge_request id is used instead of iid' do get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/commits", user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end end @@ -1083,7 +1083,7 @@ describe API::MergeRequests do it 'returns a 404 when merge_request_iid not found' do get api("/projects/#{project.id}/merge_requests/0/context_commits", user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end end @@ -1091,19 +1091,19 @@ describe API::MergeRequests do it 'returns the change information of the merge_request' do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/changes", user) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['changes'].size).to eq(merge_request.diffs.size) end it 'returns a 404 when merge_request_iid not found' do get api("/projects/#{project.id}/merge_requests/0/changes", user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end it 'returns a 404 when merge_request id is used instead of iid' do get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/changes", user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end end @@ -1123,14 +1123,14 @@ describe API::MergeRequests do it 'exposes basic attributes' do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/pipelines") - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('public_api/v4/pipelines') end it 'returns 404 if MR does not exist' do get api("/projects/#{project.id}/merge_requests/777/pipelines") - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end end @@ -1143,7 +1143,7 @@ describe API::MergeRequests do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/pipelines", guest) - expect(response).to have_gitlab_http_status(403) + expect(response).to have_gitlab_http_status(:forbidden) end end end @@ -1186,7 +1186,7 @@ describe API::MergeRequests do context 'when authorized' do it 'creates and returns the new Pipeline' do expect { request }.to change(Ci::Pipeline, :count).by(1) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_a Hash end end @@ -1196,7 +1196,7 @@ describe API::MergeRequests do it 'responds with a blank 404' do expect { request }.not_to change(Ci::Pipeline, :count) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end end @@ -1205,7 +1205,7 @@ describe API::MergeRequests do it 'responds with a blank 404' do expect { request }.not_to change(Ci::Pipeline, :count) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end end end @@ -1225,7 +1225,7 @@ describe API::MergeRequests do it 'creates a new merge request' do post api("/projects/#{project.id}/merge_requests", user), params: params - expect(response).to have_gitlab_http_status(201) + expect(response).to have_gitlab_http_status(:created) expect(json_response['title']).to eq('Test merge request') expect(json_response['assignee']['name']).to eq(user2.name) expect(json_response['assignees'].first['name']).to eq(user2.name) @@ -1236,7 +1236,7 @@ describe API::MergeRequests do post api("/projects/#{project.id}/merge_requests", user), params: params - expect(response).to have_gitlab_http_status(201) + expect(response).to have_gitlab_http_status(:created) expect(json_response['title']).to eq('Test merge request') expect(json_response['assignee']).to be_nil end @@ -1249,7 +1249,7 @@ describe API::MergeRequests do post api("/projects/#{private_project.id}/merge_requests", user), params: params - expect(response).to have_gitlab_http_status(201) + expect(response).to have_gitlab_http_status(:created) expect(json_response['assignee']).to be_nil end end @@ -1268,7 +1268,7 @@ describe API::MergeRequests do it 'creates a new project merge request with no more than one assignee' do post api("/projects/#{project.id}/merge_requests", user), params: params - expect(response).to have_gitlab_http_status(201) + expect(response).to have_gitlab_http_status(:created) expect(json_response['title']).to eq('Test merge request') expect(json_response['assignees'].count).to eq(1) expect(json_response['assignees'].first['name']).to eq(user.name) @@ -1294,7 +1294,7 @@ describe API::MergeRequests do params[:labels] = labels post api("/projects/#{project.id}/merge_requests", user), params: params - expect(response).to have_gitlab_http_status(201) + expect(response).to have_gitlab_http_status(:created) expect(json_response['title']).to eq('Test merge_request') expect(json_response['labels']).to eq(%w(label label2)) expect(json_response['milestone']['id']).to eq(milestone.id) @@ -1319,7 +1319,7 @@ describe API::MergeRequests do params[:labels] = 'label, label?, label&foo, ?, &' post api("/projects/#{project.id}/merge_requests", user), params: params - expect(response).to have_gitlab_http_status(201) + expect(response).to have_gitlab_http_status(:created) expect(json_response['labels']).to include 'label' expect(json_response['labels']).to include 'label?' expect(json_response['labels']).to include 'label&foo' @@ -1331,7 +1331,7 @@ describe API::MergeRequests do params[:labels] = ['label', 'label?', 'label&foo, ?, &', '1, 2', 3, 4] post api("/projects/#{project.id}/merge_requests", user), params: params - expect(response).to have_gitlab_http_status(201) + expect(response).to have_gitlab_http_status(:created) expect(json_response['labels']).to include 'label' expect(json_response['labels']).to include 'label?' expect(json_response['labels']).to include 'label&foo' @@ -1347,7 +1347,7 @@ describe API::MergeRequests do params[:labels] = '' post api("/projects/#{project.id}/merge_requests", user), params: params - expect(response).to have_gitlab_http_status(201) + expect(response).to have_gitlab_http_status(:created) expect(json_response['labels']).to eq([]) end @@ -1357,7 +1357,7 @@ describe API::MergeRequests do params: params.to_json, headers: { 'Content-Type': 'application/json' } - expect(response).to have_gitlab_http_status(201) + expect(response).to have_gitlab_http_status(:created) expect(json_response['labels']).to eq([]) end @@ -1365,7 +1365,7 @@ describe API::MergeRequests do params[:labels] = [] post api("/projects/#{project.id}/merge_requests", user), params: params - expect(response).to have_gitlab_http_status(201) + expect(response).to have_gitlab_http_status(:created) expect(json_response['labels']).to eq([]) end @@ -1373,7 +1373,7 @@ describe API::MergeRequests do params[:labels] = [''] post api("/projects/#{project.id}/merge_requests", user), params: params - expect(response).to have_gitlab_http_status(201) + expect(response).to have_gitlab_http_status(:created) expect(json_response['labels']).to eq([]) end @@ -1381,7 +1381,7 @@ describe API::MergeRequests do params[:labels] = ['', '', ''] post api("/projects/#{project.id}/merge_requests", user), params: params - expect(response).to have_gitlab_http_status(201) + expect(response).to have_gitlab_http_status(:created) expect(json_response['labels']).to eq([]) end end @@ -1389,25 +1389,25 @@ describe API::MergeRequests do it "returns 422 when source_branch equals target_branch" do post api("/projects/#{project.id}/merge_requests", user), params: { title: "Test merge_request", source_branch: "master", target_branch: "master", author: user } - expect(response).to have_gitlab_http_status(422) + expect(response).to have_gitlab_http_status(:unprocessable_entity) end it "returns 400 when source_branch is missing" do post api("/projects/#{project.id}/merge_requests", user), params: { title: "Test merge_request", target_branch: "master", author: user } - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(:bad_request) end it "returns 400 when target_branch is missing" do post api("/projects/#{project.id}/merge_requests", user), params: { title: "Test merge_request", source_branch: "markdown", author: user } - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(:bad_request) end it "returns 400 when title is missing" do post api("/projects/#{project.id}/merge_requests", user), params: { target_branch: 'master', source_branch: 'markdown' } - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(:bad_request) end context 'with existing MR' do @@ -1432,7 +1432,7 @@ describe API::MergeRequests do author: user } end.to change { MergeRequest.count }.by(0) - expect(response).to have_gitlab_http_status(409) + expect(response).to have_gitlab_http_status(:conflict) end end @@ -1471,7 +1471,7 @@ describe API::MergeRequests do it "returns merge_request" do post api("/projects/#{forked_project.id}/merge_requests", user2), params: { title: 'Test merge_request', source_branch: "feature_conflict", target_branch: "master", author: user2, target_project_id: project.id, description: 'Test description for Test merge_request' } - expect(response).to have_gitlab_http_status(201) + expect(response).to have_gitlab_http_status(:created) expect(json_response['title']).to eq('Test merge_request') expect(json_response['description']).to eq('Test description for Test merge_request') end @@ -1482,7 +1482,7 @@ describe API::MergeRequests do expect(forked_project.forked_from_project).to eq(project) post api("/projects/#{forked_project.id}/merge_requests", user2), params: { title: 'Test merge_request', source_branch: "master", target_branch: "master", author: user2, target_project_id: project.id } - expect(response).to have_gitlab_http_status(201) + expect(response).to have_gitlab_http_status(:created) expect(json_response['title']).to eq('Test merge_request') end @@ -1498,31 +1498,31 @@ describe API::MergeRequests do target_project_id: project.id } - expect(response).to have_gitlab_http_status(403) + expect(response).to have_gitlab_http_status(:forbidden) end it "returns 400 when source_branch is missing" do post api("/projects/#{forked_project.id}/merge_requests", user2), params: { title: 'Test merge_request', target_branch: "master", author: user2, target_project_id: project.id } - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(:bad_request) end it "returns 400 when target_branch is missing" do post api("/projects/#{forked_project.id}/merge_requests", user2), params: { title: 'Test merge_request', target_branch: "master", author: user2, target_project_id: project.id } - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(:bad_request) end it "returns 400 when title is missing" do post api("/projects/#{forked_project.id}/merge_requests", user2), params: { target_branch: 'master', source_branch: 'markdown', author: user2, target_project_id: project.id } - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(:bad_request) end it 'allows setting `allow_collaboration`', :sidekiq_might_not_need_inline do post api("/projects/#{forked_project.id}/merge_requests", user2), params: { title: 'Test merge_request', source_branch: "feature_conflict", target_branch: "master", author: user2, target_project_id: project.id, allow_collaboration: true } - expect(response).to have_gitlab_http_status(201) + expect(response).to have_gitlab_http_status(:created) expect(json_response['allow_collaboration']).to be_truthy expect(json_response['allow_maintainer_to_push']).to be_truthy end @@ -1541,20 +1541,20 @@ describe API::MergeRequests do post api("/projects/#{forked_project.id}/merge_requests", user2), params: params - expect(response).to have_gitlab_http_status(422) + expect(response).to have_gitlab_http_status(:unprocessable_entity) end it 'returns 403 if targeting a different fork which user can not access' do post api("/projects/#{forked_project.id}/merge_requests", user2), params: params - expect(response).to have_gitlab_http_status(403) + expect(response).to have_gitlab_http_status(:forbidden) end end it "returns 201 when target_branch is specified and for the same project", :sidekiq_might_not_need_inline do post api("/projects/#{forked_project.id}/merge_requests", user2), params: { title: 'Test merge_request', target_branch: 'master', source_branch: 'markdown', author: user2, target_project_id: forked_project.id } - expect(response).to have_gitlab_http_status(201) + expect(response).to have_gitlab_http_status(:created) end end end @@ -1585,7 +1585,7 @@ describe API::MergeRequests do describe 'when authenticated' do it 'creates and returns the new context commit' do post api("/projects/#{project.id}/merge_requests/#{merge_request_iid}/context_commits", authenticated_user), params: params - expect(response).to have_gitlab_http_status(201) + expect(response).to have_gitlab_http_status(:created) expect(json_response).to be_an Array expect(json_response.first['short_id']).to eq(commit.short_id) expect(json_response.first['title']).to eq(commit.title) @@ -1602,46 +1602,46 @@ describe API::MergeRequests do end it 'returns 400 when the context commit is already created' do post api("/projects/#{project.id}/merge_requests/#{merge_request_iid}/context_commits", authenticated_user), params: params - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(:bad_request) expect(json_response['message']).to eq("Context commits: [\"#{commit.id}\"] are already created") end end it 'returns 400 when one or more shas are invalid' do post api("/projects/#{project.id}/merge_requests/#{merge_request_iid}/context_commits", authenticated_user), params: params_invalid_shas - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(:bad_request) expect(json_response['message']).to eq('One or more context commits\' sha is not valid.') end it 'returns 400 when the commits are empty' do post api("/projects/#{project.id}/merge_requests/#{merge_request_iid}/context_commits", authenticated_user), params: params_empty_commits - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(:bad_request) end it 'returns 400 when params is empty' do post api("/projects/#{project.id}/merge_requests/#{merge_request_iid}/context_commits", authenticated_user) - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(:bad_request) end it 'returns 403 when creating new context commit for guest role' do guest = create(:user) project.add_guest(guest) post api("/projects/#{project.id}/merge_requests/#{merge_request_iid}/context_commits", guest), params: params - expect(response).to have_gitlab_http_status(403) + expect(response).to have_gitlab_http_status(:forbidden) end it 'returns 403 when creating new context commit for reporter role' do reporter = create(:user) project.add_reporter(reporter) post api("/projects/#{project.id}/merge_requests/#{merge_request_iid}/context_commits", reporter), params: params - expect(response).to have_gitlab_http_status(403) + expect(response).to have_gitlab_http_status(:forbidden) end end context 'when unauthenticated' do it 'returns 401 if user tries to create context commits' do post api("/projects/#{project.id}/merge_requests/#{merge_request_iid}/context_commits"), params: params - expect(response).to have_gitlab_http_status(401) + expect(response).to have_gitlab_http_status(:unauthorized) end end end @@ -1656,7 +1656,7 @@ describe API::MergeRequests do it "denies the deletion of the merge request" do delete api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", developer) - expect(response).to have_gitlab_http_status(403) + expect(response).to have_gitlab_http_status(:forbidden) end end @@ -1664,19 +1664,19 @@ describe API::MergeRequests do it "destroys the merge request owners can destroy" do delete api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) - expect(response).to have_gitlab_http_status(204) + expect(response).to have_gitlab_http_status(:no_content) end it "returns 404 for an invalid merge request IID" do delete api("/projects/#{project.id}/merge_requests/12345", user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end it "returns 404 if the merge request id is used instead of iid" do delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end it_behaves_like '412 response' do @@ -1712,40 +1712,40 @@ describe API::MergeRequests do it "deletes context commit" do delete api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/context_commits", authenticated_user), params: params - expect(response).to have_gitlab_http_status(204) + expect(response).to have_gitlab_http_status(:no_content) end it "returns 400 when invalid commit sha is passed" do delete api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/context_commits", authenticated_user), params: params_invalid_shas - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(:bad_request) expect(json_response["message"]).to eq('One or more context commits\' sha is not valid.') end it "returns 400 when commits is empty" do delete api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/context_commits", authenticated_user), params: params_empty_commits - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(:bad_request) end it "returns 400 when no params is passed" do delete api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/context_commits", authenticated_user) - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(:bad_request) end it 'returns 403 when deleting existing context commit for guest role' do guest = create(:user) project.add_guest(guest) delete api("/projects/#{project.id}/merge_requests/#{merge_request_iid}/context_commits", guest), params: params - expect(response).to have_gitlab_http_status(403) + expect(response).to have_gitlab_http_status(:forbidden) end it 'returns 403 when deleting existing context commit for reporter role' do reporter = create(:user) project.add_reporter(reporter) delete api("/projects/#{project.id}/merge_requests/#{merge_request_iid}/context_commits", reporter), params: params - expect(response).to have_gitlab_http_status(403) + expect(response).to have_gitlab_http_status(:forbidden) end end @@ -1753,7 +1753,7 @@ describe API::MergeRequests do it "returns 401, unauthorised error" do delete api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/context_commits") - expect(response).to have_gitlab_http_status(401) + expect(response).to have_gitlab_http_status(:unauthorized) end end end @@ -1764,7 +1764,7 @@ describe API::MergeRequests do it "returns merge_request in case of success" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) end it "returns 406 if branch can't be merged" do @@ -1773,21 +1773,21 @@ describe API::MergeRequests do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) - expect(response).to have_gitlab_http_status(406) + expect(response).to have_gitlab_http_status(:not_acceptable) expect(json_response['message']).to eq('Branch cannot be merged') end it "returns 405 if merge_request is not open" do merge_request.close put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) - expect(response).to have_gitlab_http_status(405) + expect(response).to have_gitlab_http_status(:method_not_allowed) expect(json_response['message']).to eq('405 Method Not Allowed') end it "returns 405 if merge_request is a work in progress" do merge_request.update_attribute(:title, "WIP: #{merge_request.title}") put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) - expect(response).to have_gitlab_http_status(405) + expect(response).to have_gitlab_http_status(:method_not_allowed) expect(json_response['message']).to eq('405 Method Not Allowed') end @@ -1801,7 +1801,7 @@ describe API::MergeRequests do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) - expect(response).to have_gitlab_http_status(405) + expect(response).to have_gitlab_http_status(:method_not_allowed) expect(json_response['message']).to eq('405 Method Not Allowed') end @@ -1809,21 +1809,21 @@ describe API::MergeRequests do user2 = create(:user) project.add_reporter(user2) put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user2) - expect(response).to have_gitlab_http_status(401) + expect(response).to have_gitlab_http_status(:unauthorized) expect(json_response['message']).to eq('401 Unauthorized') end it "returns 409 if the SHA parameter doesn't match" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), params: { sha: merge_request.diff_head_sha.reverse } - expect(response).to have_gitlab_http_status(409) + expect(response).to have_gitlab_http_status(:conflict) expect(json_response['message']).to start_with('SHA does not match HEAD of source branch') end it "succeeds if the SHA parameter matches" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), params: { sha: merge_request.diff_head_sha } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) end it "updates the MR's squash attribute" do @@ -1831,7 +1831,7 @@ describe API::MergeRequests do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), params: { squash: true } end.to change { merge_request.reload.squash } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) end it 'does not merge if merge_when_pipeline_succeeds is passed and the pipeline has failed' do @@ -1842,7 +1842,7 @@ describe API::MergeRequests do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), params: { merge_when_pipeline_succeeds: true } - expect(response).to have_gitlab_http_status(405) + expect(response).to have_gitlab_http_status(:method_not_allowed) expect(merge_request.reload.state).to eq('opened') end @@ -1851,7 +1851,7 @@ describe API::MergeRequests do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), params: { merge_when_pipeline_succeeds: true } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['state']).to eq('merged') end @@ -1861,7 +1861,7 @@ describe API::MergeRequests do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), params: { merge_when_pipeline_succeeds: true } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['title']).to eq('Test') expect(json_response['merge_when_pipeline_succeeds']).to eq(true) end @@ -1873,7 +1873,7 @@ describe API::MergeRequests do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), params: { merge_when_pipeline_succeeds: true } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['title']).to eq('Test') expect(json_response['merge_when_pipeline_succeeds']).to eq(true) end @@ -1881,13 +1881,13 @@ describe API::MergeRequests do it "returns 404 for an invalid merge request IID" do put api("/projects/#{project.id}/merge_requests/12345/merge", user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end it "returns 404 if the merge request id is used instead of iid" do put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end describe "the squash_commit_message param" do @@ -1923,7 +1923,7 @@ describe API::MergeRequests do params: { should_remove_source_branch: true } ) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(source_repository.branch_exists?(source_branch)).to be_falsy end end @@ -1938,7 +1938,7 @@ describe API::MergeRequests do it "records the squash commit SHA and returns it in the response" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['squash_commit_sha'].length).to eq(40) end end @@ -1958,7 +1958,7 @@ describe API::MergeRequests do it 'returns the generated ID from the merge service in case of success' do get api(url, user) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['commit_id']).to eq(merge_request.merge_ref_head.sha) end @@ -1970,7 +1970,7 @@ describe API::MergeRequests do it 'returns 200 if MR can be merged' do get api(url, user) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['commit_id']).to eq(merge_request.merge_ref_head.sha) end @@ -1981,7 +1981,7 @@ describe API::MergeRequests do get api(url, user) - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(:bad_request) expect(json_response['message']).to eq('Merge request is not mergeable') end end @@ -1995,7 +1995,7 @@ describe API::MergeRequests do get api(url, user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Not found') end end @@ -2006,7 +2006,7 @@ describe API::MergeRequests do it 'returns 404' do get api(url, user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end end @@ -2016,7 +2016,7 @@ describe API::MergeRequests do it 'returns 404' do get api(url, user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end end end @@ -2030,7 +2030,7 @@ describe API::MergeRequests do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: { state_event: "close", remove_source_branch: false } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['state']).to eq('closed') expect(json_response['force_remove_source_branch']).to be_falsey end @@ -2042,7 +2042,7 @@ describe API::MergeRequests do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: { state_event: "close", remove_source_branch: true } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['state']).to eq('closed') expect(json_response['force_remove_source_branch']).to be_truthy end @@ -2063,7 +2063,7 @@ describe API::MergeRequests do it 'is true for an authorized user' do put api("/projects/#{target_project.id}/merge_requests/#{merge_request.iid}", fork_owner), params: { state_event: 'close', remove_source_branch: true } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['state']).to eq('closed') expect(json_response['force_remove_source_branch']).to be true end @@ -2073,7 +2073,7 @@ describe API::MergeRequests do put api("/projects/#{target_project.id}/merge_requests/#{merge_request.iid}", target_project.owner), params: { state_event: 'close', remove_source_branch: true } end.not_to change { merge_request.reload.merge_params } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['state']).to eq('closed') expect(json_response['force_remove_source_branch']).to be false end @@ -2084,46 +2084,46 @@ describe API::MergeRequests do it "returns merge_request" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: { state_event: "close" } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['state']).to eq('closed') end end it "updates title and returns merge_request" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: { title: "New title" } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['title']).to eq('New title') end it "updates description and returns merge_request" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: { description: "New description" } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['description']).to eq('New description') end it "updates milestone_id and returns merge_request" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: { milestone_id: milestone.id } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['milestone']['id']).to eq(milestone.id) end it "updates squash and returns merge_request" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: { squash: true } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['squash']).to be_truthy end it "returns merge_request with renamed target_branch" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: { target_branch: "wiki" } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['target_branch']).to eq('wiki') end it "returns merge_request that removes the source branch" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: { remove_source_branch: true } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['force_remove_source_branch']).to be_truthy end @@ -2136,7 +2136,7 @@ describe API::MergeRequests do put api("/projects/#{private_project.id}/merge_requests/#{mr.iid}", user), params: params - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['assignee']).to be_nil end @@ -2236,7 +2236,7 @@ describe API::MergeRequests do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: { state_event: 'close', title: nil } merge_request.reload - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(:bad_request) expect(merge_request.state).to eq('opened') end @@ -2244,20 +2244,20 @@ describe API::MergeRequests do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: { state_event: 'close', target_branch: nil } merge_request.reload - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(:bad_request) expect(merge_request.state).to eq('opened') end it "returns 404 for an invalid merge request IID" do put api("/projects/#{project.id}/merge_requests/12345", user), params: { state_event: "close" } - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end it "returns 404 if the merge request id is used instead of iid" do put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), params: { state_event: "close" } - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end end @@ -2310,19 +2310,19 @@ describe API::MergeRequests do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/closes_issues", guest) - expect(response).to have_gitlab_http_status(403) + expect(response).to have_gitlab_http_status(:forbidden) end it "returns 404 for an invalid merge request IID" do get api("/projects/#{project.id}/merge_requests/12345/closes_issues", user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end it "returns 404 if the merge request id is used instead of iid" do get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/closes_issues", user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end end @@ -2330,26 +2330,26 @@ describe API::MergeRequests do it 'subscribes to a merge request' do post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/subscribe", admin) - expect(response).to have_gitlab_http_status(201) + expect(response).to have_gitlab_http_status(:created) expect(json_response['subscribed']).to eq(true) end it 'returns 304 if already subscribed' do post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/subscribe", user) - expect(response).to have_gitlab_http_status(304) + expect(response).to have_gitlab_http_status(:not_modified) end it 'returns 404 if the merge request is not found' do post api("/projects/#{project.id}/merge_requests/123/subscribe", user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end it 'returns 404 if the merge request id is used instead of iid' do post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/subscribe", user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end it 'returns 403 if user has no access to read code' do @@ -2358,7 +2358,7 @@ describe API::MergeRequests do post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/subscribe", guest) - expect(response).to have_gitlab_http_status(403) + expect(response).to have_gitlab_http_status(:forbidden) end end @@ -2366,26 +2366,26 @@ describe API::MergeRequests do it 'unsubscribes from a merge request' do post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/unsubscribe", user) - expect(response).to have_gitlab_http_status(201) + expect(response).to have_gitlab_http_status(:created) expect(json_response['subscribed']).to eq(false) end it 'returns 304 if not subscribed' do post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/unsubscribe", admin) - expect(response).to have_gitlab_http_status(304) + expect(response).to have_gitlab_http_status(:not_modified) end it 'returns 404 if the merge request is not found' do post api("/projects/#{project.id}/merge_requests/123/unsubscribe", user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end it 'returns 404 if the merge request id is used instead of iid' do post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/unsubscribe", user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end it 'returns 403 if user has no access to read code' do @@ -2394,7 +2394,7 @@ describe API::MergeRequests do post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/unsubscribe", guest) - expect(response).to have_gitlab_http_status(403) + expect(response).to have_gitlab_http_status(:forbidden) end end @@ -2406,19 +2406,19 @@ describe API::MergeRequests do it 'removes the merge_when_pipeline_succeeds status' do post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/cancel_merge_when_pipeline_succeeds", user) - expect(response).to have_gitlab_http_status(201) + expect(response).to have_gitlab_http_status(:created) end it 'returns 404 if the merge request is not found' do post api("/projects/#{project.id}/merge_requests/123/cancel_merge_when_pipeline_succeeds", user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end it 'returns 404 if the merge request id is used instead of iid' do post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/cancel_merge_when_pipeline_succeeds", user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end end @@ -2431,7 +2431,7 @@ describe API::MergeRequests do end.to change { RebaseWorker.jobs.size }.by(1) end - expect(response).to have_gitlab_http_status(202) + expect(response).to have_gitlab_http_status(:accepted) expect(merge_request.reload).to be_rebase_in_progress expect(json_response['rebase_in_progress']).to be(true) end @@ -2446,7 +2446,7 @@ describe API::MergeRequests do end.to change { RebaseWorker.jobs.size }.by(1) end - expect(response).to have_gitlab_http_status(202) + expect(response).to have_gitlab_http_status(:accepted) expect(merge_request.reload).to be_rebase_in_progress expect(json_response['rebase_in_progress']).to be(true) end @@ -2459,7 +2459,7 @@ describe API::MergeRequests do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/rebase", guest) - expect(response).to have_gitlab_http_status(403) + expect(response).to have_gitlab_http_status(:forbidden) end it 'returns 409 if a rebase is already in progress' do @@ -2469,7 +2469,7 @@ describe API::MergeRequests do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/rebase", user) end - expect(response).to have_gitlab_http_status(409) + expect(response).to have_gitlab_http_status(:conflict) end it "returns 409 if rebase can't lock the row" do @@ -2478,7 +2478,7 @@ describe API::MergeRequests do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/rebase", user) - expect(response).to have_gitlab_http_status(409) + expect(response).to have_gitlab_http_status(:conflict) expect(json_response['message']).to eq('Failed to enqueue the rebase operation, possibly due to a long-lived transaction. Try again later.') end end diff --git a/spec/requests/api/namespaces_spec.rb b/spec/requests/api/namespaces_spec.rb index e0bf1509be3..68fffc638df 100644 --- a/spec/requests/api/namespaces_spec.rb +++ b/spec/requests/api/namespaces_spec.rb @@ -12,7 +12,7 @@ describe API::Namespaces do context "when unauthenticated" do it "returns authentication error" do get api("/namespaces") - expect(response).to have_gitlab_http_status(401) + expect(response).to have_gitlab_http_status(:unauthorized) end end @@ -23,7 +23,7 @@ describe API::Namespaces do group_kind_json_response = json_response.find { |resource| resource['kind'] == 'group' } user_kind_json_response = json_response.find { |resource| resource['kind'] == 'user' } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(group_kind_json_response.keys).to include('id', 'kind', 'name', 'path', 'full_path', 'parent_id', 'members_count_with_descendants') @@ -34,7 +34,7 @@ describe API::Namespaces do it "admin: returns an array of all namespaces" do get api("/namespaces", admin) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(Namespace.count) @@ -43,7 +43,7 @@ describe API::Namespaces do it "admin: returns an array of matched namespaces" do get api("/namespaces?search=#{group2.name}", admin) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(1) @@ -77,7 +77,7 @@ describe API::Namespaces do it "user: returns an array of namespaces" do get api("/namespaces", user) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(1) @@ -86,7 +86,7 @@ describe API::Namespaces do it "admin: returns an array of matched namespaces" do get api("/namespaces?search=#{user.username}", user) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(1) @@ -102,7 +102,7 @@ describe API::Namespaces do it 'returns namespace details' do get api("/namespaces/#{namespace_id}", request_actor) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['id']).to eq(requested_namespace.id) expect(json_response['path']).to eq(requested_namespace.path) @@ -153,7 +153,7 @@ describe API::Namespaces do it 'returns not-found' do get api('/namespaces/0', request_actor) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end end end @@ -162,7 +162,7 @@ describe API::Namespaces do it 'returns authentication error' do get api("/namespaces/#{group1.id}") - expect(response).to have_gitlab_http_status(401) + expect(response).to have_gitlab_http_status(:unauthorized) end end @@ -174,7 +174,7 @@ describe API::Namespaces do it 'returns not-found' do get api("/namespaces/#{group2.id}", request_actor) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end end @@ -182,7 +182,7 @@ describe API::Namespaces do it 'returns not-found' do get api("/namespaces/#{user2.namespace.id}", request_actor) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end end end diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index b4416344ecf..6cf978e717e 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -72,7 +72,7 @@ describe API::Notes do it "returns an empty array" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.iid}/notes", user) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response).to be_empty @@ -86,7 +86,7 @@ describe API::Notes do it "returns 404" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.iid}/notes", user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end end end @@ -95,7 +95,7 @@ describe API::Notes do it "returns a non-empty array" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.iid}/notes", private_user) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.first['body']).to eq(cross_reference_note.note) @@ -114,7 +114,7 @@ describe API::Notes do shared_examples 'a notes request' do it 'is a note array response' do - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array end @@ -177,7 +177,7 @@ describe API::Notes do it "returns a 404 error" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.iid}/notes/#{cross_reference_note.id}", user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end context "when issue is confidential" do @@ -188,7 +188,7 @@ describe API::Notes do it "returns 404" do get api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{issue_note.id}", private_user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(:not_found) end end end @@ -197,7 +197,7 @@ describe API::Notes do it "returns an issue note by id" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.iid}/notes/#{cross_reference_note.id}", private_user) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['body']).to eq(cross_reference_note.note) end end @@ -237,7 +237,7 @@ describe API::Notes do it 'returns 200 status' do subject - expect(response).to have_gitlab_http_status(201) + expect(response).to have_gitlab_http_status(:created) end it 'creates a new note' do @@ -251,7 +251,7 @@ describe API::Notes do it 'returns 403 status' do subject - expect(response).to have_gitlab_http_status(403) + expect(response).to have_gitlab_http_status(:forbidden) end it 'does not create a new note' do diff --git a/spec/requests/api/notification_settings_spec.rb b/spec/requests/api/notification_settings_spec.rb index 09fc0197c58..cbdab2f53a6 100644 --- a/spec/requests/api/notification_settings_spec.rb +++ b/spec/requests/api/notification_settings_spec.rb @@ -11,7 +11,7 @@ describe API::NotificationSettings do it "returns global notification settings for the current user" do get api("/notification_settings", user) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_a Hash expect(json_response['notification_email']).to eq(user.notification_email) expect(json_response['level']).to eq(user.global_notification_setting.level) @@ -24,7 +24,7 @@ describe API::NotificationSettings do it "updates global notification settings for the current user" do put api("/notification_settings", user), params: { level: 'watch', notification_email: email.email } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['notification_email']).to eq(email.email) expect(user.reload.notification_email).to eq(email.email) expect(json_response['level']).to eq(user.reload.global_notification_setting.level) @@ -35,7 +35,7 @@ describe API::NotificationSettings do it "fails on non-user email address" do put api("/notification_settings", user), params: { notification_email: 'invalid@example.com' } - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(:bad_request) end end @@ -43,7 +43,7 @@ describe API::NotificationSettings do it "returns group level notification settings for the current user" do get api("/groups/#{group.id}/notification_settings", user) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_a Hash expect(json_response['level']).to eq(user.notification_settings_for(group).level) end @@ -53,7 +53,7 @@ describe API::NotificationSettings do it "updates group level notification settings for the current user" do put api("/groups/#{group.id}/notification_settings", user), params: { level: 'watch' } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['level']).to eq(user.reload.notification_settings_for(group).level) end end @@ -62,7 +62,7 @@ describe API::NotificationSettings do it "returns project level notification settings for the current user" do get api("/projects/#{project.id}/notification_settings", user) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_a Hash expect(json_response['level']).to eq(user.notification_settings_for(project).level) end @@ -72,7 +72,7 @@ describe API::NotificationSettings do it "updates project level notification settings for the current user" do put api("/projects/#{project.id}/notification_settings", user), params: { level: 'custom', new_note: true } - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['level']).to eq(user.reload.notification_settings_for(project).level) expect(json_response['events']['new_note']).to be_truthy expect(json_response['events']['new_issue']).to be_falsey @@ -83,7 +83,7 @@ describe API::NotificationSettings do it "fails on invalid level" do put api("/projects/#{project.id}/notification_settings", user), params: { level: 'invalid' } - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(:bad_request) end end end diff --git a/spec/requests/api/oauth_tokens_spec.rb b/spec/requests/api/oauth_tokens_spec.rb index ce03756a19a..80eae97f41a 100644 --- a/spec/requests/api/oauth_tokens_spec.rb +++ b/spec/requests/api/oauth_tokens_spec.rb @@ -14,7 +14,7 @@ describe 'OAuth tokens' do request_oauth_token(user) - expect(response).to have_gitlab_http_status(401) + expect(response).to have_gitlab_http_status(:unauthorized) expect(json_response['error']).to eq('invalid_grant') end end @@ -25,7 +25,7 @@ describe 'OAuth tokens' do request_oauth_token(user) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(:ok) expect(json_response['access_token']).not_to be_nil end end @@ -33,7 +33,7 @@ describe 'OAuth tokens' do shared_examples 'does not create an access token' do let(:user) { create(:user) } - it { expect(response).to have_gitlab_http_status(401) } + it { expect(response).to have_gitlab_http_status(:unauthorized) } end context 'when user is blocked' do diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index 9d01a44916c..e9657c903e9 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -1979,6 +1979,21 @@ describe API::Runner, :clean_gitlab_redis_shared_state do end end + context 'when object storage throws errors' do + let(:params) { { artifact_type: :archive, artifact_format: :zip } } + + it 'does not store artifacts' do + allow_next_instance_of(JobArtifactUploader) do |uploader| + allow(uploader).to receive(:store!).and_raise(Errno::EIO) + end + + upload_artifacts(file_upload, headers_with_token, params) + + expect(response).to have_gitlab_http_status(:service_unavailable) + expect(job.reload.job_artifacts_archive).to be_nil + end + end + context 'when artifacts are being stored outside of tmp path' do let(:new_tmpdir) { Dir.mktmpdir } diff --git a/spec/services/ci/create_job_artifacts_service_spec.rb b/spec/services/ci/create_job_artifacts_service_spec.rb index e1146fc3df6..03106687678 100644 --- a/spec/services/ci/create_job_artifacts_service_spec.rb +++ b/spec/services/ci/create_job_artifacts_service_spec.rb @@ -3,8 +3,9 @@ require 'spec_helper' describe Ci::CreateJobArtifactsService do - let(:service) { described_class.new } - let(:job) { create(:ci_build) } + let_it_be(:project) { create(:project) } + let(:service) { described_class.new(project) } + let(:job) { create(:ci_build, project: project) } let(:artifacts_sha256) { '0' * 64 } let(:metadata_file) { nil } @@ -64,7 +65,7 @@ describe Ci::CreateJobArtifactsService do it 'sets expiration date according to application settings' do expected_expire_at = 1.day.from_now - expect(subject).to be_truthy + expect(subject).to match(a_hash_including(status: :success)) archive_artifact, metadata_artifact = job.job_artifacts.last(2) expect(job.artifacts_expire_at).to be_within(1.minute).of(expected_expire_at) @@ -80,7 +81,7 @@ describe Ci::CreateJobArtifactsService do it 'sets expiration date according to the parameter' do expected_expire_at = 2.hours.from_now - expect(subject).to be_truthy + expect(subject).to match(a_hash_including(status: :success)) archive_artifact, metadata_artifact = job.job_artifacts.last(2) expect(job.artifacts_expire_at).to be_within(1.minute).of(expected_expire_at) @@ -101,21 +102,50 @@ describe Ci::CreateJobArtifactsService do it 'ignores the changes' do expect { subject }.not_to change { Ci::JobArtifact.count } - expect(subject).to be_truthy + expect(subject).to match(a_hash_including(status: :success)) end end context 'when sha256 of uploading artifact is different than the existing one' do let(:existing_sha256) { '1' * 64 } - it 'returns false and logs the error' do + it 'returns error status' do expect(Gitlab::ErrorTracking).to receive(:track_exception).and_call_original expect { subject }.not_to change { Ci::JobArtifact.count } - expect(subject).to be_falsey - expect(job.errors[:base]).to contain_exactly('another artifact of the same type already exists') + expect(subject).to match( + a_hash_including(http_status: :bad_request, + message: 'another artifact of the same type already exists', + status: :error)) end end end + + shared_examples 'rescues object storage error' do |klass, message, expected_message| + it "handles #{klass}" do + allow_next_instance_of(JobArtifactUploader) do |uploader| + allow(uploader).to receive(:store!).and_raise(klass, message) + end + + expect(Gitlab::ErrorTracking) + .to receive(:track_exception) + .and_call_original + + expect(subject).to match( + a_hash_including( + http_status: :service_unavailable, + message: expected_message || message, + status: :error)) + end + end + + it_behaves_like 'rescues object storage error', + Errno::EIO, 'some/path', 'Input/output error - some/path' + + it_behaves_like 'rescues object storage error', + Google::Apis::ServerError, 'Server error' + + it_behaves_like 'rescues object storage error', + Signet::RemoteServerError, 'The service is currently unavailable' end end diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index 8ca9ce86574..188271f4a75 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -36,7 +36,7 @@ describe Ci::RetryBuildService do job_artifacts_performance job_artifacts_lsif job_artifacts_codequality job_artifacts_metrics scheduled_at job_variables waiting_for_resource_at job_artifacts_metrics_referee - job_artifacts_network_referee].freeze + job_artifacts_network_referee needs].freeze IGNORE_ACCESSORS = %i[type lock_version target_url base_tags trace_sections @@ -46,7 +46,8 @@ describe Ci::RetryBuildService do sourced_pipelines artifacts_file_store artifacts_metadata_store metadata runner_session trace_chunks upstream_pipeline_id artifacts_file artifacts_metadata artifacts_size commands - resource resource_group_id processed security_scans].freeze + resource resource_group_id processed security_scans author + pipeline_id].freeze shared_examples 'build duplication' do let(:another_pipeline) { create(:ci_empty_pipeline, project: project) } @@ -79,8 +80,15 @@ describe Ci::RetryBuildService do end describe 'clone accessors' do + let(:forbidden_associations) do + Ci::Build.reflect_on_all_associations.each_with_object(Set.new) do |assoc, memo| + memo << assoc.name unless assoc.macro == :belongs_to + end + end + CLONE_ACCESSORS.each do |attribute| it "clones #{attribute} build attribute" do + expect(attribute).not_to be_in(forbidden_associations), "association #{attribute} must be `belongs_to`" expect(build.send(attribute)).not_to be_nil expect(new_build.send(attribute)).not_to be_nil expect(new_build.send(attribute)).to eq build.send(attribute) @@ -97,9 +105,17 @@ describe Ci::RetryBuildService do expect(new_build.protected).to eq build.protected end end + + it 'clones only the needs attributes' do + expect(new_build.needs.exists?).to be_truthy + expect(build.needs.exists?).to be_truthy + + expect(new_build.needs_attributes).to match(build.needs_attributes) + expect(new_build.needs).not_to match(build.needs) + end end - describe 'reject acessors' do + describe 'reject accessors' do REJECT_ACCESSORS.each do |attribute| it "does not clone #{attribute} build attribute" do expect(new_build.send(attribute)).not_to eq build.send(attribute) @@ -117,8 +133,9 @@ describe Ci::RetryBuildService do # current_accessors = Ci::Build.attribute_names.map(&:to_sym) + + Ci::Build.attribute_aliases.keys.map(&:to_sym) + Ci::Build.reflect_on_all_associations.map(&:name) + - [:tag_list] + [:tag_list, :needs_attributes] current_accessors.uniq! diff --git a/spec/services/error_tracking/issue_update_service_spec.rb b/spec/services/error_tracking/issue_update_service_spec.rb index 78388328a22..ffb15901f80 100644 --- a/spec/services/error_tracking/issue_update_service_spec.rb +++ b/spec/services/error_tracking/issue_update_service_spec.rb @@ -43,6 +43,12 @@ describe ErrorTracking::IssueUpdateService do update_service.execute end + it 'clears the reactive cache' do + expect(error_tracking_setting).to receive(:expire_issues_cache) + + result + end + context 'related issue and resolving' do let(:issue) { create(:issue, project: project) } let(:sentry_issue) { create(:sentry_issue, issue: issue) } diff --git a/spec/services/metrics/dashboard/update_dashboard_service_spec.rb b/spec/services/metrics/dashboard/update_dashboard_service_spec.rb new file mode 100644 index 00000000000..2bb08579fb9 --- /dev/null +++ b/spec/services/metrics/dashboard/update_dashboard_service_spec.rb @@ -0,0 +1,113 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Metrics::Dashboard::UpdateDashboardService, :use_clean_rails_memory_store_caching do + include MetricsDashboardHelpers + + set(:user) { create(:user) } + set(:project) { create(:project, :repository) } + set(:environment) { create(:environment, project: project) } + + describe '#execute' do + subject(:service_call) { described_class.new(project, user, params).execute } + + let(:commit_message) { 'test' } + let(:branch) { 'dashboard_new_branch' } + let(:dashboard) { 'config/prometheus/common_metrics.yml' } + let(:file_name) { 'custom_dashboard.yml' } + let(:file_content_hash) { YAML.safe_load(File.read(dashboard)) } + let(:params) do + { + file_name: file_name, + file_content: file_content_hash, + commit_message: commit_message, + branch: branch + } + end + + context 'user does not have push right to repository' do + it_behaves_like 'misconfigured dashboard service response', :forbidden, "You can't commit to this project" + end + + context 'with rights to push to the repository' do + before do + project.add_maintainer(user) + end + + context 'path traversal attack attempt' do + context 'with a yml extension' do + let(:file_name) { 'config/prometheus/../database.yml' } + + it_behaves_like 'misconfigured dashboard service response', :bad_request, "A file with this name doesn't exist" + end + + context 'without a yml extension' do + let(:file_name) { '../../..../etc/passwd' } + + it_behaves_like 'misconfigured dashboard service response', :bad_request, "The file name should have a .yml extension" + end + end + + context 'valid parameters' do + it_behaves_like 'valid dashboard update process' + end + + context 'selected branch already exists' do + let(:branch) { 'existing_branch' } + + before do + project.repository.add_branch(user, branch, 'master') + end + + it_behaves_like 'misconfigured dashboard service response', :bad_request, "There was an error updating the dashboard, branch named: existing_branch already exists." + end + + context 'Files::UpdateService success' do + before do + allow(::Files::UpdateService).to receive(:new).and_return(double(execute: { status: :success })) + end + + it 'returns success', :aggregate_failures do + dashboard_details = { + path: '.gitlab/dashboards/custom_dashboard.yml', + display_name: 'custom_dashboard.yml', + default: false, + system_dashboard: false + } + + expect(service_call[:status]).to be :success + expect(service_call[:http_status]).to be :created + expect(service_call[:dashboard]).to match dashboard_details + end + + context 'with escaped characters in file name' do + let(:file_name) { "custom_dashboard%26copy.yml" } + + it 'escapes the special characters', :aggregate_failures do + dashboard_details = { + path: '.gitlab/dashboards/custom_dashboard©.yml', + display_name: 'custom_dashboard©.yml', + default: false, + system_dashboard: false + } + + expect(service_call[:status]).to be :success + expect(service_call[:http_status]).to be :created + expect(service_call[:dashboard]).to match dashboard_details + end + end + end + + context 'Files::UpdateService fails' do + before do + allow(::Files::UpdateService).to receive(:new).and_return(double(execute: { status: :error })) + end + + it 'returns error' do + expect(service_call[:status]).to be :error + end + end + end + end +end diff --git a/spec/support/shared_examples/services/metrics/dashboard_shared_examples.rb b/spec/support/shared_examples/services/metrics/dashboard_shared_examples.rb index 1f229d6b783..a4f81097ffa 100644 --- a/spec/support/shared_examples/services/metrics/dashboard_shared_examples.rb +++ b/spec/support/shared_examples/services/metrics/dashboard_shared_examples.rb @@ -85,3 +85,24 @@ RSpec.shared_examples 'valid dashboard cloning process' do |dashboard_template, end end end + +RSpec.shared_examples 'valid dashboard update process' do + let(:dashboard_attrs) do + { + commit_message: commit_message, + branch_name: branch, + start_branch: project.default_branch, + encoding: 'text', + file_path: ".gitlab/dashboards/#{file_name}", + file_content: ::PerformanceMonitoring::PrometheusDashboard.from_json(file_content_hash).to_yaml + } + end + + it 'delegates commit creation to Files::UpdateService', :aggregate_failures do + service_instance = instance_double(::Files::UpdateService) + expect(::Files::UpdateService).to receive(:new).with(project, user, dashboard_attrs).and_return(service_instance) + expect(service_instance).to receive(:execute).and_return(status: :success) + + service_call + end +end |