summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/assets/javascripts/ide/stores/actions/file.js1
-rw-r--r--app/assets/javascripts/repository/log_tree.js2
-rw-r--r--app/assets/stylesheets/framework/snippets.scss5
-rw-r--r--app/controllers/projects/error_tracking_controller.rb10
-rw-r--r--app/models/error_tracking/project_error_tracking_setting.rb4
-rw-r--r--app/services/error_tracking/base_service.rb25
-rw-r--r--app/services/error_tracking/issue_details_service.rb4
-rw-r--r--app/services/error_tracking/issue_latest_event_service.rb4
-rw-r--r--app/services/error_tracking/issue_update_service.rb18
-rw-r--r--app/services/error_tracking/list_issues_service.rb21
-rw-r--r--app/services/error_tracking/list_projects_service.rb12
-rw-r--r--app/views/search/results/_snippet_blob.html.haml21
-rw-r--r--changelogs/unreleased/199060-fix-snippet-search-results.yml5
-rw-r--r--changelogs/unreleased/21801-migrate-epic-and-epic-notes-mentions-to-epic-user-mentions-table.yml5
-rw-r--r--changelogs/unreleased/34722-dashboard-projectscontroller-should-tolerate-single-gitaly-node-fai.yml6
-rw-r--r--db/post_migrate/20191115115043_migrate_epic_mentions_to_db.rb36
-rw-r--r--db/post_migrate/20191115115522_migrate_epic_notes_mentions_to_db.rb45
-rw-r--r--db/schema.rb1
-rw-r--r--doc/development/README.md19
-rw-r--r--doc/development/contributing/design.md2
-rw-r--r--doc/development/contributing/index.md5
-rw-r--r--doc/development/contributing/style_guides.md123
-rw-r--r--doc/development/dangerbot.md11
-rw-r--r--lib/gitlab/background_migration/user_mentions/create_resource_user_mention.rb42
-rw-r--r--lib/gitlab/background_migration/user_mentions/models/epic.rb50
-rw-r--r--lib/gitlab/background_migration/user_mentions/models/epic_user_mention.rb18
-rw-r--r--lib/gitlab/background_migration/user_mentions/models/isolated_mentionable.rb95
-rw-r--r--lib/gitlab/background_migration/user_mentions/models/note.rb65
-rw-r--r--lib/gitlab/cache/ci/project_pipeline_status.rb4
-rw-r--r--spec/frontend/ide/stores/actions/file_spec.js8
-rw-r--r--spec/frontend/repository/log_tree_spec.js2
-rw-r--r--spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb18
-rw-r--r--spec/routing/project_routing_spec.rb20
-rw-r--r--spec/services/error_tracking/base_service_spec.rb74
-rw-r--r--spec/services/error_tracking/list_issues_service_spec.rb37
-rw-r--r--spec/services/error_tracking/list_projects_service_spec.rb26
-rw-r--r--spec/support/shared_examples/models/mentionable_shared_examples.rb10
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
+ &middot;
+ 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