diff options
37 files changed, 338 insertions, 516 deletions
diff --git a/app/assets/javascripts/ide/stores/actions/file.js b/app/assets/javascripts/ide/stores/actions/file.js index 76e09f96bd6..55ee8a699c8 100644 --- a/app/assets/javascripts/ide/stores/actions/file.js +++ b/app/assets/javascripts/ide/stores/actions/file.js @@ -71,6 +71,7 @@ export const getFileData = ( const url = joinPaths( gon.relative_url_root || '/', state.currentProjectId, + '-', file.type, getters.lastCommit && getters.lastCommit.id, escapeFileUrl(file.prevPath || file.path), diff --git a/app/assets/javascripts/repository/log_tree.js b/app/assets/javascripts/repository/log_tree.js index 6498725adb6..2a66659fecb 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/${ref}/logs_tree/${path.replace(/^\//, '')}`, + `${gon.relative_url_root}/${projectPath}/-/refs/${ref}/logs_tree/${path.replace(/^\//, '')}`, { params: { format: 'json', offset }, }, diff --git a/app/assets/stylesheets/framework/snippets.scss b/app/assets/stylesheets/framework/snippets.scss index 404f60f17ee..fbe241df32f 100644 --- a/app/assets/stylesheets/framework/snippets.scss +++ b/app/assets/stylesheets/framework/snippets.scss @@ -1,6 +1,7 @@ .snippet-row { .title { margin-bottom: 2px; + font-weight: $gl-font-weight-bold; } .snippet-filename { @@ -11,6 +12,10 @@ .snippet-info { color: $gl-text-color-secondary; } + + a { + color: $gl-text-color; + } } .snippet-form-holder .file-holder .file-title { diff --git a/app/controllers/projects/error_tracking_controller.rb b/app/controllers/projects/error_tracking_controller.rb index 88f739ce29e..b4b03e219ab 100644 --- a/app/controllers/projects/error_tracking_controller.rb +++ b/app/controllers/projects/error_tracking_controller.rb @@ -30,7 +30,7 @@ class Projects::ErrorTrackingController < Projects::ErrorTracking::BaseControlle service = ErrorTracking::IssueUpdateService.new(project, current_user, issue_update_params) result = service.execute - return if handle_errors(result) + return if render_errors(result) render json: { result: result @@ -47,7 +47,7 @@ class Projects::ErrorTrackingController < Projects::ErrorTracking::BaseControlle ) result = service.execute - return if handle_errors(result) + return if render_errors(result) render json: { errors: serialize_errors(result[:issues]), @@ -60,14 +60,14 @@ class Projects::ErrorTrackingController < Projects::ErrorTracking::BaseControlle service = ErrorTracking::IssueDetailsService.new(project, current_user, issue_details_params) result = service.execute - return if handle_errors(result) + return if render_errors(result) render json: { error: serialize_detailed_error(result[:issue]) } end - def handle_errors(result) + def render_errors(result) unless result[:status] == :success render json: { message: result[:message] }, status: result[:http_status] || :bad_request @@ -75,7 +75,7 @@ class Projects::ErrorTrackingController < Projects::ErrorTracking::BaseControlle end def list_issues_params - params.permit(:search_term, :sort, :cursor) + params.permit(:search_term, :sort, :cursor, :issue_status) end def issue_update_params diff --git a/app/models/error_tracking/project_error_tracking_setting.rb b/app/models/error_tracking/project_error_tracking_setting.rb index d73c177db26..5ee6522fb9d 100644 --- a/app/models/error_tracking/project_error_tracking_setting.rb +++ b/app/models/error_tracking/project_error_tracking_setting.rb @@ -89,7 +89,9 @@ module ErrorTracking end def list_sentry_projects - { projects: sentry_client.projects } + handle_exceptions do + { projects: sentry_client.projects } + end end def issue_details(opts = {}) diff --git a/app/services/error_tracking/base_service.rb b/app/services/error_tracking/base_service.rb index 4fe01716704..289c125b9d1 100644 --- a/app/services/error_tracking/base_service.rb +++ b/app/services/error_tracking/base_service.rb @@ -3,21 +3,9 @@ module ErrorTracking class BaseService < ::BaseService def execute - unauthorized = check_permissions return unauthorized if unauthorized - begin - response = perform - rescue Sentry::Client::Error => e - return error(e.message, :bad_request) - rescue Sentry::Client::MissingKeysError => e - return error(e.message, :internal_server_error) - end - - errors = parse_errors(response) - return errors if errors - - success(parse_response(response)) + perform end private @@ -27,12 +15,21 @@ module ErrorTracking "#{self.class} does not implement #{__method__}" end + def compose_response(response, &block) + errors = parse_errors(response) + return errors if errors + + yield if block_given? + + success(parse_response(response)) + end + def parse_response(response) raise NotImplementedError, "#{self.class} does not implement #{__method__}" end - def check_permissions + def unauthorized return error('Error Tracking is not enabled') unless enabled? return error('Access denied', :unauthorized) unless can_read? end diff --git a/app/services/error_tracking/issue_details_service.rb b/app/services/error_tracking/issue_details_service.rb index 31fb6a9618c..ee6d556518b 100644 --- a/app/services/error_tracking/issue_details_service.rb +++ b/app/services/error_tracking/issue_details_service.rb @@ -5,7 +5,9 @@ module ErrorTracking private def perform - project_error_tracking_setting.issue_details(issue_id: params[:issue_id]) + response = project_error_tracking_setting.issue_details(issue_id: params[:issue_id]) + + compose_response(response) end def parse_response(response) diff --git a/app/services/error_tracking/issue_latest_event_service.rb b/app/services/error_tracking/issue_latest_event_service.rb index dd6b7f8285f..a39f1cde1b2 100644 --- a/app/services/error_tracking/issue_latest_event_service.rb +++ b/app/services/error_tracking/issue_latest_event_service.rb @@ -5,7 +5,9 @@ module ErrorTracking private def perform - project_error_tracking_setting.issue_latest_event(issue_id: params[:issue_id]) + response = project_error_tracking_setting.issue_latest_event(issue_id: params[:issue_id]) + + compose_response(response) end def parse_response(response) diff --git a/app/services/error_tracking/issue_update_service.rb b/app/services/error_tracking/issue_update_service.rb index 8c9d52cefd9..904aed27684 100644 --- a/app/services/error_tracking/issue_update_service.rb +++ b/app/services/error_tracking/issue_update_service.rb @@ -7,21 +7,15 @@ module ErrorTracking private def perform - response = fetch + response = project_error_tracking_setting.update_issue( + issue_id: params[:issue_id], + params: update_params + ) - unless parse_errors(response).present? + compose_response(response) do response[:closed_issue_iid] = update_related_issue&.iid project_error_tracking_setting.expire_issues_cache end - - response - end - - def fetch - project_error_tracking_setting.update_issue( - issue_id: params[:issue_id], - params: update_params - ) end def update_related_issue @@ -74,7 +68,7 @@ module ErrorTracking } end - def check_permissions + def unauthorized return error('Error Tracking is not enabled') unless enabled? return error('Access denied', :unauthorized) unless can_update? end diff --git a/app/services/error_tracking/list_issues_service.rb b/app/services/error_tracking/list_issues_service.rb index d34ea8aa3b0..7087e3825d6 100644 --- a/app/services/error_tracking/list_issues_service.rb +++ b/app/services/error_tracking/list_issues_service.rb @@ -6,6 +6,13 @@ module ErrorTracking DEFAULT_LIMIT = 20 DEFAULT_SORT = 'last_seen' + # Sentry client supports 'muted' and 'assigned' but GitLab does not + ISSUE_STATUS_VALUES = %w[ + resolved + unresolved + ignored + ].freeze + def external_url project_error_tracking_setting&.sentry_external_url end @@ -13,19 +20,31 @@ module ErrorTracking private def perform - project_error_tracking_setting.list_sentry_issues( + return invalid_status_error unless valid_status? + + response = project_error_tracking_setting.list_sentry_issues( issue_status: issue_status, limit: limit, search_term: params[:search_term].presence, sort: sort, cursor: params[:cursor].presence ) + + compose_response(response) end def parse_response(response) response.slice(:issues, :pagination) end + def invalid_status_error + error('Bad Request: Invalid issue_status', http_status_for(:bad_Request)) + end + + def valid_status? + ISSUE_STATUS_VALUES.include?(issue_status) + end + def issue_status params[:issue_status] || DEFAULT_ISSUE_STATUS end diff --git a/app/services/error_tracking/list_projects_service.rb b/app/services/error_tracking/list_projects_service.rb index 6523a66bbed..625addaf915 100644 --- a/app/services/error_tracking/list_projects_service.rb +++ b/app/services/error_tracking/list_projects_service.rb @@ -2,18 +2,16 @@ module ErrorTracking class ListProjectsService < ErrorTracking::BaseService - def execute + private + + def perform unless project_error_tracking_setting.valid? return error(project_error_tracking_setting.errors.full_messages.join(', '), :bad_request) end - super - end + response = project_error_tracking_setting.list_sentry_projects - private - - def perform - project_error_tracking_setting.list_sentry_projects + compose_response(response) end def parse_response(response) diff --git a/app/views/search/results/_snippet_blob.html.haml b/app/views/search/results/_snippet_blob.html.haml index 0b114bf67ee..5126351b0bb 100644 --- a/app/views/search/results/_snippet_blob.html.haml +++ b/app/views/search/results/_snippet_blob.html.haml @@ -3,17 +3,22 @@ - snippet_chunks = snippet_blob[:snippet_chunks] - snippet_path = gitlab_snippet_path(snippet) -.search-result-row - %span - = snippet.title +.search-result-row.snippet-row + = image_tag avatar_icon_for_user(snippet.author), class: "avatar s40 d-none d-sm-block", alt: '' + .title + = link_to gitlab_snippet_path(snippet) do + = snippet.title + .snippet-info + = snippet.to_reference + · + authored + = time_ago_with_tooltip(snippet.created_at) by = link_to user_snippets_path(snippet.author) do - = image_tag avatar_icon_for_user(snippet.author), class: "avatar avatar-inline s16", alt: '' = snippet.author_name - %span.light= time_ago_with_tooltip(snippet.created_at) - %h4.snippet-title - .file-holder - .js-file-title.file-title + + .file-holder.my-2 + .js-file-title.file-title-flex-parent = link_to snippet_path do %i.fa.fa-file %strong= snippet.file_name diff --git a/changelogs/unreleased/199060-fix-snippet-search-results.yml b/changelogs/unreleased/199060-fix-snippet-search-results.yml new file mode 100644 index 00000000000..d69a03df768 --- /dev/null +++ b/changelogs/unreleased/199060-fix-snippet-search-results.yml @@ -0,0 +1,5 @@ +--- +title: Fix design of snippet search results page +merge_request: 23780 +author: +type: fixed diff --git a/changelogs/unreleased/21801-migrate-epic-and-epic-notes-mentions-to-epic-user-mentions-table.yml b/changelogs/unreleased/21801-migrate-epic-and-epic-notes-mentions-to-epic-user-mentions-table.yml deleted file mode 100644 index 168a8c7ac12..00000000000 --- a/changelogs/unreleased/21801-migrate-epic-and-epic-notes-mentions-to-epic-user-mentions-table.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Migrate epic, epic notes mentions to respective DB table -merge_request: 22333 -author: -type: changed diff --git a/changelogs/unreleased/34722-dashboard-projectscontroller-should-tolerate-single-gitaly-node-fai.yml b/changelogs/unreleased/34722-dashboard-projectscontroller-should-tolerate-single-gitaly-node-fai.yml new file mode 100644 index 00000000000..6e5256e27e5 --- /dev/null +++ b/changelogs/unreleased/34722-dashboard-projectscontroller-should-tolerate-single-gitaly-node-fai.yml @@ -0,0 +1,6 @@ +--- +title: Fix pipeline status loading errors on project dashboard page caused by Gitaly + connection failures +merge_request: 23378 +author: +type: fixed diff --git a/db/post_migrate/20191115115043_migrate_epic_mentions_to_db.rb b/db/post_migrate/20191115115043_migrate_epic_mentions_to_db.rb deleted file mode 100644 index 97f2e568a7e..00000000000 --- a/db/post_migrate/20191115115043_migrate_epic_mentions_to_db.rb +++ /dev/null @@ -1,36 +0,0 @@ -# frozen_string_literal: true - -class MigrateEpicMentionsToDb < ActiveRecord::Migration[5.2] - DOWNTIME = false - - disable_ddl_transaction! - - DELAY = 2.minutes.to_i - BATCH_SIZE = 10000 - MIGRATION = 'UserMentions::CreateResourceUserMention' - - JOIN = "LEFT JOIN epic_user_mentions on epics.id = epic_user_mentions.epic_id" - QUERY_CONDITIONS = "(description like '%@%' OR title like '%@%') AND epic_user_mentions.epic_id is null" - - class Epic < ActiveRecord::Base - include EachBatch - - self.table_name = 'epics' - end - - def up - return unless Gitlab.ee? - - Epic - .joins(JOIN) - .where(QUERY_CONDITIONS) - .each_batch(of: BATCH_SIZE) do |batch, index| - range = batch.pluck(Arel.sql('MIN(epics.id)'), Arel.sql('MAX(epics.id)')).first - BackgroundMigrationWorker.perform_in(index * DELAY, MIGRATION, ['Epic', JOIN, QUERY_CONDITIONS, false, *range]) - end - end - - def down - # no-op - end -end diff --git a/db/post_migrate/20191115115522_migrate_epic_notes_mentions_to_db.rb b/db/post_migrate/20191115115522_migrate_epic_notes_mentions_to_db.rb deleted file mode 100644 index e0b3c36b57d..00000000000 --- a/db/post_migrate/20191115115522_migrate_epic_notes_mentions_to_db.rb +++ /dev/null @@ -1,45 +0,0 @@ -# frozen_string_literal: true - -class MigrateEpicNotesMentionsToDb < ActiveRecord::Migration[5.2] - include Gitlab::Database::MigrationHelpers - - DOWNTIME = false - - disable_ddl_transaction! - - DELAY = 2.minutes.to_i - BATCH_SIZE = 10000 - MIGRATION = 'UserMentions::CreateResourceUserMention' - - INDEX_NAME = 'epic_mentions_temp_index' - INDEX_CONDITION = "note LIKE '%@%'::text AND notes.noteable_type = 'Epic'" - QUERY_CONDITIONS = "#{INDEX_CONDITION} AND epic_user_mentions.epic_id IS NULL" - JOIN = 'LEFT JOIN epic_user_mentions ON notes.id = epic_user_mentions.note_id' - - class Note < ActiveRecord::Base - include EachBatch - - self.table_name = 'notes' - end - - def up - return unless Gitlab.ee? - - # create temporary index for notes with mentions, may take well over 1h - add_concurrent_index(:notes, :id, where: INDEX_CONDITION, name: INDEX_NAME) - - Note - .joins(JOIN) - .where(QUERY_CONDITIONS) - .each_batch(of: BATCH_SIZE) do |batch, index| - range = batch.pluck(Arel.sql('MIN(notes.id)'), Arel.sql('MAX(notes.id)')).first - BackgroundMigrationWorker.perform_in(index * DELAY, MIGRATION, ['Epic', JOIN, QUERY_CONDITIONS, true, *range]) - end - end - - def down - # no-op - # temporary index is to be dropped in a different migration in an upcoming release: - # https://gitlab.com/gitlab-org/gitlab/issues/196842 - end -end diff --git a/db/schema.rb b/db/schema.rb index f48ead215bc..3248768aa0b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -2778,7 +2778,6 @@ ActiveRecord::Schema.define(version: 2020_01_27_090233) do t.index ["commit_id"], name: "index_notes_on_commit_id" t.index ["created_at"], name: "index_notes_on_created_at" t.index ["discussion_id"], name: "index_notes_on_discussion_id" - t.index ["id"], name: "epic_mentions_temp_index", where: "((note ~~ '%@%'::text) AND ((noteable_type)::text = 'Epic'::text))" t.index ["line_code"], name: "index_notes_on_line_code" t.index ["note"], name: "index_notes_on_note_trigram", opclass: :gin_trgm_ops, using: :gin t.index ["noteable_id", "noteable_type"], name: "index_notes_on_noteable_id_and_noteable_type" diff --git a/doc/development/README.md b/doc/development/README.md index c30b7cbc558..5338db38430 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -9,20 +9,29 @@ description: 'Learn how to contribute to GitLab.' - Set up GitLab's development environment with [GitLab Development Kit (GDK)](https://gitlab.com/gitlab-org/gitlab-development-kit/blob/master/doc/howto/README.md) - [GitLab contributing guide](contributing/index.md) -- [Architecture](architecture.md) of GitLab + - [Issues workflow](contributing/issue_workflow.md) (issue tracker guidelines, triaging, labels, feature proposals, issue weight, regression issues, technical and UX debt) + - [Merge requests workflow](contributing/merge_request_workflow.md) (merge request guidelines, contribution acceptance criteria, definition of done, dependencies) + - [Style guides](contributing/style_guides.md) + - [Implement design & UI elements](contributing/design.md) +- [GitLab Architecture Overview](architecture.md) - [Rake tasks](rake_tasks.md) for development ## Processes -- [GitLab core team & GitLab Inc. contribution process](https://gitlab.com/gitlab-org/gitlab/blob/master/PROCESS.md) -- [Generate a changelog entry with `bin/changelog`](changelog.md) +**Must-reads:** + - [Code review guidelines](code_review.md) for reviewing code and having code reviewed - [Database review guidelines](database_review.md) for reviewing database-related changes and complex SQL queries, and having them reviewed - [Pipelines for the GitLab project](pipelines.md) -- [Guidelines for implementing Enterprise Edition features](ee_features.md) + +Complementary reads: + +- [GitLab core team & GitLab Inc. contribution process](https://gitlab.com/gitlab-org/gitlab/blob/master/PROCESS.md) - [Security process for developers](https://gitlab.com/gitlab-org/release/docs/blob/master/general/security/developer.md#security-releases-critical-non-critical-as-a-developer) -- [Requesting access to Chatops on GitLab.com](chatops_on_gitlabcom.md#requesting-access) (for GitLabbers) +- [Guidelines for implementing Enterprise Edition features](ee_features.md) - [Danger bot](dangerbot.md) +- [Generate a changelog entry with `bin/changelog`](changelog.md) +- [Requesting access to Chatops on GitLab.com](chatops_on_gitlabcom.md#requesting-access) (for GitLabbers) ## UX and Frontend guides diff --git a/doc/development/contributing/design.md b/doc/development/contributing/design.md index 9796e195f86..8426db84aa4 100644 --- a/doc/development/contributing/design.md +++ b/doc/development/contributing/design.md @@ -4,7 +4,7 @@ For guidance on UX implementation at GitLab, please refer to our [Design System] The UX team uses labels to manage their workflow. -The ~"UX" label on an issue is a signal to the UX team that it will need UX attention. +The ~"UX" label on an issue is a signal to the UX team that it will need UX attention. To better understand the priority by which UX tackles issues, see the [UX section](https://about.gitlab.com/handbook/engineering/ux/) of the handbook. Once an issue has been worked on and is ready for development, a UXer removes the ~"UX" label and applies the ~"UX ready" label to that issue. diff --git a/doc/development/contributing/index.md b/doc/development/contributing/index.md index 481a18aac3d..233bf255361 100644 --- a/doc/development/contributing/index.md +++ b/doc/development/contributing/index.md @@ -118,6 +118,11 @@ This [documentation](merge_request_workflow.md) outlines the current merge reque This [documentation](style_guides.md) outlines the current style guidelines. +## Implement design & UI elements + +This [design documentation](design.md) outlines the current process for implementing +design & UI elements. + ## Getting an Enterprise Edition License If you need a license for contributing to an EE-feature, please [follow these instructions](https://about.gitlab.com/handbook/marketing/community-relations/code-contributor-program/#for-contributors-to-the-gitlab-enterprise-edition-ee). diff --git a/doc/development/contributing/style_guides.md b/doc/development/contributing/style_guides.md index 33c872cbb93..f35757249fc 100644 --- a/doc/development/contributing/style_guides.md +++ b/doc/development/contributing/style_guides.md @@ -1,49 +1,86 @@ # Style guides -1. [Ruby](https://github.com/rubocop-hq/ruby-style-guide). - Important sections include [Source Code Layout][rss-source] and - [Naming][rss-naming]. Use: - - multi-line method chaining style **Option A**: dot `.` on the second line - - string literal quoting style **Option A**: single quoted by default -1. [Rails](https://github.com/rubocop-hq/rails-style-guide) -1. [Newlines styleguide][newlines-styleguide] -1. [Testing][testing] -1. [JavaScript styleguide][js-styleguide] -1. [SCSS styleguide][scss-styleguide] -1. [Shell commands (Ruby)](../shell_commands.md) created by GitLab - contributors to enhance security -1. [Database Migrations](../migration_style_guide.md) -1. [Markdown](https://cirosantilli.com/markdown-style-guide/) -1. [Documentation styleguide](../documentation/styleguide.md) -1. Interface text should be written subjectively instead of objectively. It - should be the GitLab core team addressing a person. It should be written in - present time and never use past tense (has been/was). For example instead - of _prohibited this user from being saved due to the following errors:_ the - text should be _sorry, we could not create your account because:_ -1. Code should be written in [US English][us-english] -1. [Go](../go_guide/index.md) -1. [Python](../python_guide/index.md) -1. [Shell scripting](../shell_scripting_guide/index.md) - -## Checking the style and other issues - -This is also the style used by linting tools such as -[RuboCop](https://github.com/rubocop-hq/rubocop) and [Hound CI](https://houndci.com). -You can run RuboCop by hand or install a tool like [Overcommit](https://github.com/sds/overcommit) to run it for you. - -Overcommit will automatically run the configured checks (like Rubocop) on every modified file before commit. -You can use the example overcommit configuration found in `.overcommit.yml.example` as a quickstart. -This saves you time as you don't have to wait for the same errors to be detected by the CI. +## Pre-commit static analysis + +You're strongly advised to install +[Overcommit](https://github.com/sds/overcommit) to automatically check for +static analysis offenses before committing locally: + +```shell +cd tooling/overcommit && make && cd - +``` + +Then before a commit is created, Overcommit will automatically check for +RuboCop (and other checks) offenses on every modified file. + +This saves you time as you don't have to wait for the same errors to be detected +by the CI. + +## Ruby, Rails, RSpec + +Our codebase style is defined and enforced by [RuboCop](https://github.com/rubocop-hq/rubocop). + +You can check for any offenses locally with `bundle exec rubocop --parallel`. +On the CI, this is automatically checked by the `static-analysis` jobs. + +For RuboCop rules that we have not taken a decision yet, we follow the +[Ruby Style Guide](https://github.com/rubocop-hq/ruby-style-guide), +[Rails Style Guide](https://github.com/rubocop-hq/rails-style-guide), and +[RSpec Style Guide](https://github.com/rubocop-hq/rspec-style-guide) as general +guidelines to write idiomatic Ruby/Rails/RSpec, but reviewers/maintainers should +be tolerant and not too pedantic about style. + +Similarly, some RuboCop rules are currently disabled, and for those, +reviewers/maintainers must not ask authors to use one style or the other, as both +are accepted. This isn't an ideal situation since this leaves space for +[bike-shedding](https://en.wiktionary.org/wiki/bikeshedding), and ideally we +should enable all RuboCop rules to avoid style-related +discussions/nitpicking/back-and-forth in reviews. + +Additionally, we have a dedicated +[newlines styleguide](../newlines_styleguide.md), as well as dedicated +[test-specific style guides and best practices](../testing_guide/index.md). + +## Database migrations + +See the dedicated [Database Migrations Style Guide](../migration_style_guide.md). + +## JavaScript + +See the dedicated [JS Style Guide](../fe_guide/style/javascript.md). + +## SCSS + +See the dedicated [SCSS Style Guide](../fe_guide/style/scss.md). + +## Go + +See the dedicated [Go standards and style guidelines](../go_guide/index.md). + +## Shell commands (Ruby) + +See the dedicated [Guidelines for shell commands in the GitLab codebase](../shell_commands.md). + +## Shell scripting + +See the dedicated [Shell scripting standards and style guidelines](../shell_scripting_guide/index.md). + +## Markdown + +We're following [Ciro Santilli's Markdown Style Guide](https://cirosantilli.com/markdown-style-guide). + +## Documentation + +See the dedicated [Documentation Style Guide](../documentation/styleguide.md). + +## Python + +See the dedicated [Python Development Guidelines](../python_guide/index.md). + +## Misc + +Code should be written in [US English](https://en.wikipedia.org/wiki/American_English). --- [Return to Contributing documentation](index.md) - -[rss-source]: https://github.com/rubocop-hq/ruby-style-guide/blob/master/README.adoc#source-code-layout -[rss-naming]: https://github.com/rubocop-hq/ruby-style-guide/blob/master/README.adoc#naming-conventions -[doc-guidelines]: ../documentation/index.md "Documentation guidelines" -[js-styleguide]: ../fe_guide/style/javascript.md "JavaScript styleguide" -[scss-styleguide]: ../fe_guide/style/scss.md "SCSS styleguide" -[newlines-styleguide]: ../newlines_styleguide.md "Newlines styleguide" -[testing]: ../testing_guide/index.md -[us-english]: https://en.wikipedia.org/wiki/American_English diff --git a/doc/development/dangerbot.md b/doc/development/dangerbot.md index 51f7a18dd08..b30ac0f0748 100644 --- a/doc/development/dangerbot.md +++ b/doc/development/dangerbot.md @@ -4,7 +4,7 @@ The GitLab CI pipeline includes a `danger-review` job that uses [Danger](https:/ to perform a variety of automated checks on the code under test. Danger is a gem that runs in the CI environment, like any other analysis tool. -What sets it apart from, e.g., Rubocop, is that it's designed to allow you to +What sets it apart from, e.g., RuboCop, is that it's designed to allow you to easily write arbitrary code to test properties of your code or changes. To this end, it provides a set of common helpers and access to information about what has actually changed in your environment, then simply runs your code! @@ -13,6 +13,14 @@ If Danger is asking you to change something about your merge request, it's best just to make the change. If you want to learn how Danger works, or make changes to the existing rules, then this is the document for you. +## Run Danger locally + +A subset of the current checks can be run locally with the following rake task: + +```shell +bundle exec danger_local +``` + ## Operation On startup, Danger reads a [`Dangerfile`](https://gitlab.com/gitlab-org/gitlab/blob/master/Dangerfile) @@ -118,7 +126,6 @@ at GitLab so far: ## Limitations -- [`danger local` does not work on GitLab](https://github.com/danger/danger/issues/458) - Danger output is not added to a merge request comment if working on a fork. This happens because the secret variable from the canonical project is not shared to forks. diff --git a/lib/gitlab/background_migration/user_mentions/create_resource_user_mention.rb b/lib/gitlab/background_migration/user_mentions/create_resource_user_mention.rb deleted file mode 100644 index e951b44b036..00000000000 --- a/lib/gitlab/background_migration/user_mentions/create_resource_user_mention.rb +++ /dev/null @@ -1,42 +0,0 @@ -# frozen_string_literal: true -# rubocop:disable Style/Documentation - -module Gitlab - module BackgroundMigration - module UserMentions - class CreateResourceUserMention - # Resources that have mentions to be migrated: - # issue, merge_request, epic, commit, snippet, design - - BULK_INSERT_SIZE = 5000 - ISOLATION_MODULE = 'Gitlab::BackgroundMigration::UserMentions::Models' - - def perform(resource_model, join, conditions, with_notes, start_id, end_id) - resource_model = "#{ISOLATION_MODULE}::#{resource_model}".constantize if resource_model.is_a?(String) - model = with_notes ? "#{ISOLATION_MODULE}::Note".constantize : resource_model - resource_user_mention_model = resource_model.user_mention_model - - records = model.joins(join).where(conditions).where(id: start_id..end_id) - - records.in_groups_of(BULK_INSERT_SIZE, false).each do |records| - mentions = [] - records.each do |record| - mentions << record.build_mention_values - end - - no_quote_columns = [:note_id] - no_quote_columns << resource_user_mention_model.resource_foreign_key - - Gitlab::Database.bulk_insert( - resource_user_mention_model.table_name, - mentions, - return_ids: true, - disable_quote: no_quote_columns, - on_conflict: :do_nothing - ) - end - end - end - end - end -end diff --git a/lib/gitlab/background_migration/user_mentions/models/epic.rb b/lib/gitlab/background_migration/user_mentions/models/epic.rb deleted file mode 100644 index 019d8f0ea8b..00000000000 --- a/lib/gitlab/background_migration/user_mentions/models/epic.rb +++ /dev/null @@ -1,50 +0,0 @@ -# frozen_string_literal: true -# rubocop:disable Style/Documentation - -module Gitlab - module BackgroundMigration - module UserMentions - module Models - class Epic < ActiveRecord::Base - include IsolatedMentionable - include CacheMarkdownField - - attr_mentionable :title, pipeline: :single_line - attr_mentionable :description - cache_markdown_field :title, pipeline: :single_line - cache_markdown_field :description, issuable_state_filter_enabled: true - - self.table_name = 'epics' - - belongs_to :author, class_name: "User" - belongs_to :project - belongs_to :group - - def self.user_mention_model - Gitlab::BackgroundMigration::UserMentions::Models::EpicUserMention - end - - def user_mention_model - self.class.user_mention_model - end - - def project - nil - end - - def mentionable_params - { group: group, label_url_method: :group_epics_url } - end - - def user_mention_resource_id - id - end - - def user_mention_note_id - 'NULL' - end - end - end - end - end -end diff --git a/lib/gitlab/background_migration/user_mentions/models/epic_user_mention.rb b/lib/gitlab/background_migration/user_mentions/models/epic_user_mention.rb deleted file mode 100644 index 4e3ce9bf3a7..00000000000 --- a/lib/gitlab/background_migration/user_mentions/models/epic_user_mention.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true -# rubocop:disable Style/Documentation - -module Gitlab - module BackgroundMigration - module UserMentions - module Models - class EpicUserMention < ActiveRecord::Base - self.table_name = 'epic_user_mentions' - - def self.resource_foreign_key - :epic_id - end - end - end - end - end -end diff --git a/lib/gitlab/background_migration/user_mentions/models/isolated_mentionable.rb b/lib/gitlab/background_migration/user_mentions/models/isolated_mentionable.rb deleted file mode 100644 index 40aab896212..00000000000 --- a/lib/gitlab/background_migration/user_mentions/models/isolated_mentionable.rb +++ /dev/null @@ -1,95 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module BackgroundMigration - module UserMentions - module Models - # == IsolatedMentionable concern - # - # Shortcutted for isolation version of Mentionable to be used in mentions migrations - # - module IsolatedMentionable - extend ::ActiveSupport::Concern - - class_methods do - # Indicate which attributes of the Mentionable to search for GFM references. - def attr_mentionable(attr, options = {}) - attr = attr.to_s - mentionable_attrs << [attr, options] - end - end - - included do - # Accessor for attributes marked mentionable. - cattr_accessor :mentionable_attrs, instance_accessor: false do - [] - end - - if self < Participable - participant -> (user, ext) { all_references(user, extractor: ext) } - end - end - - def all_references(current_user = nil, extractor: nil) - # Use custom extractor if it's passed in the function parameters. - if extractor - extractors[current_user] = extractor - else - extractor = extractors[current_user] ||= ::Gitlab::ReferenceExtractor.new(project, current_user) - - extractor.reset_memoized_values - end - - self.class.mentionable_attrs.each do |attr, options| - text = __send__(attr) # rubocop:disable GitlabSecurity/PublicSend - options = options.merge( - cache_key: [self, attr], - author: author, - skip_project_check: skip_project_check? - ).merge(mentionable_params) - - cached_html = self.try(:updated_cached_html_for, attr.to_sym) - options[:rendered] = cached_html if cached_html - - extractor.analyze(text, options) - end - - extractor - end - - def extractors - @extractors ||= {} - end - - def skip_project_check? - false - end - - def build_mention_values - refs = all_references(author) - - { - "#{self.user_mention_model.resource_foreign_key}": user_mention_resource_id, - note_id: user_mention_note_id, - mentioned_users_ids: array_to_sql(refs.mentioned_users.pluck(:id)), - mentioned_projects_ids: array_to_sql(refs.mentioned_projects.pluck(:id)), - mentioned_groups_ids: array_to_sql(refs.mentioned_groups.pluck(:id)) - } - end - - def array_to_sql(ids_array) - return unless ids_array.present? - - '{' + ids_array.join(", ") + '}' - end - - private - - def mentionable_params - {} - end - end - end - end - end -end diff --git a/lib/gitlab/background_migration/user_mentions/models/note.rb b/lib/gitlab/background_migration/user_mentions/models/note.rb deleted file mode 100644 index c2828202907..00000000000 --- a/lib/gitlab/background_migration/user_mentions/models/note.rb +++ /dev/null @@ -1,65 +0,0 @@ -# frozen_string_literal: true -# rubocop:disable Style/Documentation - -module Gitlab - module BackgroundMigration - module UserMentions - module Models - class Note < ActiveRecord::Base - include IsolatedMentionable - include CacheMarkdownField - - self.table_name = 'notes' - self.inheritance_column = :_type_disabled - - attr_mentionable :note, pipeline: :note - cache_markdown_field :note, pipeline: :note, issuable_state_filter_enabled: true - - belongs_to :author, class_name: "User" - belongs_to :noteable, polymorphic: true - belongs_to :project - - def user_mention_model - "#{CreateResourceUserMention::ISOLATION_MODULE}::#{noteable.class}".constantize.user_mention_model - end - - def for_personal_snippet? - noteable.class.name == 'PersonalSnippet' - end - - def for_project_noteable? - !for_personal_snippet? - end - - def skip_project_check? - !for_project_noteable? - end - - def for_epic? - noteable.class.name == 'Epic' - end - - def user_mention_resource_id - noteable_id || commit_id - end - - def user_mention_note_id - id - end - - private - - def mentionable_params - return super unless for_epic? - - super.merge(banzai_context_params) - end - - def banzai_context_params - { group: noteable.group, label_url_method: :group_epics_url } - end - end - end - end - end -end diff --git a/lib/gitlab/cache/ci/project_pipeline_status.rb b/lib/gitlab/cache/ci/project_pipeline_status.rb index ea7013db2ce..e7a7d23ef7e 100644 --- a/lib/gitlab/cache/ci/project_pipeline_status.rb +++ b/lib/gitlab/cache/ci/project_pipeline_status.rb @@ -59,6 +59,10 @@ module Gitlab end self.loaded = true + rescue GRPC::Unavailable, GRPC::DeadlineExceeded => e + # Handle Gitaly connection issues gracefully + Gitlab::ErrorTracking + .track_exception(e, project_id: project.id) end def load_from_project diff --git a/spec/frontend/ide/stores/actions/file_spec.js b/spec/frontend/ide/stores/actions/file_spec.js index a180dd5edc1..dd729651a61 100644 --- a/spec/frontend/ide/stores/actions/file_spec.js +++ b/spec/frontend/ide/stores/actions/file_spec.js @@ -251,7 +251,7 @@ describe('IDE store file actions', () => { describe('success', () => { beforeEach(() => { - mock.onGet(`${RELATIVE_URL_ROOT}/test/test/7297abc/${localFile.path}`).replyOnce( + mock.onGet(`${RELATIVE_URL_ROOT}/test/test/-/7297abc/${localFile.path}`).replyOnce( 200, { blame_path: 'blame_path', @@ -273,7 +273,7 @@ describe('IDE store file actions', () => { .dispatch('getFileData', { path: localFile.path }) .then(() => { expect(service.getFileData).toHaveBeenCalledWith( - `${RELATIVE_URL_ROOT}/test/test/7297abc/${localFile.path}`, + `${RELATIVE_URL_ROOT}/test/test/-/7297abc/${localFile.path}`, ); done(); @@ -345,7 +345,7 @@ describe('IDE store file actions', () => { localFile.path = 'new-shiny-file'; store.state.entries[localFile.path] = localFile; - mock.onGet(`${RELATIVE_URL_ROOT}/test/test/7297abc/old-dull-file`).replyOnce( + mock.onGet(`${RELATIVE_URL_ROOT}/test/test/-/7297abc/old-dull-file`).replyOnce( 200, { blame_path: 'blame_path', @@ -376,7 +376,7 @@ describe('IDE store file actions', () => { describe('error', () => { beforeEach(() => { - mock.onGet(`${RELATIVE_URL_ROOT}/test/test/7297abc/${localFile.path}`).networkError(); + mock.onGet(`${RELATIVE_URL_ROOT}/test/test/-/7297abc/${localFile.path}`).networkError(); }); it('dispatches error action', () => { diff --git a/spec/frontend/repository/log_tree_spec.js b/spec/frontend/repository/log_tree_spec.js index 4271a038680..8da2f39f71f 100644 --- a/spec/frontend/repository/log_tree_spec.js +++ b/spec/frontend/repository/log_tree_spec.js @@ -71,7 +71,7 @@ describe('fetchLogsTree', () => { it('calls axios get', () => fetchLogsTree(client, '', '0', resolver).then(() => { - expect(axios.get).toHaveBeenCalledWith('/gitlab-org/gitlab-foss/refs/master/logs_tree/', { + expect(axios.get).toHaveBeenCalledWith('/gitlab-org/gitlab-foss/-/refs/master/logs_tree/', { params: { format: 'json', offset: '0' }, }); })); diff --git a/spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb b/spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb index c2816f35cec..fc9266f75fb 100644 --- a/spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb +++ b/spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb @@ -114,6 +114,24 @@ describe Gitlab::Cache::Ci::ProjectPipelineStatus, :clean_gitlab_redis_cache do pipeline_status.load_status pipeline_status.load_status end + + it 'handles Gitaly unavailable exceptions gracefully' do + allow(pipeline_status).to receive(:commit).and_raise(GRPC::Unavailable) + + expect(Gitlab::ErrorTracking).to receive(:track_exception).with( + an_instance_of(GRPC::Unavailable), project_id: project.id + ) + expect { pipeline_status.load_status }.not_to raise_error + end + + it 'handles Gitaly timeout exceptions gracefully' do + allow(pipeline_status).to receive(:commit).and_raise(GRPC::DeadlineExceeded) + + expect(Gitlab::ErrorTracking).to receive(:track_exception).with( + an_instance_of(GRPC::DeadlineExceeded), project_id: project.id + ) + expect { pipeline_status.load_status }.not_to raise_error + end end describe "#load_from_project", :clean_gitlab_redis_cache do diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index a46381fcedf..ef26676a6e4 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -190,23 +190,23 @@ describe 'project routing' do end it 'to #archive_alternative' do - expect(get('/gitlab/gitlabhq/repository/archive')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', append_sha: true) + expect(get('/gitlab/gitlabhq/-/repository/archive')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', append_sha: true) end it 'to #archive_deprecated' do - expect(get('/gitlab/gitlabhq/repository/master/archive')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master', append_sha: true) + expect(get('/gitlab/gitlabhq/-/repository/master/archive')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master', append_sha: true) end it 'to #archive_deprecated format:zip' do - expect(get('/gitlab/gitlabhq/repository/master/archive.zip')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', format: 'zip', id: 'master', append_sha: true) + expect(get('/gitlab/gitlabhq/-/repository/master/archive.zip')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', format: 'zip', id: 'master', append_sha: true) end it 'to #archive_deprecated format:tar.bz2' do - expect(get('/gitlab/gitlabhq/repository/master/archive.tar.bz2')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', format: 'tar.bz2', id: 'master', append_sha: true) + expect(get('/gitlab/gitlabhq/-/repository/master/archive.tar.bz2')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', format: 'tar.bz2', id: 'master', append_sha: true) end it 'to #archive_deprecated with "/" in route' do - expect(get('/gitlab/gitlabhq/repository/improve/awesome/archive')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'improve/awesome', append_sha: true) + expect(get('/gitlab/gitlabhq/-/repository/improve/awesome/archive')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'improve/awesome', append_sha: true) end end @@ -269,7 +269,7 @@ describe 'project routing' do # logs_file_project_ref GET /:project_id/refs/:id/logs_tree/:path(.:format) refs#logs_tree describe Projects::RefsController, 'routing' do it 'to #switch' do - expect(get('/gitlab/gitlabhq/refs/switch')).to route_to('projects/refs#switch', namespace_id: 'gitlab', project_id: 'gitlabhq') + expect(get('/gitlab/gitlabhq/-/refs/switch')).to route_to('projects/refs#switch', namespace_id: 'gitlab', project_id: 'gitlabhq') end it 'to #logs_tree' do @@ -695,16 +695,16 @@ describe 'project routing' do # project_compare /:project_id/compare/:from...:to(.:format) compare#show {from: /.+/, to: /.+/, id: /[^\/]+/, project_id: /[^\/]+/} describe Projects::CompareController, 'routing' do it 'to #index' do - expect(get('/gitlab/gitlabhq/compare')).to route_to('projects/compare#index', namespace_id: 'gitlab', project_id: 'gitlabhq') + expect(get('/gitlab/gitlabhq/-/compare')).to route_to('projects/compare#index', namespace_id: 'gitlab', project_id: 'gitlabhq') end it 'to #compare' do - expect(post('/gitlab/gitlabhq/compare')).to route_to('projects/compare#create', namespace_id: 'gitlab', project_id: 'gitlabhq') + expect(post('/gitlab/gitlabhq/-/compare')).to route_to('projects/compare#create', namespace_id: 'gitlab', project_id: 'gitlabhq') end it 'to #show' do - expect(get('/gitlab/gitlabhq/compare/master...stable')).to route_to('projects/compare#show', namespace_id: 'gitlab', project_id: 'gitlabhq', from: 'master', to: 'stable') - expect(get('/gitlab/gitlabhq/compare/issue/1234...stable')).to route_to('projects/compare#show', namespace_id: 'gitlab', project_id: 'gitlabhq', from: 'issue/1234', to: 'stable') + expect(get('/gitlab/gitlabhq/-/compare/master...stable')).to route_to('projects/compare#show', namespace_id: 'gitlab', project_id: 'gitlabhq', from: 'master', to: 'stable') + expect(get('/gitlab/gitlabhq/-/compare/issue/1234...stable')).to route_to('projects/compare#show', namespace_id: 'gitlab', project_id: 'gitlabhq', from: 'issue/1234', to: 'stable') end end diff --git a/spec/services/error_tracking/base_service_spec.rb b/spec/services/error_tracking/base_service_spec.rb new file mode 100644 index 00000000000..68deb2e2a73 --- /dev/null +++ b/spec/services/error_tracking/base_service_spec.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ErrorTracking::BaseService do + describe '#compose_response' do + let(:project) { double('project') } + let(:user) { double('user') } + let(:service) { described_class.new(project, user) } + + it 'returns bad_request error when response has an error key' do + data = { error: 'Unexpected Error' } + + result = service.send(:compose_response, data) + + expect(result[:status]).to be(:error) + expect(result[:message]).to be('Unexpected Error') + expect(result[:http_status]).to be(:bad_request) + end + + it 'returns server error when response has missing key error_type' do + data = { error: 'Unexpected Error', error_type: ErrorTracking::ProjectErrorTrackingSetting::SENTRY_API_ERROR_TYPE_MISSING_KEYS } + + result = service.send(:compose_response, data) + + expect(result[:status]).to be(:error) + expect(result[:message]).to be('Unexpected Error') + expect(result[:http_status]).to be(:internal_server_error) + end + + it 'returns no content when response is nil' do + data = nil + + result = service.send(:compose_response, data) + + expect(result[:status]).to be(:error) + expect(result[:message]).to be('Not ready. Try again later') + expect(result[:http_status]).to be(:no_content) + end + + context 'when result has no errors key' do + let(:data) { { thing: :cat } } + + it 'raises NotImplementedError' do + expect { service.send(:compose_response, data) } + .to raise_error(NotImplementedError) + end + + context 'when parse_response is implemented' do + before do + expect(service).to receive(:parse_response) do |response| + { animal: response[:thing] } + end + end + + it 'returns successful response' do + result = service.send(:compose_response, data) + + expect(result[:animal]).to eq(:cat) + expect(result[:status]).to eq(:success) + end + + it 'returns successful response with changes from passed block' do + result = service.send(:compose_response, data) do + data[:thing] = :fish + end + + expect(result[:animal]).to eq(:fish) + expect(result[:status]).to eq(:success) + end + end + end + end +end diff --git a/spec/services/error_tracking/list_issues_service_spec.rb b/spec/services/error_tracking/list_issues_service_spec.rb index 1e39146fd18..5f6e071e10d 100644 --- a/spec/services/error_tracking/list_issues_service_spec.rb +++ b/spec/services/error_tracking/list_issues_service_spec.rb @@ -20,21 +20,31 @@ describe ErrorTracking::ListIssuesService do describe '#execute' do context 'with authorized user' do - context 'when list_sentry_issues returns issues' do - let(:issues) { [:list, :of, :issues] } - - before do - expect(error_tracking_setting) - .to receive(:list_sentry_issues) - .with(list_sentry_issues_args) - .and_return(issues: issues, pagination: {}) - end + let(:issues) { [] } + + described_class::ISSUE_STATUS_VALUES.each do |status| + it "returns the issues with #{status} issue_status" do + params[:issue_status] = status + list_sentry_issues_args[:issue_status] = status + expect_list_sentry_issues_with(list_sentry_issues_args) - it 'returns the issues' do expect(result).to eq(status: :success, pagination: {}, issues: issues) end end + it 'returns the issues with no issue_status' do + expect_list_sentry_issues_with(list_sentry_issues_args) + + expect(result).to eq(status: :success, pagination: {}, issues: issues) + end + + it 'returns bad request for an issue_status not on the whitelist' do + params[:issue_status] = 'assigned' + + expect(error_tracking_setting).not_to receive(:list_sentry_issues) + expect(result).to eq(message: "Bad Request: Invalid issue_status", status: :error, http_status: :bad_request) + end + include_examples 'error tracking service data not ready', :list_sentry_issues include_examples 'error tracking service sentry error handling', :list_sentry_issues include_examples 'error tracking service http status handling', :list_sentry_issues @@ -52,3 +62,10 @@ describe ErrorTracking::ListIssuesService do end end end + +def expect_list_sentry_issues_with(list_sentry_issues_args) + expect(error_tracking_setting) + .to receive(:list_sentry_issues) + .with(list_sentry_issues_args) + .and_return(issues: [], pagination: {}) +end diff --git a/spec/services/error_tracking/list_projects_service_spec.rb b/spec/services/error_tracking/list_projects_service_spec.rb index ddd369d45f2..565610c64ac 100644 --- a/spec/services/error_tracking/list_projects_service_spec.rb +++ b/spec/services/error_tracking/list_projects_service_spec.rb @@ -63,32 +63,6 @@ describe ErrorTracking::ListProjectsService do end end - context 'sentry client raises exception' do - context 'Sentry::Client::Error' do - before do - expect(error_tracking_setting).to receive(:list_sentry_projects) - .and_raise(Sentry::Client::Error, 'Sentry response status code: 500') - end - - it 'returns error response' do - expect(result[:message]).to eq('Sentry response status code: 500') - expect(result[:http_status]).to eq(:bad_request) - end - end - - context 'Sentry::Client::MissingKeysError' do - before do - expect(error_tracking_setting).to receive(:list_sentry_projects) - .and_raise(Sentry::Client::MissingKeysError, 'Sentry API response is missing keys. key not found: "id"') - end - - it 'returns error response' do - expect(result[:message]).to eq('Sentry API response is missing keys. key not found: "id"') - expect(result[:http_status]).to eq(:internal_server_error) - end - end - end - context 'with invalid url' do let(:params) do ActionController::Parameters.new( diff --git a/spec/support/shared_examples/models/mentionable_shared_examples.rb b/spec/support/shared_examples/models/mentionable_shared_examples.rb index 0e8ee6f66f5..0c55e9de045 100644 --- a/spec/support/shared_examples/models/mentionable_shared_examples.rb +++ b/spec/support/shared_examples/models/mentionable_shared_examples.rb @@ -229,17 +229,16 @@ RSpec.shared_examples 'mentions in description' do |mentionable_type| context 'when mentionable description contains mentions' do let(:user) { create(:user) } - let(:user2) { create(:user) } let(:group) { create(:group) } - let(:mentionable_desc) { "#{user.to_reference} #{user2.to_reference} #{user.to_reference} some description #{group.to_reference(full: true)} and #{user2.to_reference} @all" } + let(:mentionable_desc) { "#{user.to_reference} some description #{group.to_reference(full: true)} and @all" } let(:mentionable) { create(mentionable_type, description: mentionable_desc) } it 'stores mentions' do add_member(user) expect(mentionable.user_mentions.count).to eq 1 - expect(mentionable.referenced_users).to match_array([user, user2]) + expect(mentionable.referenced_users).to match_array([user]) expect(mentionable.referenced_projects(user)).to match_array([mentionable.project].compact) # epic.project is nil, and we want empty [] expect(mentionable.referenced_groups(user)).to match_array([group]) end @@ -250,9 +249,8 @@ end RSpec.shared_examples 'mentions in notes' do |mentionable_type| context 'when mentionable notes contain mentions' do let(:user) { create(:user) } - let(:user2) { create(:user) } let(:group) { create(:group) } - let(:note_desc) { "#{user.to_reference} #{user2.to_reference} #{user.to_reference} and #{group.to_reference(full: true)} and #{user2.to_reference} @all" } + let(:note_desc) { "#{user.to_reference} and #{group.to_reference(full: true)} and @all" } let!(:mentionable) { note.noteable } before do @@ -263,7 +261,7 @@ RSpec.shared_examples 'mentions in notes' do |mentionable_type| it 'returns all mentionable mentions' do expect(mentionable.user_mentions.count).to eq 1 - expect(mentionable.referenced_users).to eq [user, user2] + expect(mentionable.referenced_users).to eq [user] expect(mentionable.referenced_projects(user)).to eq [mentionable.project].compact # epic.project is nil, and we want empty [] expect(mentionable.referenced_groups(user)).to eq [group] end |