diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index 9dc2f5eff23..bc8f7596ff0 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -550,8 +550,6 @@ - <<: *if-merge-request-labels-run-all-rspec - <<: *if-merge-request changes: *core-backend-patterns - - <<: *if-merge-request - changes: *ci-patterns - <<: *if-automated-merge-request changes: *backend-patterns - <<: *if-security-merge-request @@ -579,8 +577,6 @@ changes: *core-backend-patterns - <<: *if-merge-request changes: *workhorse-patterns - - <<: *if-merge-request - changes: *ci-patterns - <<: *if-automated-merge-request changes: *code-backstage-patterns - <<: *if-security-merge-request @@ -629,8 +625,6 @@ ###################### .build-images:rules:build-qa-image: rules: - - <<: *if-not-ee - when: never - <<: *if-not-canonical-namespace when: never - <<: *if-merge-request-targeting-stable-branch @@ -768,8 +762,6 @@ - <<: *if-merge-request-labels-run-all-rspec - <<: *if-merge-request changes: *startup-css-patterns - - <<: *if-merge-request - changes: *ci-patterns .frontend:rules:frontend_fixture-as-if-foss: rules: @@ -906,8 +898,6 @@ changes: *code-qa-patterns - <<: *if-merge-request-labels-as-if-foss - <<: *if-merge-request-labels-run-all-rspec - - <<: *if-merge-request - changes: *ci-patterns .qa:rules:internal-as-if-foss: rules: @@ -1009,8 +999,6 @@ - <<: *if-merge-request-labels-run-all-rspec - <<: *if-merge-request changes: *core-backend-patterns - - <<: *if-merge-request - changes: *ci-patterns # When DB schema changes, many migrations spec may be affected. However, the test mapping from Crystalball does not map db change to a specific migration spec well. # See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68840. - <<: *if-merge-request @@ -1112,8 +1100,6 @@ - <<: *if-merge-request-labels-run-all-rspec - <<: *if-merge-request changes: *core-backend-patterns - - <<: *if-merge-request - changes: *ci-patterns # When DB schema changes, many migrations spec may be affected. However, the test mapping from Crystalball does not map db change to a specific migration spec well. # See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68840. - <<: *if-merge-request @@ -1201,8 +1187,6 @@ - <<: *if-merge-request-labels-run-all-rspec - <<: *if-merge-request changes: *core-backend-patterns - - <<: *if-merge-request - changes: *ci-patterns # When DB schema changes, many migrations spec may be affected. However, the test mapping from Crystalball does not map db change to a specific migration spec well. # See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68840. - <<: *if-merge-request-labels-as-if-foss @@ -1416,9 +1400,6 @@ - if: '$SKIP_FLAKY_TESTS_AUTOMATICALLY == "true" || $RETRY_FAILED_TESTS_IN_NEW_PROCESS == "true"' changes: *code-backstage-patterns when: always - - if: '$SKIP_FLAKY_TESTS_AUTOMATICALLY == "true" || $RETRY_FAILED_TESTS_IN_NEW_PROCESS == "true"' - changes: *ci-patterns - when: always ######################### # Static analysis rules # diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js index 7925a10344a..c39e4562c14 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js +++ b/app/assets/javascripts/lib/utils/common_utils.js @@ -60,6 +60,15 @@ export const disableButtonIfEmptyField = (fieldSelector, buttonSelector, eventNa }); }; +/** + * Return the given element's offset height, or 0 if the element doesn't exist. + * Probably not useful outside of handleLocationHash. + * + * @param {HTMLElement} element The element to measure. + * @returns {number} The element's offset height. + */ +const getElementOffsetHeight = (element) => element?.offsetHeight ?? 0; + // automatically adjust scroll position for hash urls taking the height of the navbar into account // https://github.com/twitter/bootstrap/issues/1768 export const handleLocationHash = () => { @@ -84,40 +93,26 @@ export const handleLocationHash = () => { const fixedIssuableTitle = document.querySelector('.issue-sticky-header'); let adjustment = 0; - if (fixedNav) adjustment -= fixedNav.offsetHeight; - if (target && target.scrollIntoView) { - target.scrollIntoView(true); - } - - if (fixedTabs) { - adjustment -= fixedTabs.offsetHeight; - } - - if (fixedDiffStats) { - adjustment -= fixedDiffStats.offsetHeight; - } - - if (performanceBar) { - adjustment -= performanceBar.offsetHeight; - } - - if (diffFileHeader) { - adjustment -= diffFileHeader.offsetHeight; - } - - if (versionMenusContainer) { - adjustment -= versionMenusContainer.offsetHeight; - } + adjustment -= getElementOffsetHeight(fixedNav); + adjustment -= getElementOffsetHeight(fixedTabs); + adjustment -= getElementOffsetHeight(fixedDiffStats); + adjustment -= getElementOffsetHeight(performanceBar); + adjustment -= getElementOffsetHeight(diffFileHeader); + adjustment -= getElementOffsetHeight(versionMenusContainer); if (isInIssuePage()) { - adjustment -= fixedIssuableTitle.offsetHeight; + adjustment -= getElementOffsetHeight(fixedIssuableTitle); } if (isInMRPage()) { adjustment -= topPadding; } + if (target?.scrollIntoView) { + target.scrollIntoView(true); + } + setTimeout(() => { window.scrollBy(0, adjustment); }); diff --git a/app/assets/javascripts/pipelines/components/pipelines_list/pipelines.vue b/app/assets/javascripts/pipelines/components/pipelines_list/pipelines.vue index f9022be888a..8c54cdf8686 100644 --- a/app/assets/javascripts/pipelines/components/pipelines_list/pipelines.vue +++ b/app/assets/javascripts/pipelines/components/pipelines_list/pipelines.vue @@ -249,7 +249,7 @@ export default { this.updateContent(params); - this.track('click_filter_tabs', { label: TRACKING_CATEGORIES.tabs }); + this.track('click_filter_tabs', { label: TRACKING_CATEGORIES.tabs, property: scope }); }, successCallback(resp) { // Because we are polling & the user is interacting verify if the response received diff --git a/app/controllers/boards/issues_controller.rb b/app/controllers/boards/issues_controller.rb index 5028544795c..8bd7cd496e5 100644 --- a/app/controllers/boards/issues_controller.rb +++ b/app/controllers/boards/issues_controller.rb @@ -34,13 +34,14 @@ module Boards end def create - service = Boards::Issues::CreateService.new(board_parent, project, current_user, issue_params) - issue = service.execute + result = Boards::Issues::CreateService.new(board_parent, project, current_user, issue_params).execute - if issue.valid? - render json: serialize_as_json(issue) + if result.success? + render json: serialize_as_json(result[:issue]) + elsif result[:issue] + render json: result[:issue].errors, status: :unprocessable_entity else - render json: issue.errors, status: :unprocessable_entity + render json: result.errors, status: result.http_status || 422 end end diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 800a7df2566..5168c9adb79 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -147,19 +147,26 @@ class Projects::IssuesController < Projects::ApplicationController spam_params = ::Spam::SpamParams.new_from_request(request: request) service = ::Issues::CreateService.new(project: project, current_user: current_user, params: create_params, spam_params: spam_params) - @issue = service.execute + result = service.execute - create_vulnerability_issue_feedback(issue) - - if service.discussions_to_resolve.count(&:resolved?) > 0 - flash[:notice] = if service.discussion_to_resolve_id - _("Resolved 1 discussion.") - else - _("Resolved all discussions.") - end + # Only irrecoverable errors such as unauthorized user won't contain an issue in the response + if result.error? && result[:issue].blank? + render_by_create_result_error(result) && return end - if @issue.valid? + @issue = result[:issue] + + if result.success? + create_vulnerability_issue_feedback(@issue) + + if service.discussions_to_resolve.count(&:resolved?) > 0 + flash[:notice] = if service.discussion_to_resolve_id + _("Resolved 1 discussion.") + else + _("Resolved all discussions.") + end + end + redirect_to project_issue_path(@project, @issue) else # NOTE: this CAPTCHA support method is indirectly included via IssuableActions @@ -372,6 +379,21 @@ class Projects::IssuesController < Projects::ApplicationController private + def render_by_create_result_error(result) + Gitlab::AppLogger.warn( + message: 'Cannot create issue', + errors: result.errors, + http_status: result.http_status + ) + error_method_name = "render_#{result.http_status}".to_sym + + if respond_to?(error_method_name, true) + send(error_method_name) # rubocop:disable GitlabSecurity/PublicSend + else + render_404 + end + end + def clean_params(all_params) issue_type = all_params[:issue_type].to_s all_params.delete(:issue_type) unless WorkItems::Type.allowed_types_for_issues.include?(issue_type) diff --git a/app/graphql/mutations/alert_management/create_alert_issue.rb b/app/graphql/mutations/alert_management/create_alert_issue.rb index 2c128e1b339..77a7d7a4147 100644 --- a/app/graphql/mutations/alert_management/create_alert_issue.rb +++ b/app/graphql/mutations/alert_management/create_alert_issue.rb @@ -24,8 +24,8 @@ module Mutations def prepare_response(alert, result) { alert: alert, - issue: result.payload[:issue], - errors: Array(result.message) + issue: result[:issue], + errors: result.errors } end end diff --git a/app/graphql/mutations/issues/create.rb b/app/graphql/mutations/issues/create.rb index 6bf8caf82d7..0389a482822 100644 --- a/app/graphql/mutations/issues/create.rb +++ b/app/graphql/mutations/issues/create.rb @@ -83,13 +83,13 @@ module Mutations params = build_create_issue_params(attributes.merge(author_id: current_user.id), project) spam_params = ::Spam::SpamParams.new_from_request(request: context[:request]) - issue = ::Issues::CreateService.new(project: project, current_user: current_user, params: params, spam_params: spam_params).execute + result = ::Issues::CreateService.new(project: project, current_user: current_user, params: params, spam_params: spam_params).execute - check_spam_action_response!(issue) + check_spam_action_response!(result[:issue]) if result[:issue] { - issue: issue.valid? ? issue : nil, - errors: errors_on_object(issue) + issue: result.success? ? result[:issue] : nil, + errors: result.errors } end diff --git a/app/helpers/sessions_helper.rb b/app/helpers/sessions_helper.rb index 129180d1ccf..56138ba95c2 100644 --- a/app/helpers/sessions_helper.rb +++ b/app/helpers/sessions_helper.rb @@ -49,6 +49,6 @@ module SessionsHelper match = regex.match(email) return email unless match - match[1] + '*' * match[2].length + match[3] + '*' * match[4].length + match[5] + match[1] + '*' * (match[2] || '').length + match[3] + '*' * (match[4] || '').length + match[5] end end diff --git a/app/models/repository.rb b/app/models/repository.rb index ee1bea0e8d2..81f1c385bf3 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -48,22 +48,19 @@ class Repository # For example, for entry `:commit_count` there's a method called `commit_count` which # stores its data in the `commit_count` cache key. CACHED_METHODS = %i(size commit_count readme_path contribution_guide - changelog license_blob license_key gitignore + changelog license_blob license_licensee license_gitaly gitignore gitlab_ci_yml branch_names tag_names branch_count tag_count avatar exists? root_ref merged_branch_names has_visible_content? issue_template_names_hash merge_request_template_names_hash user_defined_metrics_dashboard_paths xcode_project? has_ambiguous_refs?).freeze - # Methods that use cache_method but only memoize the value - MEMOIZED_CACHED_METHODS = %i(license).freeze - # Certain method caches should be refreshed when certain types of files are # changed. This Hash maps file types (as returned by Gitlab::FileDetector) to # the corresponding methods to call for refreshing caches. METHOD_CACHES_FOR_FILE_TYPES = { readme: %i(readme_path), changelog: :changelog, - license: %i(license_blob license_key license), + license: %i(license_blob license_licensee license_gitaly), contributing: :contribution_guide, gitignore: :gitignore, gitlab_ci: :gitlab_ci_yml, @@ -650,25 +647,30 @@ class Repository cache_method :license_blob def license_key - return unless exists? - - raw_repository.license_short_name + license&.key end - cache_method :license_key def license - return unless license_key - - licensee_object = Licensee::License.new(license_key) - - return if licensee_object.name.blank? - - licensee_object - rescue Licensee::InvalidLicense => e - Gitlab::ErrorTracking.track_exception(e) - nil + if Feature.enabled?(:license_from_gitaly) + license_gitaly + else + license_licensee + end end - memoize_method :license + + def license_licensee + return unless exists? + + raw_repository.license(false) + end + cache_method :license_licensee + + def license_gitaly + return unless exists? + + raw_repository.license(true) + end + cache_method :license_gitaly def gitignore file_on_head(:gitignore) diff --git a/app/services/alert_management/create_alert_issue_service.rb b/app/services/alert_management/create_alert_issue_service.rb index 34c2003bd01..28e312a6fa3 100644 --- a/app/services/alert_management/create_alert_issue_service.rb +++ b/app/services/alert_management/create_alert_issue_service.rb @@ -21,7 +21,7 @@ module AlertManagement result = create_incident return result unless result.success? - issue = result.payload[:issue] + issue = result[:issue] perform_after_create_tasks(issue) result diff --git a/app/services/incident_management/incidents/create_service.rb b/app/services/incident_management/incidents/create_service.rb index ef66325fdcc..f44842650b7 100644 --- a/app/services/incident_management/incidents/create_service.rb +++ b/app/services/incident_management/incidents/create_service.rb @@ -15,7 +15,7 @@ module IncidentManagement end def execute - issue = Issues::CreateService.new( + create_result = Issues::CreateService.new( project: project, current_user: current_user, params: { @@ -29,22 +29,16 @@ module IncidentManagement ).execute if alert - return error(alert.errors.full_messages.to_sentence, issue) unless alert.valid? + return error(alert.errors.full_messages, create_result[:issue]) unless alert.valid? end - return error(issue.errors.full_messages.to_sentence, issue) unless issue.valid? - - success(issue) + create_result end private attr_reader :title, :description, :severity, :alert - def success(issue) - ServiceResponse.success(payload: { issue: issue }) - end - def error(message, issue = nil) ServiceResponse.error(payload: { issue: issue }, message: message) end diff --git a/app/services/issuable/import_csv/base_service.rb b/app/services/issuable/import_csv/base_service.rb index 822e3cd787c..e84d1032e41 100644 --- a/app/services/issuable/import_csv/base_service.rb +++ b/app/services/issuable/import_csv/base_service.rb @@ -23,7 +23,7 @@ module Issuable with_csv_lines.each do |row, line_no| attributes = issuable_attributes_for(row) - if create_issuable(attributes).persisted? + if create_issuable(attributes)&.persisted? @results[:success] += 1 else @results[:error_lines].push(line_no) diff --git a/app/services/issues/clone_service.rb b/app/services/issues/clone_service.rb index 07dd9a98f89..8b05a1c2acd 100644 --- a/app/services/issues/clone_service.rb +++ b/app/services/issues/clone_service.rb @@ -75,7 +75,16 @@ module Issues # Skip creation of system notes for existing attributes of the issue when cloning with notes. # The system notes of the old issue are copied over so we don't want to end up with duplicate notes. # When cloning without notes, we want to generate system notes for the attributes that were copied. - CreateService.new(project: target_project, current_user: current_user, params: new_params, spam_params: spam_params).execute(skip_system_notes: with_notes) + create_result = CreateService.new( + project: target_project, + current_user: current_user, + params: new_params, + spam_params: spam_params + ).execute(skip_system_notes: with_notes) + + raise CloneError, create_result.errors.join(', ') if create_result.error? && create_result[:issue].blank? + + create_result[:issue] end def queue_copy_designs diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb index 92cf4811439..9cf54735416 100644 --- a/app/services/issues/create_service.rb +++ b/app/services/issues/create_service.rb @@ -4,6 +4,7 @@ module Issues class CreateService < Issues::BaseService include ResolveDiscussions prepend RateLimitedService + include ::Services::ReturnServiceResponses rate_limit key: :issues_create, opts: { scope: [:project, :current_user, :external_author] } @@ -27,7 +28,13 @@ module Issues @add_related_issue ||= params.delete(:add_related_issue) filter_resolve_discussion_params - create(@issue, skip_system_notes: skip_system_notes) + issue = create(@issue, skip_system_notes: skip_system_notes) + + if issue.persisted? + success(issue: issue) + else + error(issue.errors.full_messages, 422, pass_back: { issue: issue }) + end end def external_author diff --git a/app/services/issues/import_csv_service.rb b/app/services/issues/import_csv_service.rb index bce3ecc8bef..83e550583f6 100644 --- a/app/services/issues/import_csv_service.rb +++ b/app/services/issues/import_csv_service.rb @@ -14,6 +14,10 @@ module Issues private + def create_issuable(attributes) + super[:issue] + end + def create_issuable_class Issues::CreateService end diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb index edab62b1fdf..6366ff4076b 100644 --- a/app/services/issues/move_service.rb +++ b/app/services/issues/move_service.rb @@ -83,7 +83,16 @@ module Issues # Skip creation of system notes for existing attributes of the issue. The system notes of the old # issue are copied over so we don't want to end up with duplicate notes. - CreateService.new(project: @target_project, current_user: @current_user, params: new_params, spam_params: spam_params).execute(skip_system_notes: true) + create_result = CreateService.new( + project: @target_project, + current_user: @current_user, + params: new_params, + spam_params: spam_params + ).execute(skip_system_notes: true) + + raise MoveError, create_result.errors.join(', ') if create_result.error? && create_result[:issue].blank? + + create_result[:issue] end def queue_copy_designs diff --git a/app/services/work_items/create_service.rb b/app/services/work_items/create_service.rb index c2ceb701a2f..d5a52eb663f 100644 --- a/app/services/work_items/create_service.rb +++ b/app/services/work_items/create_service.rb @@ -2,7 +2,6 @@ module WorkItems class CreateService < Issues::CreateService - include ::Services::ReturnServiceResponses include WidgetableService def initialize(project:, current_user: nil, params: {}, spam_params:, widget_params: {}) @@ -21,7 +20,10 @@ module WorkItems return error(_('Operation not allowed'), :forbidden) end - work_item = super + result = super + return result if result.error? + + work_item = result[:issue] if work_item.valid? success(payload(work_item)) diff --git a/app/workers/incident_management/pager_duty/process_incident_worker.rb b/app/workers/incident_management/pager_duty/process_incident_worker.rb index 933d8e12d25..181e336e6b0 100644 --- a/app/workers/incident_management/pager_duty/process_incident_worker.rb +++ b/app/workers/incident_management/pager_duty/process_incident_worker.rb @@ -38,7 +38,7 @@ module IncidentManagement def log_error(result) Gitlab::AppLogger.warn( message: 'Cannot create issue for PagerDuty incident', - issue_errors: result.message + issue_errors: result.errors.join(', ') ) end end diff --git a/app/workers/incident_management/process_alert_worker_v2.rb b/app/workers/incident_management/process_alert_worker_v2.rb index f3049560bcd..04c02d17704 100644 --- a/app/workers/incident_management/process_alert_worker_v2.rb +++ b/app/workers/incident_management/process_alert_worker_v2.rb @@ -37,13 +37,13 @@ module IncidentManagement end def log_warning(alert, result) - issue_id = result.payload[:issue]&.id + issue_id = result[:issue]&.id Gitlab::AppLogger.warn( message: 'Cannot process an Incident', issue_id: issue_id, alert_id: alert.id, - errors: result.message + errors: result.errors.join(', ') ) end end diff --git a/config/feature_flags/development/license_from_gitaly.yml b/config/feature_flags/development/license_from_gitaly.yml new file mode 100644 index 00000000000..ad79d56a8ab --- /dev/null +++ b/config/feature_flags/development/license_from_gitaly.yml @@ -0,0 +1,8 @@ +--- +name: license_from_gitaly +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/77041 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/374300 +milestone: '15.5' +type: development +group: group::gitaly +default_enabled: false diff --git a/config/initializers/licensee_license_patch.rb b/config/initializers/licensee_license_patch.rb new file mode 100644 index 00000000000..d4680db5071 --- /dev/null +++ b/config/initializers/licensee_license_patch.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +require 'licensee/license' + +module Licensee + module LicensePatch + # Patch from https://github.com/licensee/licensee/pull/589 + def ==(other) + other.is_a?(self.class) && key == other.key + end + end + + License.prepend LicensePatch +end diff --git a/data/whats_new/202209220001_15_04.yml b/data/whats_new/202209220001_15_04.yml new file mode 100644 index 00000000000..056826c6457 --- /dev/null +++ b/data/whats_new/202209220001_15_04.yml @@ -0,0 +1,103 @@ +- name: "Suggested Reviewers open beta" # Match the release post entry + description: | # Do not modify this line, instead modify the lines below. + Deciding the right person to [review your merge request](https://docs.gitlab.com/ee/user/project/merge_requests/reviews/) isn't always straightforward or obvious. Choosing the wrong reviewer can cause delays, low quality reviews, back and forth reassigning reviewers, or even no review at all. + + Now, GitLab can recommend a reviewer with [Suggested Reviewers](https://docs.gitlab.com/ee/user/project/merge_requests/reviews/#suggested-reviewers). Using the changes in a merge request and a project's contribution graph, machine learning powered suggestions appear in the [reviewer dropdown](https://docs.gitlab.com/ee/user/project/merge_requests/getting_started.html#reviewer) in the merge request sidebar. + + This feature is currently in [beta](https://about.gitlab.com/handbook/product/gitlab-the-product/#open-beta) behind a [feature flag](https://gitlab.com/gitlab-org/gitlab/-/issues/368356). It will be rolling out to all Ultimate GitLab.com customers over the next week. + stage: create # String value of the stage that the feature was created in. e.g., Growth + self-managed: false # Boolean value (true or false) + gitlab-com: true # Boolean value (true or false) + available_in: [Ultimate] # Array of strings. The Array brackets are required here. e.g., [Free, Premium, Ultimate] + documentation_link: 'https://docs.gitlab.com/ee/user/project/merge_requests/reviews/#suggested-reviewers' # This is the documentation URL, but can be a URL to a video if there is one + image_url: https://about.gitlab.com/images/15_4/create-code-review-suggested-reviewers.png # This should be a full URL, generally taken from the release post content. If a video, use the youtube thumbnail URL with the structure of https://img.youtube.com/vi/UNIQUEID/hqdefault.jpg + published_at: 2022-09-22 # YYYY-MM-DD + release: 15.4 # XX.Y +- name: "Improved CI/CD integration in VS Code" # Match the release post entry + description: | # Do not modify this line, instead modify the lines below. + When you're constructing complicated GitLab CI configurations that may contain `include:` or `extends:` keywords, it's challenging to ensure the configuration is valid and the resulting file has your expected configuration. Use [GitLab Workflow](https://marketplace.visualstudio.com/items?itemName=GitLab.gitlab-workflow) for Visual Studio Code to preview your merged GitLab CI/CD configuration file directly in VS Code. You can view your changes locally, and ensure your configuration is as you expect, before you commit and push. + + GitLab Workflow [v3.50.0](https://gitlab.com/gitlab-org/gitlab-vscode-extension/-/blob/main/CHANGELOG.md#3500-2022-09-09) also provides more CI/CD pipeline interactions to help you avoid context-switching: + + * Download artifacts: [commit `f4d027c`](https://gitlab.com/gitlab-org/gitlab-vscode-extension/commit/f4d027c616c884bef9fc42e5f20dfac43b811134), [merge request `!635`](https://gitlab.com/gitlab-org/gitlab-vscode-extension/-/merge_requests/635) + * Retry or cancel an existing pipeline: [commit `c2caee4`](https://gitlab.com/gitlab-org/gitlab-vscode-extension/commit/c2caee40cfcbfb5d13cc790f9a2d1cfcf6c6a7ab), [merge request `!637`](https://gitlab.com/gitlab-org/gitlab-vscode-extension/-/merge_requests/637) + stage: create # String value of the stage that the feature was created in. e.g., Growth + self-managed: true # Boolean value (true or false) + gitlab-com: false # Boolean value (true or false) + available_in: [Free, Premium, Ultimate] # Array of strings. The Array brackets are required here. e.g., [Free, Premium, Ultimate] + documentation_link: 'https://gitlab.com/gitlab-org/gitlab-vscode-extension#show-merged-gitlab-cicd-configuration' # This is the documentation URL, but can be a URL to a video if there is one + image_url: https://about.gitlab.com/images/15_4/create-vs-code-cicd-improvements.png # This should be a full URL, generally taken from the release post content. If a video, use the youtube thumbnail URL with the structure of https://img.youtube.com/vi/UNIQUEID/hqdefault.jpg + published_at: 2022-09-22 # YYYY-MM-DD + release: 15.4 # XX.Y +- name: "Users on verified domains can bypass email validation" # Match the release post entry + description: | # Do not modify this line, instead modify the lines below. + New GitLab users created using SAML or SCIM that belong to a [verified domain](https://docs.gitlab.com/ee/user/project/pages/custom_domains_ssl_tls_certification/#1-add-a-custom-domain) no longer receive the GitLab account verification e-mail. + + This reduces account activation friction. Accounts generated through a provisioning process are already verified, so users should not have to individually verify them manually. + stage: manage # String value of the stage that the feature was created in. e.g., Growth + self-managed: false # Boolean value (true or false) + gitlab-com: true # Boolean value (true or false) + available_in: [Premium, Ultimate] # Array of strings. The Array brackets are required here. e.g., [Free, Premium, Ultimate] + documentation_link: 'https://docs.gitlab.com/ee/user/group/saml_sso/index.html#bypass-user-verification-with-verified-domains' # This is the documentation URL, but can be a URL to a video if there is one + image_url: https://about.gitlab.com/images/15_4/domain-verification.png # This should be a full URL, generally taken from the release post content. If a video, use the youtube thumbnail URL with the structure of https://img.youtube.com/vi/UNIQUEID/hqdefault.jpg + published_at: 2022-09-22 # YYYY-MM-DD + release: 15.4 # XX.Y +- name: "Sortable, filterable data-driven tables in Markdown" # Match the release post entry + description: | # Do not modify this line, instead modify the lines below. + Working with tables in Markdown can be a bit cumbersome. Not only is it difficult to figure out the correct number of pipes and empty cells, but the table output is static when you save your document. If you have to sort the table by the third column in an ascending order, you end up rewriting the whole thing. + + Now you can insert data-driven tables using JSON syntax as follows: + + 1. Write or export a table in JSON. + 1. Wrap JSON in a code block that starts with triple backticks `\`` followed by `json:table`. + 1. Save your issue, submit your comment, or publish your page. + + In the rendered table, you can also enable: + + - Sorting for specific fields using `"sortable": true` + - Dynamic filtering of data using `"filter" : true` + + Now it's as simple as a click when you have to re-sort that 100-row table and as easy as a web search when you have to find that one issue reference lost in a sea of nearly identical URLs. + stage: create # String value of the stage that the feature was created in. e.g., Growth + self-managed: true # Boolean value (true or false) + gitlab-com: true # Boolean value (true or false) + available_in: [Free, Premium, Ultimate] # Array of strings. The Array brackets are required here. e.g., [Free, Premium, Ultimate] + documentation_link: 'https://docs.gitlab.com/ee/user/markdown.html#json' # This is the documentation URL, but can be a URL to a video if there is one + image_url: https://img.youtube.com/vi/12yWKw1AdKY/hqdefault.jpg # This should be a full URL, generally taken from the release post content. If a video, use the youtube thumbnail URL with the structure of https://img.youtube.com/vi/UNIQUEID/hqdefault.jpg + published_at: 2022-09-22 # YYYY-MM-DD + release: 15.4 # XX.Y +- name: "Getting started with GitLab Pages just got easier" # Match the release post entry + description: | # Do not modify this line, instead modify the lines below. + We've made it much easier to get started with GitLab Pages. Instead of creating configuration files by hand, build them interactively using the GitLab UI. Just answer a few basic questions on how your app is built, and we'll build the `.gitlab-ci.yml` file to get you started. + + This is the first time we're using our new [Pipeline Wizard](https://docs.gitlab.com/ee/development/cicd/pipeline_wizard.html), a tool that makes it easy to create `.gitlab-ci.yml` files by building them in the GitLab UI. You can look forward to more simplified onboarding helpers like this one. + stage: create # String value of the stage that the feature was created in. e.g., Growth + self-managed: true # Boolean value (true or false) + gitlab-com: true # Boolean value (true or false) + available_in: [Free, Premium, Ultimate] # Array of strings. The Array brackets are required here. e.g., [Free, Premium, Ultimate] + documentation_link: 'https://docs.gitlab.com/ee/user/project/pages/getting_started/pages_ui.html' # This is the documentation URL, but can be a URL to a video if there is one + image_url: https://about.gitlab.com/images/15_4/create-pages-onboarding.png # This should be a full URL, generally taken from the release post content. If a video, use the youtube thumbnail URL with the structure of https://img.youtube.com/vi/UNIQUEID/hqdefault.jpg + published_at: 2022-09-22 # YYYY-MM-DD + release: 15.4 # XX.Y +- name: "More powerful Linux machine types for GitLab SaaS runners" # Match the release post entry + description: | # Do not modify this line, instead modify the lines below. + When you run jobs on GitLab SaaS Linux runners, you now have access to more powerful machine types: medium and large. With these two machine types, you have more choices for your GitLab SaaS CI/CD jobs. And with 100% job isolation on an ephemeral virtual machine, and security and autoscaling fully managed by GitLab, you can confidently run your critical CI/CD jobs on GitLab SaaS. + stage: create # String value of the stage that the feature was created in. e.g., Growth + self-managed: false # Boolean value (true or false) + gitlab-com: true # Boolean value (true or false) + available_in: [Premium, Ultimate] # Array of strings. The Array brackets are required here. e.g., [Free, Premium, Ultimate] + documentation_link: 'https://docs.gitlab.com/ee/ci/runners/saas/linux_saas_runner.html' # This is the documentation URL, but can be a URL to a video if there is one + image_url: https://about.gitlab.com/images/15_4/select-multiple-gitlab-saas-linux-runners.png # This should be a full URL, generally taken from the release post content. If a video, use the youtube thumbnail URL with the structure of https://img.youtube.com/vi/UNIQUEID/hqdefault.jpg + published_at: 2022-09-22 # YYYY-MM-DD + release: 15.4 # XX.Y +- name: "Limit the maximum number of custom domains per project" # Match the release post entry + description: | # Do not modify this line, instead modify the lines below. + You can use GitLab Pages to define custom domains for your website. Too many custom domains, however, can result in slow response times from the Pages API and impact the overall reliability of the service. Now you can limit the maximum number of custom domains per project at the instance level and strike the right balance for your needs. The default value is `0` (unlimited). + stage: create # String value of the stage that the feature was created in. e.g., Growth + self-managed: true # Boolean value (true or false) + gitlab-com: false # Boolean value (true or false) + available_in: [Free, Premium, Ultimate] # Array of strings. The Array brackets are required here. e.g., [Free, Premium, Ultimate] + documentation_link: 'https://docs.gitlab.com/ee/administration/pages/#set-maximum-number-of-gitlab-pages-custom-domains-for-a-project' # This is the documentation URL, but can be a URL to a video if there is one + image_url: https://about.gitlab.com/images/15_4/create-pages-domain-limits.png # This should be a full URL, generally taken from the release post content. If a video, use the youtube thumbnail URL with the structure of https://img.youtube.com/vi/UNIQUEID/hqdefault.jpg + published_at: 2022-09-22 # YYYY-MM-DD + release: 15.4 # XX.Y diff --git a/db/migrate/20220805180311_add_unique_index_on_sbom_component_type_and_name.rb b/db/migrate/20220805180311_add_unique_index_on_sbom_component_type_and_name.rb new file mode 100644 index 00000000000..852b1283e92 --- /dev/null +++ b/db/migrate/20220805180311_add_unique_index_on_sbom_component_type_and_name.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddUniqueIndexOnSbomComponentTypeAndName < Gitlab::Database::Migration[2.0] + INDEX_NAME = 'index_sbom_components_on_component_type_and_name' + + disable_ddl_transaction! + + def up + add_concurrent_index :sbom_components, [:component_type, :name], unique: true, name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :sbom_components, name: INDEX_NAME + end +end diff --git a/db/migrate/20220805183952_add_unique_index_to_sbom_component_versions_on_component_id_and_version.rb b/db/migrate/20220805183952_add_unique_index_to_sbom_component_versions_on_component_id_and_version.rb new file mode 100644 index 00000000000..03023bc6f2c --- /dev/null +++ b/db/migrate/20220805183952_add_unique_index_to_sbom_component_versions_on_component_id_and_version.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddUniqueIndexToSbomComponentVersionsOnComponentIdAndVersion < Gitlab::Database::Migration[2.0] + INDEX_NAME = 'index_sbom_component_versions_on_component_id_and_version' + + disable_ddl_transaction! + + def up + add_concurrent_index :sbom_component_versions, [:component_id, :version], unique: true, name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :sbom_component_versions, name: INDEX_NAME + end +end diff --git a/db/migrate/20220805193117_add_unique_index_to_sbom_sources_on_source_type_and_source.rb b/db/migrate/20220805193117_add_unique_index_to_sbom_sources_on_source_type_and_source.rb new file mode 100644 index 00000000000..973cacaec5b --- /dev/null +++ b/db/migrate/20220805193117_add_unique_index_to_sbom_sources_on_source_type_and_source.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddUniqueIndexToSbomSourcesOnSourceTypeAndSource < Gitlab::Database::Migration[2.0] + INDEX_NAME = 'index_sbom_sources_on_source_type_and_source' + + disable_ddl_transaction! + + def up + add_concurrent_index :sbom_sources, [:source_type, :source], unique: true, name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :sbom_sources, name: INDEX_NAME + end +end diff --git a/db/migrate/20220818140553_add_unique_index_to_sbom_occurrences_on_ingestion_attributes.rb b/db/migrate/20220818140553_add_unique_index_to_sbom_occurrences_on_ingestion_attributes.rb new file mode 100644 index 00000000000..2538017e287 --- /dev/null +++ b/db/migrate/20220818140553_add_unique_index_to_sbom_occurrences_on_ingestion_attributes.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class AddUniqueIndexToSbomOccurrencesOnIngestionAttributes < Gitlab::Database::Migration[2.0] + INDEX_NAME = 'index_sbom_occurrences_on_ingestion_attributes' + ATTRIBUTES = %i[ + project_id + component_id + component_version_id + source_id + commit_sha + ].freeze + + disable_ddl_transaction! + + def up + add_concurrent_index :sbom_occurrences, ATTRIBUTES, unique: true, name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :sbom_occurrences, name: INDEX_NAME + end +end diff --git a/db/schema_migrations/20220805180311 b/db/schema_migrations/20220805180311 new file mode 100644 index 00000000000..dff18ebd3fc --- /dev/null +++ b/db/schema_migrations/20220805180311 @@ -0,0 +1 @@ +35335f40a192889c45f71a8a3b25dd0e7024919ff823b01c2086c0e6959869e0 \ No newline at end of file diff --git a/db/schema_migrations/20220805183952 b/db/schema_migrations/20220805183952 new file mode 100644 index 00000000000..c1a1c03dcef --- /dev/null +++ b/db/schema_migrations/20220805183952 @@ -0,0 +1 @@ +a90c4b56f6cf16ec62d4a37e03add702ce8d64640b1c61f6f0b18b2d9720f24e \ No newline at end of file diff --git a/db/schema_migrations/20220805193117 b/db/schema_migrations/20220805193117 new file mode 100644 index 00000000000..36523ba0866 --- /dev/null +++ b/db/schema_migrations/20220805193117 @@ -0,0 +1 @@ +f64b85003dde31c4f0ba37cb0b550fb50b8d7753bbae3043f28ed51858349572 \ No newline at end of file diff --git a/db/schema_migrations/20220818140553 b/db/schema_migrations/20220818140553 new file mode 100644 index 00000000000..deedcc9d4b8 --- /dev/null +++ b/db/schema_migrations/20220818140553 @@ -0,0 +1 @@ +f46a411a6519723cd2ee0a5b287f26f987195ba76e5753febe47502b1152a543 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index c218ea2b07d..2756c8fa8ff 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -30187,16 +30187,24 @@ CREATE UNIQUE INDEX index_saved_replies_on_name_text_pattern_ops ON saved_replie CREATE INDEX index_sbom_component_versions_on_component_id ON sbom_component_versions USING btree (component_id); +CREATE UNIQUE INDEX index_sbom_component_versions_on_component_id_and_version ON sbom_component_versions USING btree (component_id, version); + +CREATE UNIQUE INDEX index_sbom_components_on_component_type_and_name ON sbom_components USING btree (component_type, name); + CREATE INDEX index_sbom_occurrences_on_component_id ON sbom_occurrences USING btree (component_id); CREATE INDEX index_sbom_occurrences_on_component_version_id ON sbom_occurrences USING btree (component_version_id); +CREATE UNIQUE INDEX index_sbom_occurrences_on_ingestion_attributes ON sbom_occurrences USING btree (project_id, component_id, component_version_id, source_id, commit_sha); + CREATE INDEX index_sbom_occurrences_on_pipeline_id ON sbom_occurrences USING btree (pipeline_id); CREATE INDEX index_sbom_occurrences_on_project_id ON sbom_occurrences USING btree (project_id); CREATE INDEX index_sbom_occurrences_on_source_id ON sbom_occurrences USING btree (source_id); +CREATE UNIQUE INDEX index_sbom_sources_on_source_type_and_source ON sbom_sources USING btree (source_type, source); + CREATE INDEX index_scim_identities_on_group_id ON scim_identities USING btree (group_id); CREATE UNIQUE INDEX index_scim_identities_on_lower_extern_uid_and_group_id ON scim_identities USING btree (lower((extern_uid)::text), group_id); diff --git a/doc/administration/geo/index.md b/doc/administration/geo/index.md index 7255f9a9a25..a336f5ff8bc 100644 --- a/doc/administration/geo/index.md +++ b/doc/administration/geo/index.md @@ -206,6 +206,7 @@ This list of limitations only reflects the latest version of GitLab. If you are - [Selective synchronization](replication/configuration.md#selective-synchronization) only limits what repositories and files are replicated. The entire PostgreSQL data is still replicated. Selective synchronization is not built to accommodate compliance / export control use cases. - [Pages access control](../../user/project/pages/pages_access_control.md) doesn't work on secondaries. See [GitLab issue #9336](https://gitlab.com/gitlab-org/gitlab/-/issues/9336) for details. - [GitLab chart with Geo](https://docs.gitlab.com/charts/advanced/geo/) does not support [Unified URLs](secondary_proxy/index.md#set-up-a-unified-url-for-geo-sites). See [GitLab issue #3522](https://gitlab.com/gitlab-org/charts/gitlab/-/issues/3522) for more details. +- [Disaster recovery](disaster_recovery/index.md) for multi-secondary sites causes downtime due to the complete re-synchronization and re-configuration of all non-promoted secondaries. ### Limitations on replication/verification diff --git a/doc/development/documentation/feature_flags.md b/doc/development/documentation/feature_flags.md index fea65e05d49..dae62fea603 100644 --- a/doc/development/documentation/feature_flags.md +++ b/doc/development/documentation/feature_flags.md @@ -34,7 +34,7 @@ Possible version history entries are: > - [Enabled on GitLab.com](issue-link) in GitLab X.X. > - [Enabled on GitLab.com](issue-link) in GitLab X.X. Available to GitLab.com administrators only. > - [Enabled on self-managed](issue-link) in GitLab X.X. -> - [Generally available](issue-link) in GitLab X.Y. [Feature flag `flag_name`](issue-link) removed. +> - [Generally available](issue-link) in GitLab X.Y. Feature flag `flag_name` removed. ``` You can combine entries if they happened in the same release: @@ -115,5 +115,5 @@ And, when the feature is done and fully available to all users: > - Introduced in GitLab 13.7 [with a flag](../../administration/feature_flags.md) named `forti_token_cloud`. Disabled by default. > - [Enabled on self-managed](https://gitlab.com/issue/etc) in GitLab 13.8. > - [Enabled on GitLab.com](https://gitlab.com/issue/etc) in GitLab 13.9. -> - [Generally available](issue-link) in GitLab 14.0. [Feature flag `forti_token_cloud`](issue-link) removed. +> - [Generally available](issue-link) in GitLab 14.0. Feature flag `forti_token_cloud` removed. ``` diff --git a/doc/user/project/merge_requests/reviews/data_usage.md b/doc/user/project/merge_requests/reviews/data_usage.md index 0988e3f0042..460841838bc 100644 --- a/doc/user/project/merge_requests/reviews/data_usage.md +++ b/doc/user/project/merge_requests/reviews/data_usage.md @@ -1,7 +1,7 @@ --- stage: ModelOps group: Applied ML -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments type: index, reference --- diff --git a/lib/api/entities/license.rb b/lib/api/entities/license.rb index d7a414344c1..8ecf8a430fe 100644 --- a/lib/api/entities/license.rb +++ b/lib/api/entities/license.rb @@ -2,6 +2,7 @@ module API module Entities + # Serializes a Licensee::License class License < Entities::LicenseBasic expose :popular?, as: :popular expose(:description) { |license| license.meta['description'] } diff --git a/lib/api/entities/license_basic.rb b/lib/api/entities/license_basic.rb index 08af68785a9..0916738d21d 100644 --- a/lib/api/entities/license_basic.rb +++ b/lib/api/entities/license_basic.rb @@ -2,10 +2,16 @@ module API module Entities + # Serializes a Gitlab::Git::DeclaredLicense class LicenseBasic < Grape::Entity expose :key, :name, :nickname expose :url, as: :html_url - expose(:source_url) { |license| license.meta['source'] } + + # This was dropped: + # https://github.com/github/choosealicense.com/commit/325806b42aa3d5b78e84120327ec877bc936dbdd#diff-66df8f1997786f7052d29010f2cbb4c66391d60d24ca624c356acc0ab986f139 + expose :source_url do |_| + nil + end end end end diff --git a/lib/api/issues.rb b/lib/api/issues.rb index b6ad34424a6..b8b4019765d 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -272,17 +272,21 @@ module API begin spam_params = ::Spam::SpamParams.new_from_request(request: request) - issue = ::Issues::CreateService.new(project: user_project, - current_user: current_user, - params: issue_params, - spam_params: spam_params).execute + result = ::Issues::CreateService.new(project: user_project, + current_user: current_user, + params: issue_params, + spam_params: spam_params).execute + + if result.success? + present result[:issue], with: Entities::Issue, current_user: current_user, project: user_project + elsif result[:issue] + issue = result[:issue] - if issue.valid? - present issue, with: Entities::Issue, current_user: current_user, project: user_project - else with_captcha_check_rest_api(spammable: issue) do render_validation_error!(issue) end + else + render_api_error!(result.errors.join(', '), result.http_status || 422) end rescue ::ActiveRecord::RecordNotUnique render_api_error!('Duplicated issue', 409) diff --git a/lib/gitlab/email/handler/create_issue_handler.rb b/lib/gitlab/email/handler/create_issue_handler.rb index 7b31dd9926b..434893eab82 100644 --- a/lib/gitlab/email/handler/create_issue_handler.rb +++ b/lib/gitlab/email/handler/create_issue_handler.rb @@ -36,8 +36,14 @@ module Gitlab validate_permission!(:create_issue) + result = create_issue + issue = result[:issue] + + # issue won't be present only on unrecoverable errors + raise InvalidIssueError, result.errors.join(', ') if result.error? && issue.blank? + verify_record!( - record: create_issue, + record: issue, invalid_exception: InvalidIssueError, record_name: 'issue') end diff --git a/lib/gitlab/email/handler/service_desk_handler.rb b/lib/gitlab/email/handler/service_desk_handler.rb index 8e2c7559bc1..06365296a76 100644 --- a/lib/gitlab/email/handler/service_desk_handler.rb +++ b/lib/gitlab/email/handler/service_desk_handler.rb @@ -91,7 +91,7 @@ module Gitlab end def create_issue! - @issue = ::Issues::CreateService.new( + result = ::Issues::CreateService.new( project: project, current_user: User.support_bot, params: { @@ -106,7 +106,9 @@ module Gitlab spam_params: nil ).execute - raise InvalidIssueError unless @issue.persisted? + raise InvalidIssueError if result.error? + + @issue = result[:issue] begin ::Issue::Email.create!(issue: @issue, email_message_id: mail.message_id) diff --git a/lib/gitlab/git/declared_license.rb b/lib/gitlab/git/declared_license.rb new file mode 100644 index 00000000000..bc12b1918ea --- /dev/null +++ b/lib/gitlab/git/declared_license.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module Gitlab + module Git + # DeclaredLicense is the software license declared in a LICENSE or COPYING + # file in the git repository. + class DeclaredLicense + # SPDX Identifier for the license + attr_reader :key + + # Full name of the license + attr_reader :name + + # Nickname of the license (optional, a shorter user-friendly name) + attr_reader :nickname + + # Filename of the file containing license + attr_accessor :path + + # URL that points to the LICENSE + attr_reader :url + + def initialize(key: nil, name: nil, nickname: nil, url: nil, path: nil) + @key = key + @name = name + @nickname = nickname + @url = url + @path = path + end + + def ==(other) + return unless other.is_a?(self.class) + + key == other.key + end + end + end +end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index f1cd75258be..d9b08f1d55a 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -783,10 +783,29 @@ module Gitlab end end - def license_short_name + def license(from_gitaly) wrapped_gitaly_errors do - gitaly_repository_client.license_short_name + response = gitaly_repository_client.find_license + + break nil if response.license_short_name.empty? + + if from_gitaly + break ::Gitlab::Git::DeclaredLicense.new(key: response.license_short_name, + name: response.license_name, + nickname: response.license_nickname.presence, + url: response.license_url.presence, + path: response.license_path) + end + + licensee_object = Licensee::License.new(response.license_short_name) + + break nil if licensee_object.name.blank? + + licensee_object end + rescue Licensee::InvalidLicense => e + Gitlab::ErrorTracking.track_exception(e) + nil end def fetch_source_branch!(source_repository, source_branch, local_ref) diff --git a/lib/gitlab/gitaly_client/repository_service.rb b/lib/gitlab/gitaly_client/repository_service.rb index 04d6f92e8d8..c9e584a07ae 100644 --- a/lib/gitlab/gitaly_client/repository_service.rb +++ b/lib/gitlab/gitaly_client/repository_service.rb @@ -283,12 +283,10 @@ module Gitlab response.path.presence end - def license_short_name + def find_license request = Gitaly::FindLicenseRequest.new(repository: @gitaly_repo) - response = GitalyClient.call(@storage, :repository_service, :find_license, request, timeout: GitalyClient.fast_timeout) - - response.license_short_name.presence + GitalyClient.call(@storage, :repository_service, :find_license, request, timeout: GitalyClient.fast_timeout) end def calculate_checksum diff --git a/lib/gitlab/slash_commands/issue_new.rb b/lib/gitlab/slash_commands/issue_new.rb index ef368767689..1527fd263e0 100644 --- a/lib/gitlab/slash_commands/issue_new.rb +++ b/lib/gitlab/slash_commands/issue_new.rb @@ -21,12 +21,16 @@ module Gitlab title = match[:title] description = match[:description].to_s.rstrip - issue = create_issue(title: title, description: description) + result = create_issue(title: title, description: description) - if issue.persisted? - presenter(issue).present + if result.success? + presenter(result[:issue]).present + elsif result[:issue] + presenter(result[:issue]).display_errors else - presenter(issue).display_errors + Gitlab::SlashCommands::Presenters::Error.new( + result.errors.join(', ') + ).message end end diff --git a/lib/quality/seeders/issues.rb b/lib/quality/seeders/issues.rb index 5d345dd30a1..bf42913e043 100644 --- a/lib/quality/seeders/issues.rb +++ b/lib/quality/seeders/issues.rb @@ -31,9 +31,9 @@ module Quality } params[:closed_at] = params[:created_at] + rand(35).days if params[:state] == 'closed' - issue = ::Issues::CreateService.new(project: project, current_user: team.sample, params: params, spam_params: nil).execute_without_rate_limiting + create_result = ::Issues::CreateService.new(project: project, current_user: team.sample, params: params, spam_params: nil).execute_without_rate_limiting - if issue.persisted? + if create_result.success? created_issues_count += 1 print '.' # rubocop:disable Rails/Output end diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index 70e58124d21..e9b39d44e46 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -96,7 +96,7 @@ RSpec.describe AutocompleteController do end context 'user order' do - it 'shows exact matches first' do + it 'shows exact matches first', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/375028' do reported_user = create(:user, username: 'reported_user', name: 'Doug') user = create(:user, username: 'user', name: 'User') user1 = create(:user, username: 'user1', name: 'Ian') diff --git a/spec/controllers/boards/issues_controller_spec.rb b/spec/controllers/boards/issues_controller_spec.rb index 1fd249eba69..6bf2f088ab9 100644 --- a/spec/controllers/boards/issues_controller_spec.rb +++ b/spec/controllers/boards/issues_controller_spec.rb @@ -481,6 +481,23 @@ RSpec.describe Boards::IssuesController do end end + context 'when create service returns an unrecoverable error' do + before do + allow_next_instance_of(Issues::CreateService) do |create_service| + allow(create_service).to receive(:execute).and_return( + ServiceResponse.error(message: 'unrecoverable error', http_status: 404) + ) + end + end + + it 'returns an array with errors an service http_status' do + create_issue user: user, board: board, list: list1, title: 'New issue' + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response).to contain_exactly('unrecoverable error') + end + end + context 'with guest user' do context 'in open list' do it 'returns a successful 200 response' do diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index c48be8efb1b..64d4b276519 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -1107,6 +1107,46 @@ RSpec.describe Projects::IssuesController do end end + context 'when create service return an unrecoverable error with http_status' do + let(:http_status) { 403 } + + before do + allow_next_instance_of(::Issues::CreateService) do |create_service| + allow(create_service).to receive(:execute).and_return( + ServiceResponse.error(message: 'unrecoverable error', http_status: http_status) + ) + end + end + + it 'renders 403 and logs the error' do + expect(Gitlab::AppLogger).to receive(:warn).with( + message: 'Cannot create issue', + errors: ['unrecoverable error'], + http_status: http_status + ) + + post_new_issue + + expect(response).to have_gitlab_http_status :forbidden + end + + context 'when no render method is found for the returned http_status' do + let(:http_status) { nil } + + it 'renders 404 and logs the error' do + expect(Gitlab::AppLogger).to receive(:warn).with( + message: 'Cannot create issue', + errors: ['unrecoverable error'], + http_status: http_status + ) + + post_new_issue + + expect(response).to have_gitlab_http_status :not_found + end + end + end + it 'creates the issue successfully', :aggregate_failures do issue = post_new_issue diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index b30610d98d7..1f7c124a601 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -299,14 +299,16 @@ RSpec.describe ProjectsController do end it "renders files even with invalid license" do + invalid_license = ::Gitlab::Git::DeclaredLicense.new(key: 'woozle', name: 'woozle wuzzle') + controller.instance_variable_set(:@project, public_project) - expect(public_project.repository).to receive(:license_key).and_return('woozle wuzzle').at_least(:once) + expect(public_project.repository).to receive(:license).and_return(invalid_license).at_least(:once) get_show expect(response).to have_gitlab_http_status(:ok) expect(response).to render_template('_files') - expect(response.body).to have_content('LICENSE') # would be 'MIT license' if stub not works + expect(response.body).to have_content('woozle wuzzle') end describe 'tracking events', :snowplow do diff --git a/spec/factories/ci/reports/sbom/components.rb b/spec/factories/ci/reports/sbom/components.rb index 317e1c863cf..fd9b4386130 100644 --- a/spec/factories/ci/reports/sbom/components.rb +++ b/spec/factories/ci/reports/sbom/components.rb @@ -2,7 +2,7 @@ FactoryBot.define do factory :ci_reports_sbom_component, class: '::Gitlab::Ci::Reports::Sbom::Component' do - type { :library } + type { "library" } sequence(:name) { |n| "component-#{n}" } sequence(:version) { |n| "v0.0.#{n}" } diff --git a/spec/features/merge_request/user_posts_diff_notes_spec.rb b/spec/features/merge_request/user_posts_diff_notes_spec.rb index 1eebb6c2e28..47a25533809 100644 --- a/spec/features/merge_request/user_posts_diff_notes_spec.rb +++ b/spec/features/merge_request/user_posts_diff_notes_spec.rb @@ -219,7 +219,7 @@ RSpec.describe 'Merge request > User posts diff notes', :js do end context 'with a match line' do - it 'does not allow commenting' do + it 'does not allow commenting', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/375024' do match_should_not_allow_commenting(find_by_scrolling('.match', match: :first)) end end diff --git a/spec/features/projects/blobs/blob_show_spec.rb b/spec/features/projects/blobs/blob_show_spec.rb index 13a4c1b5912..93e5be18229 100644 --- a/spec/features/projects/blobs/blob_show_spec.rb +++ b/spec/features/projects/blobs/blob_show_spec.rb @@ -675,7 +675,7 @@ RSpec.describe 'File blob', :js do expect(page).to have_content('This project is licensed under the MIT License.') # shows a learn more link - expect(page).to have_link('Learn more', href: 'http://choosealicense.com/licenses/mit/') + expect(page).to have_link('Learn more', href: 'https://opensource.org/licenses/MIT') end end end diff --git a/spec/features/unsubscribe_links_spec.rb b/spec/features/unsubscribe_links_spec.rb index 5317f586390..f90b2060418 100644 --- a/spec/features/unsubscribe_links_spec.rb +++ b/spec/features/unsubscribe_links_spec.rb @@ -9,7 +9,7 @@ RSpec.describe 'Unsubscribe links', :sidekiq_might_not_need_inline do let(:author) { create(:user) } let(:project) { create(:project, :public) } let(:params) { { title: 'A bug!', description: 'Fix it!', assignees: [recipient] } } - let(:issue) { Issues::CreateService.new(project: project, current_user: author, params: params, spam_params: nil).execute } + let(:issue) { Issues::CreateService.new(project: project, current_user: author, params: params, spam_params: nil).execute[:issue] } let(:mail) { ActionMailer::Base.deliveries.last } let(:body) { Capybara::Node::Simple.new(mail.default_part_body.to_s) } diff --git a/spec/frontend/__helpers__/stub_component.js b/spec/frontend/__helpers__/stub_component.js index 4f9d1ee6f5d..3e9af994ee3 100644 --- a/spec/frontend/__helpers__/stub_component.js +++ b/spec/frontend/__helpers__/stub_component.js @@ -38,7 +38,7 @@ export function stubComponent(Component, options = {}) { // Do not render any slots/scoped slots except default // This differs from VTU behavior which renders all slots template: '
', - // allows wrapper.find(Component) to work for stub + // allows wrapper.findComponent(Component) to work for stub $_vueTestUtils_original: Component, ...options, }; diff --git a/spec/frontend/pipelines/pipelines_spec.js b/spec/frontend/pipelines/pipelines_spec.js index cc2ff90de57..bdde2874535 100644 --- a/spec/frontend/pipelines/pipelines_spec.js +++ b/spec/frontend/pipelines/pipelines_spec.js @@ -261,9 +261,14 @@ describe('Pipelines', () => { ); }); - it('tracks tab change click', () => { + it.each(['all', 'finished', 'branches', 'tags'])('tracks %p tab click', async (scope) => { + goToTab(scope); + + await waitForPromises(); + expect(trackingSpy).toHaveBeenCalledWith(undefined, 'click_filter_tabs', { label: TRACKING_CATEGORIES.tabs, + property: scope, }); }); }); diff --git a/spec/frontend/vue_merge_request_widget/deployment/deployment_spec.js b/spec/frontend/vue_merge_request_widget/deployment/deployment_spec.js index 12ee100bb9c..f310f7669a9 100644 --- a/spec/frontend/vue_merge_request_widget/deployment/deployment_spec.js +++ b/spec/frontend/vue_merge_request_widget/deployment/deployment_spec.js @@ -137,9 +137,11 @@ describe('Deployment component', () => { if (actionButtons.includes(DeploymentViewButton)) { it('renders the View button with expected text', () => { if (status === SUCCESS) { - expect(wrapper.find(DeploymentViewButton).text()).toContain('View app'); + expect(wrapper.findComponent(DeploymentViewButton).text()).toContain('View app'); } else { - expect(wrapper.find(DeploymentViewButton).text()).toContain('View latest app'); + expect(wrapper.findComponent(DeploymentViewButton).text()).toContain( + 'View latest app', + ); } }); } diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index a9db2a1c008..a3826ec8eb8 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -825,7 +825,7 @@ RSpec.describe ProjectsHelper do end context 'gitaly is working appropriately' do - let(:license) { Licensee::License.new('mit') } + let(:license) { ::Gitlab::Git::DeclaredLicense.new(key: 'mit', name: 'MIT License') } before do expect(repository).to receive(:license).and_return(license) diff --git a/spec/helpers/sessions_helper_spec.rb b/spec/helpers/sessions_helper_spec.rb index 15424425060..c7b8225b866 100644 --- a/spec/helpers/sessions_helper_spec.rb +++ b/spec/helpers/sessions_helper_spec.rb @@ -92,6 +92,12 @@ RSpec.describe SessionsHelper do end context 'when an email address is very short' do + let(:email) { 'a@b.c' } + + it { is_expected.to eq('a@b.c') } + end + + context 'when an email address is even shorter' do let(:email) { 'a@b' } it { is_expected.to eq('a@b') } diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 9a87911b6e8..52c6557b93b 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1784,22 +1784,32 @@ RSpec.describe Gitlab::Git::Repository do end end - describe '#license_short_name' do - subject { repository.license_short_name } + describe '#license' do + where(from_gitaly: [true, false]) + with_them do + subject(:license) { repository.license(from_gitaly) } - context 'when no license file can be found' do - let(:project) { create(:project, :repository) } - let(:repository) { project.repository.raw_repository } + context 'when no license file can be found' do + let(:project) { create(:project, :repository) } + let(:repository) { project.repository.raw_repository } - before do - project.repository.delete_file(project.owner, 'LICENSE', message: 'remove license', branch_name: 'master') + before do + project.repository.delete_file(project.owner, 'LICENSE', message: 'remove license', branch_name: 'master') + end + + it { is_expected.to be_nil } end - it { is_expected.to be_nil } + context 'when an mit license is found' do + it { is_expected.to have_attributes(key: 'mit') } + end end - context 'when an mit license is found' do - it { is_expected.to eq('mit') } + it 'does not crash when license is not recognized' do + expect(Licensee::License).to receive(:new) + .and_raise(Licensee::InvalidLicense) + + expect(repository.license(false)).to be_nil end end diff --git a/spec/lib/gitlab/slash_commands/issue_new_spec.rb b/spec/lib/gitlab/slash_commands/issue_new_spec.rb index c17cee887ee..29a941f3691 100644 --- a/spec/lib/gitlab/slash_commands/issue_new_spec.rb +++ b/spec/lib/gitlab/slash_commands/issue_new_spec.rb @@ -53,6 +53,21 @@ RSpec.describe Gitlab::SlashCommands::IssueNew do expect(subject[:response_type]).to be(:ephemeral) expect(subject[:text]).to match("- Title is too long") end + + context 'when create issue service return an unrecoverable error' do + let(:regex_match) { described_class.match("issue create title}") } + + before do + allow_next_instance_of(Issues::CreateService) do |create_service| + allow(create_service).to receive(:execute).and_return(ServiceResponse.error(message: 'unauthorized')) + end + end + + it 'displays the errors' do + expect(subject[:response_type]).to be(:ephemeral) + expect(subject[:text]).to eq('unauthorized') + end + end end end diff --git a/spec/models/integrations/microsoft_teams_spec.rb b/spec/models/integrations/microsoft_teams_spec.rb index b1b3e42b5e9..89ccc9eaf35 100644 --- a/spec/models/integrations/microsoft_teams_spec.rb +++ b/spec/models/integrations/microsoft_teams_spec.rb @@ -81,7 +81,7 @@ RSpec.describe Integrations::MicrosoftTeams do let(:opts) { { title: 'Awesome issue', description: 'please fix' } } let(:issues_sample_data) do service = Issues::CreateService.new(project: project, current_user: user, params: opts, spam_params: nil) - issue = service.execute + issue = service.execute[:issue] service.hook_data(issue, 'open') end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index d77a8f50cd1..bd46fb9b1a6 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1331,7 +1331,7 @@ RSpec.describe Repository do end end - describe '#license_key', :use_clean_rails_memory_store_caching do + describe '#license_key', :clean_gitlab_redis_cache do let(:project) { create(:project, :repository) } before do @@ -1377,48 +1377,46 @@ RSpec.describe Repository do end end - describe '#license' do - let(:project) { create(:project, :repository) } - - before do - repository.delete_file(user, 'LICENSE', - message: 'Remove LICENSE', branch_name: 'master') - end - - it 'returns nil when no license is detected' do - expect(repository.license).to be_nil - end - - it 'returns nil when the repository does not exist' do - expect(repository).to receive(:exists?).and_return(false) - - expect(repository.license).to be_nil - end - - it 'returns nil when license_key is not recognized' do - expect(repository).to receive(:license_key).twice.and_return('not-recognized') - expect(Gitlab::ErrorTracking).to receive(:track_exception) do |ex| - expect(ex).to be_a(Licensee::InvalidLicense) + [true, false].each do |ff| + context "with feature flag license_from_gitaly=#{ff}" do + before do + stub_feature_flags(license_from_gitaly: ff) end - expect(repository.license).to be_nil - end + describe '#license', :use_clean_rails_memory_store_caching, :clean_gitlab_redis_cache do + let(:project) { create(:project, :repository) } - it 'returns other when the content is not recognizable' do - license = Licensee::License.new('other') - repository.create_file(user, 'LICENSE', 'Gitlab B.V.', - message: 'Add LICENSE', branch_name: 'master') + before do + repository.delete_file(user, 'LICENSE', + message: 'Remove LICENSE', branch_name: 'master') + end - expect(repository.license).to eq(license) - end + it 'returns nil when no license is detected' do + expect(repository.license).to be_nil + end - it 'returns the license' do - license = Licensee::License.new('mit') - repository.create_file(user, 'LICENSE', - license.content, - message: 'Add LICENSE', branch_name: 'master') + it 'returns nil when the repository does not exist' do + expect(repository).to receive(:exists?).and_return(false) - expect(repository.license).to eq(license) + expect(repository.license).to be_nil + end + + it 'returns other when the content is not recognizable' do + repository.create_file(user, 'LICENSE', 'Gitlab B.V.', + message: 'Add LICENSE', branch_name: 'master') + + expect(repository.license_key).to eq('other') + end + + it 'returns the license' do + license = Licensee::License.new('mit') + repository.create_file(user, 'LICENSE', + license.content, + message: 'Add LICENSE', branch_name: 'master') + + expect(repository.license_key).to eq(license.key) + end + end end end @@ -2207,7 +2205,8 @@ RSpec.describe Repository do :contribution_guide, :changelog, :license_blob, - :license_key, + :license_licensee, + :license_gitaly, :gitignore, :gitlab_ci_yml, :branch_names, @@ -2695,7 +2694,7 @@ RSpec.describe Repository do match[1].to_sym if match end.compact - expect(Repository::CACHED_METHODS + Repository::MEMOIZED_CACHED_METHODS).to include(*methods) + expect(Repository::CACHED_METHODS).to include(*methods) end end @@ -2860,12 +2859,12 @@ RSpec.describe Repository do describe '#refresh_method_caches' do it 'refreshes the caches of the given types' do expect(repository).to receive(:expire_method_caches) - .with(%i(readme_path license_blob license_key license)) + .with(%i(readme_path license_blob license_licensee license_gitaly)) expect(repository).to receive(:readme_path) expect(repository).to receive(:license_blob) - expect(repository).to receive(:license_key) - expect(repository).to receive(:license) + expect(repository).to receive(:license_licensee) + expect(repository).to receive(:license_gitaly) repository.refresh_method_caches(%i(readme license)) end diff --git a/spec/presenters/project_presenter_spec.rb b/spec/presenters/project_presenter_spec.rb index 7ff19b1b770..832deee6186 100644 --- a/spec/presenters/project_presenter_spec.rb +++ b/spec/presenters/project_presenter_spec.rb @@ -10,13 +10,15 @@ RSpec.describe ProjectPresenter do describe '#license_short_name' do context 'when project.repository has a license_key' do it 'returns the nickname of the license if present' do - allow(project.repository).to receive(:license_key).and_return('agpl-3.0') + allow(project.repository).to receive(:license).and_return( + ::Gitlab::Git::DeclaredLicense.new(name: 'foo', nickname: 'GNU AGPLv3')) expect(presenter.license_short_name).to eq('GNU AGPLv3') end it 'returns the name of the license if nickname is not present' do - allow(project.repository).to receive(:license_key).and_return('mit') + allow(project.repository).to receive(:license).and_return( + ::Gitlab::Git::DeclaredLicense.new(name: 'MIT License')) expect(presenter.license_short_name).to eq('MIT License') end @@ -24,7 +26,7 @@ RSpec.describe ProjectPresenter do context 'when project.repository has no license_key but a license_blob' do it 'returns LICENSE' do - allow(project.repository).to receive(:license_key).and_return(nil) + allow(project.repository).to receive(:license).and_return(nil) expect(presenter.license_short_name).to eq('LICENSE') end diff --git a/spec/requests/api/issues/issues_spec.rb b/spec/requests/api/issues/issues_spec.rb index dd7d32f3565..f5c73846173 100644 --- a/spec/requests/api/issues/issues_spec.rb +++ b/spec/requests/api/issues/issues_spec.rb @@ -1164,6 +1164,21 @@ RSpec.describe API::Issues do expect(json_response['title']).to eq('new issue') expect(json_response['issue_type']).to eq('issue') end + + context 'when issue create service returns an unrecoverable error' do + before do + allow_next_instance_of(Issues::CreateService) do |create_service| + allow(create_service).to receive(:execute).and_return(ServiceResponse.error(message: 'some error', http_status: 403)) + end + end + + it 'returns and error message and status code from the service' do + post api("/projects/#{project.id}/issues", user), params: { title: 'new issue' } + + expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response['message']).to eq('some error') + end + end end describe 'PUT /projects/:id/issues/:issue_iid' do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 7ad1ce0ede9..4ffc6055a23 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -2516,7 +2516,7 @@ RSpec.describe API::Projects do 'name' => project.repository.license.name, 'nickname' => project.repository.license.nickname, 'html_url' => project.repository.license.url, - 'source_url' => project.repository.license.meta['source'] + 'source_url' => nil }) end diff --git a/spec/services/alert_management/create_alert_issue_service_spec.rb b/spec/services/alert_management/create_alert_issue_service_spec.rb index 083e5b8c6f1..fc2bd9e90a6 100644 --- a/spec/services/alert_management/create_alert_issue_service_spec.rb +++ b/spec/services/alert_management/create_alert_issue_service_spec.rb @@ -161,7 +161,7 @@ RSpec.describe AlertManagement::CreateAlertIssueService do it 'has an unsuccessful status' do expect(execute).to be_error - expect(execute.message).to eq("Title can't be blank") + expect(execute.errors).to contain_exactly("Title can't be blank") end end @@ -170,7 +170,7 @@ RSpec.describe AlertManagement::CreateAlertIssueService do it 'responds with error' do expect(execute).to be_error - expect(execute.message).to eq('Hosts hosts array is over 255 chars') + expect(execute.errors).to contain_exactly('Hosts hosts array is over 255 chars') end end diff --git a/spec/services/boards/issues/create_service_spec.rb b/spec/services/boards/issues/create_service_spec.rb index 9a6b48c13bf..c4f1eb093dc 100644 --- a/spec/services/boards/issues/create_service_spec.rb +++ b/spec/services/boards/issues/create_service_spec.rb @@ -29,9 +29,10 @@ RSpec.describe Boards::Issues::CreateService do end it 'adds the label of the list to the issue' do - issue = service.execute + result = service.execute - expect(issue.labels).to eq [label] + expect(result).to be_success + expect(result[:issue].labels).to contain_exactly(label) end end end diff --git a/spec/services/incident_management/incidents/create_service_spec.rb b/spec/services/incident_management/incidents/create_service_spec.rb index ac44bc4608c..851b21e1227 100644 --- a/spec/services/incident_management/incidents/create_service_spec.rb +++ b/spec/services/incident_management/incidents/create_service_spec.rb @@ -77,7 +77,7 @@ RSpec.describe IncidentManagement::Incidents::CreateService do it 'responds with errors' do expect(create_incident).to be_error - expect(create_incident.message).to eq("Title can't be blank") + expect(create_incident.errors).to contain_exactly("Title can't be blank") end it 'result payload contains an Issue object' do @@ -98,7 +98,7 @@ RSpec.describe IncidentManagement::Incidents::CreateService do it 'responds with errors' do expect(create_incident).to be_error - expect(create_incident.message).to eq('Hosts hosts array is over 255 chars') + expect(create_incident.errors).to contain_exactly('Hosts hosts array is over 255 chars') end end end diff --git a/spec/services/incident_management/pager_duty/create_incident_issue_service_spec.rb b/spec/services/incident_management/pager_duty/create_incident_issue_service_spec.rb index fb536df5d17..572b1a20166 100644 --- a/spec/services/incident_management/pager_duty/create_incident_issue_service_spec.rb +++ b/spec/services/incident_management/pager_duty/create_incident_issue_service_spec.rb @@ -63,7 +63,7 @@ RSpec.describe IncidentManagement::PagerDuty::CreateIncidentIssueService do it 'responds with error' do expect(execute).to be_error - expect(execute.message).to eq("Title can't be blank") + expect(execute.errors).to contain_exactly("Title can't be blank") end end end diff --git a/spec/services/issues/clone_service_spec.rb b/spec/services/issues/clone_service_spec.rb index 435488b7f66..67f32b85350 100644 --- a/spec/services/issues/clone_service_spec.rb +++ b/spec/services/issues/clone_service_spec.rb @@ -36,6 +36,21 @@ RSpec.describe Issues::CloneService do context 'issue movable' do include_context 'user can clone issue' + context 'when issue creation fails' do + before do + allow_next_instance_of(Issues::CreateService) do |create_service| + allow(create_service).to receive(:execute).and_return(ServiceResponse.error(message: 'some error')) + end + end + + it 'raises a clone error' do + expect { clone_service.execute(old_issue, new_project) }.to raise_error( + Issues::CloneService::CloneError, + 'some error' + ) + end + end + context 'generic issue' do let!(:new_issue) { clone_service.execute(old_issue, new_project, with_notes: with_notes) } diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 4a84862b9d5..b2b7de338da 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -23,12 +23,28 @@ RSpec.describe Issues::CreateService do let_it_be(:assignee) { create(:user) } let_it_be(:milestone) { create(:milestone, project: project) } - let(:issue) { described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute } + let(:result) { described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute } + let(:issue) { result[:issue] } before do stub_spam_services end + context 'when params are invalid' do + let(:opts) { { title: '' } } + + before_all do + project.add_guest(user) + project.add_guest(assignee) + end + + it 'returns an error service response' do + expect(result).to be_error + expect(result.errors).to include("Title can't be blank") + expect(issue).not_to be_persisted + end + end + context 'when params are valid' do let_it_be(:labels) { create_pair(:label, project: project) } @@ -58,6 +74,7 @@ RSpec.describe Issues::CreateService do it 'creates the issue with the given params' do expect(Issuable::CommonSystemNotesService).to receive_message_chain(:new, :execute) + expect(result).to be_success expect(issue).to be_persisted expect(issue).to be_a(::Issue) expect(issue.title).to eq('Awesome issue') @@ -76,12 +93,13 @@ RSpec.describe Issues::CreateService do end context 'when a build_service is provided' do - let(:issue) { described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params, build_service: build_service).execute } + let(:result) { described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params, build_service: build_service).execute } let(:issue_from_builder) { WorkItem.new(project: project, title: 'Issue from builder') } let(:build_service) { double(:build_service, execute: issue_from_builder) } it 'uses the provided service to build the issue' do + expect(result).to be_success expect(issue).to be_persisted expect(issue).to be_a(WorkItem) end @@ -106,6 +124,7 @@ RSpec.describe Issues::CreateService do end it 'sets the correct relative position' do + expect(result).to be_success expect(issue).to be_persisted expect(issue.relative_position).to be_present expect(issue.relative_position).to be_between(issue_before.relative_position, issue_after.relative_position) @@ -183,8 +202,10 @@ RSpec.describe Issues::CreateService do let_it_be(:non_member) { create(:user) } it 'filters out params that cannot be set without the :set_issue_metadata permission' do - issue = described_class.new(project: project, current_user: non_member, params: opts, spam_params: spam_params).execute + result = described_class.new(project: project, current_user: non_member, params: opts, spam_params: spam_params).execute + issue = result[:issue] + expect(result).to be_success expect(issue).to be_persisted expect(issue.title).to eq('Awesome issue') expect(issue.description).to eq('please fix') @@ -195,8 +216,10 @@ RSpec.describe Issues::CreateService do end it 'can create confidential issues' do - issue = described_class.new(project: project, current_user: non_member, params: { confidential: true }, spam_params: spam_params).execute + result = described_class.new(project: project, current_user: non_member, params: opts.merge(confidential: true), spam_params: spam_params).execute + issue = result[:issue] + expect(result).to be_success expect(issue.confidential).to be_truthy end end @@ -391,16 +414,20 @@ RSpec.describe Issues::CreateService do it 'removes assignee when user id is invalid' do opts = { title: 'Title', description: 'Description', assignee_ids: [-1] } - issue = described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute + result = described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute + issue = result[:issue] + expect(result).to be_success expect(issue.assignees).to be_empty end it 'removes assignee when user id is 0' do opts = { title: 'Title', description: 'Description', assignee_ids: [0] } - issue = described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute + result = described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute + issue = result[:issue] + expect(result).to be_success expect(issue.assignees).to be_empty end @@ -408,8 +435,10 @@ RSpec.describe Issues::CreateService do project.add_maintainer(assignee) opts = { title: 'Title', description: 'Description', assignee_ids: [assignee.id] } - issue = described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute + result = described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute + issue = result[:issue] + expect(result).to be_success expect(issue.assignees).to eq([assignee]) end @@ -426,8 +455,10 @@ RSpec.describe Issues::CreateService do project.update!(visibility_level: level) opts = { title: 'Title', description: 'Description', assignee_ids: [assignee.id] } - issue = described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute + result = described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute + issue = result[:issue] + expect(result).to be_success expect(issue.assignees).to be_empty end end @@ -436,7 +467,7 @@ RSpec.describe Issues::CreateService do end it_behaves_like 'issuable record that supports quick actions' do - let(:issuable) { described_class.new(project: project, current_user: user, params: params, spam_params: spam_params).execute } + let(:issuable) { described_class.new(project: project, current_user: user, params: params, spam_params: spam_params).execute[:issue] } end context 'Quick actions' do @@ -459,6 +490,7 @@ RSpec.describe Issues::CreateService do end it 'assigns, sets milestone, and sets contact to issuable from command' do + expect(result).to be_success expect(issue).to be_persisted expect(issue.assignees).to eq([assignee]) expect(issue.milestone).to eq(milestone) @@ -480,6 +512,8 @@ RSpec.describe Issues::CreateService do context 'with permission' do it 'assigns contact to issue' do group.add_reporter(user) + + expect(result).to be_success expect(issue).to be_persisted expect(issue.issue_customer_relations_contacts.last.contact).to eq(contact) end @@ -488,6 +522,8 @@ RSpec.describe Issues::CreateService do context 'without permission' do it 'does not assign contact to issue' do group.add_guest(user) + + expect(result).to be_success expect(issue).to be_persisted expect(issue.issue_customer_relations_contacts).to be_empty end @@ -522,6 +558,7 @@ RSpec.describe Issues::CreateService do end it 'can apply labels' do + expect(result).to be_success expect(issue).to be_persisted expect(issue.labels).to eq([label]) end @@ -556,25 +593,32 @@ RSpec.describe Issues::CreateService do end it 'sets default title and description values if not provided' do - issue = described_class.new( + result = described_class.new( project: project, current_user: user, params: opts, spam_params: spam_params ).execute + issue = result[:issue] + expect(result).to be_success expect(issue).to be_persisted expect(issue.title).to eq("Follow-up from \"#{merge_request.title}\"") expect(issue.description).to include("The following discussion from #{merge_request.to_reference} should be addressed") end it 'takes params from the request over the default values' do - issue = described_class.new(project: project, current_user: user, - params: opts.merge( - description: 'Custom issue description', - title: 'My new issue' - ), - spam_params: spam_params).execute + result = described_class.new( + project: project, + current_user: user, + params: opts.merge( + description: 'Custom issue description', + title: 'My new issue' + ), + spam_params: spam_params + ).execute + issue = result[:issue] + expect(result).to be_success expect(issue).to be_persisted expect(issue.description).to eq('Custom issue description') expect(issue.title).to eq('My new issue') @@ -600,25 +644,32 @@ RSpec.describe Issues::CreateService do end it 'sets default title and description values if not provided' do - issue = described_class.new( + result = described_class.new( project: project, current_user: user, params: opts, spam_params: spam_params ).execute + issue = result[:issue] + expect(result).to be_success expect(issue).to be_persisted expect(issue.title).to eq("Follow-up from \"#{merge_request.title}\"") expect(issue.description).to include("The following discussion from #{merge_request.to_reference} should be addressed") end it 'takes params from the request over the default values' do - issue = described_class.new(project: project, current_user: user, - params: opts.merge( - description: 'Custom issue description', - title: 'My new issue' - ), - spam_params: spam_params).execute + result = described_class.new( + project: project, + current_user: user, + params: opts.merge( + description: 'Custom issue description', + title: 'My new issue' + ), + spam_params: spam_params + ).execute + issue = result[:issue] + expect(result).to be_success expect(issue).to be_persisted expect(issue.description).to eq('Custom issue description') expect(issue.title).to eq('My new issue') @@ -635,6 +686,7 @@ RSpec.describe Issues::CreateService do it 'ignores related issue if not accessible' do expect { issue }.not_to change { IssueLink.count } + expect(result).to be_success expect(issue).to be_persisted end @@ -645,6 +697,7 @@ RSpec.describe Issues::CreateService do it 'adds a link to the issue' do expect { issue }.to change { IssueLink.count }.by(1) + expect(result).to be_success expect(issue).to be_persisted expect(issue.related_issues(user)).to eq([related_issue]) end diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb index 863df810d01..23180f75eb3 100644 --- a/spec/services/issues/move_service_spec.rb +++ b/spec/services/issues/move_service_spec.rb @@ -35,6 +35,23 @@ RSpec.describe Issues::MoveService do let!(:new_issue) { move_service.execute(old_issue, new_project) } end + context 'when issue creation fails' do + include_context 'user can move issue' + + before do + allow_next_instance_of(Issues::CreateService) do |create_service| + allow(create_service).to receive(:execute).and_return(ServiceResponse.error(message: 'some error')) + end + end + + it 'raises a move error' do + expect { move_service.execute(old_issue, new_project) }.to raise_error( + Issues::MoveService::MoveError, + 'some error' + ) + end + end + context 'issue movable' do include_context 'user can move issue' diff --git a/spec/support/shared_examples/models/chat_integration_shared_examples.rb b/spec/support/shared_examples/models/chat_integration_shared_examples.rb index 6cfeeabc952..769011c1675 100644 --- a/spec/support/shared_examples/models/chat_integration_shared_examples.rb +++ b/spec/support/shared_examples/models/chat_integration_shared_examples.rb @@ -166,7 +166,7 @@ RSpec.shared_examples "chat integration" do |integration_name| let(:opts) { { title: "Awesome issue", description: "please fix" } } let(:sample_data) do service = Issues::CreateService.new(project: project, current_user: user, params: opts, spam_params: nil) - issue = service.execute + issue = service.execute[:issue] service.hook_data(issue, "open") end