diff options
97 files changed, 1430 insertions, 257 deletions
diff --git a/Dangerfile b/Dangerfile index 6a2c5cf2773..715a2bcbbae 100644 --- a/Dangerfile +++ b/Dangerfile @@ -11,3 +11,4 @@ danger.import_dangerfile(path: 'danger/commit_messages') danger.import_dangerfile(path: 'danger/duplicate_yarn_dependencies') danger.import_dangerfile(path: 'danger/prettier') danger.import_dangerfile(path: 'danger/eslint') +danger.import_dangerfile(path: 'danger/roulette') diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 815d5ca06d5..66e2ae6c25c 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -1.19.0 +1.19.1 diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION index 2bf50aaf17a..56b6be4ebb2 100644 --- a/GITLAB_WORKHORSE_VERSION +++ b/GITLAB_WORKHORSE_VERSION @@ -1 +1 @@ -8.3.0 +8.3.1 diff --git a/app/assets/javascripts/behaviors/markdown/copy_as_gfm.js b/app/assets/javascripts/behaviors/markdown/copy_as_gfm.js index 52d9f2f0322..9482a9f166d 100644 --- a/app/assets/javascripts/behaviors/markdown/copy_as_gfm.js +++ b/app/assets/javascripts/behaviors/markdown/copy_as_gfm.js @@ -36,13 +36,20 @@ export class CopyAsGFM { div.appendChild(el.cloneNode(true)); const html = div.innerHTML; + clipboardData.setData('text/plain', el.textContent); + clipboardData.setData('text/html', html); + // We are also setting this as fallback to transform the selection to gfm on paste + clipboardData.setData('text/x-gfm-html', html); + CopyAsGFM.nodeToGFM(el) .then(res => { - clipboardData.setData('text/plain', el.textContent); clipboardData.setData('text/x-gfm', res); - clipboardData.setData('text/html', html); }) - .catch(() => {}); + .catch(() => { + // Not showing the error as Firefox might doesn't allow + // it or other browsers who have a time limit on the execution + // of the copy event + }); } static pasteGFM(e) { @@ -51,11 +58,28 @@ export class CopyAsGFM { const text = clipboardData.getData('text/plain'); const gfm = clipboardData.getData('text/x-gfm'); - if (!gfm) return; + const gfmHtml = clipboardData.getData('text/x-gfm-html'); + if (!gfm && !gfmHtml) return; e.preventDefault(); - window.gl.utils.insertText(e.target, textBefore => { + // We have the original selection already converted to gfm + if (gfm) { + CopyAsGFM.insertPastedText(e.target, text, gfm); + } else { + // Due to the async copy call we are not able to produce gfm so we transform the cached HTML + const div = document.createElement('div'); + div.innerHTML = gfmHtml; + CopyAsGFM.nodeToGFM(div) + .then(transformedGfm => { + CopyAsGFM.insertPastedText(e.target, text, transformedGfm); + }) + .catch(() => {}); + } + } + + static insertPastedText(target, text, gfm) { + window.gl.utils.insertText(target, textBefore => { // If the text before the cursor contains an odd number of backticks, // we are either inside an inline code span that starts with 1 backtick // or a code block that starts with 3 backticks. diff --git a/app/assets/javascripts/boards/components/board_list.vue b/app/assets/javascripts/boards/components/board_list.vue index a689dfc3768..f3f341ece5c 100644 --- a/app/assets/javascripts/boards/components/board_list.vue +++ b/app/assets/javascripts/boards/components/board_list.vue @@ -221,7 +221,7 @@ export default { </script> <template> - <div class="board-list-component d-flex flex-column"> + <div class="board-list-component"> <div v-if="loading" class="board-list-loading text-center" aria-label="Loading issues"> <gl-loading-icon /> </div> diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 7bbafe66199..5a27388863c 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -144,6 +144,7 @@ export default { if (left || right) { return { + ...line, left: line.left ? mapDiscussions(line.left) : null, right: line.right ? mapDiscussions(line.right, () => !left) : null, }; diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index effb6202327..062024b8cdd 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -161,6 +161,7 @@ export function addContextLines(options) { const normalizedParallelLines = contextLines.map(line => ({ left: line, right: line, + line_code: line.line_code, })); if (options.bottom) { diff --git a/app/assets/javascripts/vue_shared/components/file_row.vue b/app/assets/javascripts/vue_shared/components/file_row.vue index f54033efc54..0cbcdbf2eb4 100644 --- a/app/assets/javascripts/vue_shared/components/file_row.vue +++ b/app/assets/javascripts/vue_shared/components/file_row.vue @@ -136,6 +136,7 @@ export default { <div v-else :class="fileClass" + :title="file.name" class="file-row" role="button" @click="clickFile" diff --git a/app/assets/stylesheets/framework/header.scss b/app/assets/stylesheets/framework/header.scss index 36dd1cee4de..23dcc1817b1 100644 --- a/app/assets/stylesheets/framework/header.scss +++ b/app/assets/stylesheets/framework/header.scss @@ -565,15 +565,14 @@ } .navbar-empty { + justify-content: center; height: $header-height; background: $white-light; border-bottom: 1px solid $white-normal; - .mx-auto { - .tanuki-logo, - img { - height: 36px; - } + .tanuki-logo, + .brand-header-logo { + max-height: 100%; } } diff --git a/app/assets/stylesheets/pages/boards.scss b/app/assets/stylesheets/pages/boards.scss index bc28ffb3a92..a9324ba2ed0 100644 --- a/app/assets/stylesheets/pages/boards.scss +++ b/app/assets/stylesheets/pages/boards.scss @@ -164,6 +164,13 @@ display: none; } } + + &:not(.is-collapsed) { + .board-list-component { + display: flex; + flex-direction: column; + } + } } .board-inner { diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 135730d71e9..c78876d9e2a 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -738,6 +738,8 @@ z-index: 103; background: $gray-light; color: $gl-text-color; + margin-top: -1px; + border-top: 1px solid $border-color; .mr-version-menus-container { display: flex; @@ -789,7 +791,6 @@ position: sticky; top: $header-height + $mr-tabs-height; width: 100%; - border-top: 1px solid $border-color; &.is-fileTreeOpen { margin-left: -16px; @@ -810,10 +811,7 @@ top: $header-height; z-index: 200; background-color: $white-light; - - @include media-breakpoint-down(md) { - border-bottom: 1px solid $border-color; - } + border-bottom: 1px solid $border-color; @include media-breakpoint-up(sm) { position: sticky; diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 26cd5dc801f..af0b0c64814 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -137,6 +137,8 @@ class ApplicationController < ActionController::Base if response.status == 422 && response.body.present? && response.content_type == 'application/json'.freeze payload[:response] = response.body end + + payload[:queue_duration] = request.env[::Gitlab::Middleware::RailsQueueDuration::GITLAB_RAILS_QUEUE_DURATION_KEY] end ## diff --git a/app/controllers/concerns/send_file_upload.rb b/app/controllers/concerns/send_file_upload.rb index 9ca54c5519b..28e4cece548 100644 --- a/app/controllers/concerns/send_file_upload.rb +++ b/app/controllers/concerns/send_file_upload.rb @@ -3,7 +3,7 @@ module SendFileUpload def send_upload(file_upload, send_params: {}, redirect_params: {}, attachment: nil, proxy: false, disposition: 'attachment') if attachment - response_disposition = ::Gitlab::ContentDisposition.format(disposition: 'attachment', filename: attachment) + response_disposition = ::Gitlab::ContentDisposition.format(disposition: disposition, filename: attachment) # Response-Content-Type will not override an existing Content-Type in # Google Cloud Storage, so the metadata needs to be cleared on GCS for diff --git a/app/controllers/help_controller.rb b/app/controllers/help_controller.rb index e5a1fc9d6ff..a9d6addd4a4 100644 --- a/app/controllers/help_controller.rb +++ b/app/controllers/help_controller.rb @@ -13,9 +13,10 @@ class HelpController < ApplicationController # Remove YAML frontmatter so that it doesn't look weird @help_index = File.read(Rails.root.join('doc', 'README.md')).sub(YAML_FRONT_MATTER_REGEXP, '') - # Prefix Markdown links with `help/` unless they are external links - # See http://rubular.com/r/X3baHTbPO2 - @help_index.gsub!(%r{(?<delim>\]\()(?!.+://)(?!/)(?<link>[^\)\(]+\))}) do + # Prefix Markdown links with `help/` unless they are external links. + # '//' not necessarily part of URL, e.g., mailto:mail@example.com + # See https://rubular.com/r/DFHZl5w8d3bpzV + @help_index.gsub!(%r{(?<delim>\]\()(?!\w+:)(?!/)(?<link>[^\)\(]+\))}) do "#{$~[:delim]}#{Gitlab.config.gitlab.relative_url_root}/help/#{$~[:link]}" end end diff --git a/app/helpers/appearances_helper.rb b/app/helpers/appearances_helper.rb index 473c90c882c..7fbbbb04154 100644 --- a/app/helpers/appearances_helper.rb +++ b/app/helpers/appearances_helper.rb @@ -28,7 +28,7 @@ module AppearancesHelper def brand_header_logo if current_appearance&.header_logo? - image_tag current_appearance.header_logo_path + image_tag current_appearance.header_logo_path, class: 'brand-header-logo' else render 'shared/logo.svg' end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index acef5d2e643..372f6d678f6 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -687,9 +687,18 @@ module Ci end end + # Returns the modified paths. + # + # The returned value is + # * Array: List of modified paths that should be evaluated + # * nil: Modified path can not be evaluated def modified_paths strong_memoize(:modified_paths) do - push_details.modified_paths + if merge_request? + merge_request.modified_paths + elsif branch_updated? + push_details.modified_paths + end end end diff --git a/app/models/concerns/with_uploads.rb b/app/models/concerns/with_uploads.rb index d79c0eae77e..6c6febd186c 100644 --- a/app/models/concerns/with_uploads.rb +++ b/app/models/concerns/with_uploads.rb @@ -27,40 +27,14 @@ module WithUploads included do has_many :uploads, as: :model - has_many :file_uploads, -> { where(uploader: FILE_UPLOADERS) }, class_name: 'Upload', as: :model + has_many :file_uploads, -> { where(uploader: FILE_UPLOADERS) }, + class_name: 'Upload', as: :model, + dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent - # TODO: when feature flag is removed, we can use just dependent: destroy - # option on :file_uploads - before_destroy :remove_file_uploads - - use_fast_destroy :file_uploads, if: :fast_destroy_enabled? + use_fast_destroy :file_uploads end def retrieve_upload(_identifier, paths) uploads.find_by(path: paths) end - - private - - # mounted uploads are deleted in carrierwave's after_commit hook, - # but FileUploaders which are not mounted must be deleted explicitly and - # it can not be done in after_commit because FileUploader requires loads - # associated model on destroy (which is already deleted in after_commit) - def remove_file_uploads - fast_destroy_enabled? ? delete_uploads : destroy_uploads - end - - def delete_uploads - file_uploads.delete_all(:delete_all) - end - - def destroy_uploads - file_uploads.find_each do |upload| - upload.destroy - end - end - - def fast_destroy_enabled? - Feature.enabled?(:fast_destroy_uploads, self) - end end diff --git a/app/models/discussion.rb b/app/models/discussion.rb index f2678e0597d..32529ebf71d 100644 --- a/app/models/discussion.rb +++ b/app/models/discussion.rb @@ -17,8 +17,6 @@ class Discussion :for_commit?, :for_merge_request?, - :save, - to: :first_note def project_id diff --git a/app/models/individual_note_discussion.rb b/app/models/individual_note_discussion.rb index aab0ff93468..b4a661ae5b4 100644 --- a/app/models/individual_note_discussion.rb +++ b/app/models/individual_note_discussion.rb @@ -17,8 +17,12 @@ class IndividualNoteDiscussion < Discussion noteable.supports_replying_to_individual_notes? && Feature.enabled?(:reply_to_individual_notes) end - def convert_to_discussion! - first_note.becomes!(Discussion.note_class).to_discussion + def convert_to_discussion!(save: false) + first_note.becomes!(Discussion.note_class).to_discussion.tap do + # Save needs to be called on first_note instead of the transformed note + # because of https://gitlab.com/gitlab-org/gitlab-ce/issues/57324 + first_note.save if save + end end def reply_attributes diff --git a/app/models/releases/link.rb b/app/models/releases/link.rb index 6f639e5a7b2..6c507c47752 100644 --- a/app/models/releases/link.rb +++ b/app/models/releases/link.rb @@ -6,7 +6,7 @@ module Releases belongs_to :release - validates :url, presence: true, url: true, uniqueness: { scope: :release } + validates :url, presence: true, url: { protocols: %w(http https ftp) }, uniqueness: { scope: :release } validates :name, presence: true, uniqueness: { scope: :release } scope :sorted, -> { order(created_at: :desc) } diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 699b3e8555e..354e53a367c 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -36,7 +36,7 @@ module Ci project: project, current_user: current_user, push_options: params[:push_options], - **extra_options(**options)) + **extra_options(options)) sequence = Gitlab::Ci::Pipeline::Chain::Sequence .new(pipeline, command, SEQUENCE) @@ -108,7 +108,12 @@ module Ci end # rubocop: enable CodeReuse/ActiveRecord - def extra_options + def extra_options(options = {}) + # In Ruby 2.4, even when options is empty, f(**options) doesn't work when f + # doesn't have any parameters. We reproduce the Ruby 2.5 behavior by + # checking explicitely that no arguments are given. + raise ArgumentError if options.any? + {} # overriden in EE end end diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index b975c3a8cb6..5a6e7338b42 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -35,7 +35,7 @@ module Notes if !only_commands && note.save if note.part_of_discussion? && note.discussion.can_convert_to_discussion? - note.discussion.convert_to_discussion!.save(touch: false) + note.discussion.convert_to_discussion!(save: true) end todo_service.new_note(note, current_user) diff --git a/app/services/task_list_toggle_service.rb b/app/services/task_list_toggle_service.rb index af24bc0524c..f6602a35033 100644 --- a/app/services/task_list_toggle_service.rb +++ b/app/services/task_list_toggle_service.rb @@ -67,6 +67,6 @@ class TaskListToggleService # When using CommonMark, we should be able to use the embedded `sourcepos` attribute to # target the exact line in the DOM. def get_html_checkbox(html) - html.css(".task-list-item[data-sourcepos^='#{line_number}:'] > input.task-list-item-checkbox").first + html.css(".task-list-item[data-sourcepos^='#{line_number}:'] input.task-list-item-checkbox").first end end diff --git a/app/views/layouts/_mailer.html.haml b/app/views/layouts/_mailer.html.haml index ddc1cdb24b5..26fd34347ec 100644 --- a/app/views/layouts/_mailer.html.haml +++ b/app/views/layouts/_mailer.html.haml @@ -49,7 +49,7 @@ %table#body{ border: "0", cellpadding: "0", cellspacing: "0", style: "background-color:#fafafa;margin:0;padding:0;text-align:center;min-width:640px;width:100%;" } %tbody %tr.line - %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;background-color:#6b4fbb;height:4px;font-size:4px;line-height:4px;" }  + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;background-color:#6b4fbb;height:4px;font-size:4px;line-height:4px;" } %tr.header %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;padding:25px 0;font-size:13px;line-height:1.6;color:#5c5c5c;" } = header_logo diff --git a/app/views/layouts/header/_empty.html.haml b/app/views/layouts/header/_empty.html.haml index 2dfc787b7a8..348ce18b122 100644 --- a/app/views/layouts/header/_empty.html.haml +++ b/app/views/layouts/header/_empty.html.haml @@ -1,4 +1,2 @@ %header.navbar.fixed-top.navbar-empty - .container - .mx-auto - = brand_header_logo + = brand_header_logo diff --git a/changelogs/unreleased/39676-wiki-api-problems-on-update-parameters-and-500-error.yml b/changelogs/unreleased/39676-wiki-api-problems-on-update-parameters-and-500-error.yml new file mode 100644 index 00000000000..1af49fb6a2c --- /dev/null +++ b/changelogs/unreleased/39676-wiki-api-problems-on-update-parameters-and-500-error.yml @@ -0,0 +1,5 @@ +--- +title: 'API: Require only one parameter when updating a wiki' +merge_request: 25191 +author: Robert Schilling +type: fixed diff --git a/changelogs/unreleased/50006-expose-textcolor-from-public-labels-api.yml b/changelogs/unreleased/50006-expose-textcolor-from-public-labels-api.yml new file mode 100644 index 00000000000..3c8b58f3001 --- /dev/null +++ b/changelogs/unreleased/50006-expose-textcolor-from-public-labels-api.yml @@ -0,0 +1,5 @@ +--- +title: 'API: Expose text_color for project and group labels' +merge_request: 25172 +author: Robert Schilling +type: added diff --git a/changelogs/unreleased/50559-add-milestone-progress-to-api.yml b/changelogs/unreleased/50559-add-milestone-progress-to-api.yml new file mode 100644 index 00000000000..e68e4bd6059 --- /dev/null +++ b/changelogs/unreleased/50559-add-milestone-progress-to-api.yml @@ -0,0 +1,5 @@ +--- +title: 'API: Expose milestone progress' +merge_request: 25173 +author: Robert Schilling +type: added diff --git a/changelogs/unreleased/56237-api-truncated-commit-title.yml b/changelogs/unreleased/56237-api-truncated-commit-title.yml new file mode 100644 index 00000000000..1a48d0fda1b --- /dev/null +++ b/changelogs/unreleased/56237-api-truncated-commit-title.yml @@ -0,0 +1,5 @@ +--- +title: 'API: Expose full commit title' +merge_request: 25189 +author: Robert Schilling +type: fixed diff --git a/changelogs/unreleased/57101-api-docs-for-hangouts-chat-service-incorrect.yml b/changelogs/unreleased/57101-api-docs-for-hangouts-chat-service-incorrect.yml new file mode 100644 index 00000000000..2e0ae9c3732 --- /dev/null +++ b/changelogs/unreleased/57101-api-docs-for-hangouts-chat-service-incorrect.yml @@ -0,0 +1,5 @@ +--- +title: 'API: Fix docs and parameters for hangouts-chat service' +merge_request: 25180 +author: Robert Schilling +type: fixed diff --git a/changelogs/unreleased/57160-merge-request-tabs-header-is-missing-bottom-border.yml b/changelogs/unreleased/57160-merge-request-tabs-header-is-missing-bottom-border.yml new file mode 100644 index 00000000000..3146d07db3d --- /dev/null +++ b/changelogs/unreleased/57160-merge-request-tabs-header-is-missing-bottom-border.yml @@ -0,0 +1,5 @@ +--- +title: Return bottom border on MR Tabs +merge_request: !25198 +author: +type: fixed diff --git a/changelogs/unreleased/57223-wiki-finder.yml b/changelogs/unreleased/57223-wiki-finder.yml new file mode 100644 index 00000000000..5ddf197568d --- /dev/null +++ b/changelogs/unreleased/57223-wiki-finder.yml @@ -0,0 +1,5 @@ +--- +title: Remove BATCH_SIZE from WikiFileFinder +merge_request: 24933 +author: +type: other diff --git a/changelogs/unreleased/57410-api-create-release-link-with-ftp-address-return-400-bad-request.yml b/changelogs/unreleased/57410-api-create-release-link-with-ftp-address-return-400-bad-request.yml new file mode 100644 index 00000000000..6be6a2115b9 --- /dev/null +++ b/changelogs/unreleased/57410-api-create-release-link-with-ftp-address-return-400-bad-request.yml @@ -0,0 +1,5 @@ +--- +title: Add support for FTP assets for releases +merge_request: 25071 +author: Robert Schilling +type: added diff --git a/changelogs/unreleased/57589-update-workhorse.yml b/changelogs/unreleased/57589-update-workhorse.yml new file mode 100644 index 00000000000..525913bba4c --- /dev/null +++ b/changelogs/unreleased/57589-update-workhorse.yml @@ -0,0 +1,5 @@ +--- +title: Update Workhorse to v8.3.1 +merge_request: +author: +type: other diff --git a/changelogs/unreleased/add-title-attribute-to-file-row.yml b/changelogs/unreleased/add-title-attribute-to-file-row.yml new file mode 100644 index 00000000000..c68d3d544e7 --- /dev/null +++ b/changelogs/unreleased/add-title-attribute-to-file-row.yml @@ -0,0 +1,5 @@ +--- +title: add title attribute to display file name +merge_request: 25154 +author: Satoshi Nakamatsu @satoshicano +type: added diff --git a/changelogs/unreleased/fast-destroy-uploads.yml b/changelogs/unreleased/fast-destroy-uploads.yml new file mode 100644 index 00000000000..ee3363a6ae9 --- /dev/null +++ b/changelogs/unreleased/fast-destroy-uploads.yml @@ -0,0 +1,5 @@ +--- +title: File uploads are deleted asynchronously when deleting a project or group. +merge_request: +author: +type: added diff --git a/changelogs/unreleased/fix_-56347.yml b/changelogs/unreleased/fix_-56347.yml new file mode 100644 index 00000000000..1d03ed8864c --- /dev/null +++ b/changelogs/unreleased/fix_-56347.yml @@ -0,0 +1,5 @@ +--- +title: Fix overlapping empty-header logo +merge_request: 24868 +author: Jonas L. +type: fixed diff --git a/changelogs/unreleased/ravlen-fix-spaces-unicode.yml b/changelogs/unreleased/ravlen-fix-spaces-unicode.yml new file mode 100644 index 00000000000..fbcbdc53cfe --- /dev/null +++ b/changelogs/unreleased/ravlen-fix-spaces-unicode.yml @@ -0,0 +1,5 @@ +--- +title: Correct non-standard unicode spaces to regular unicode +merge_request: 24795 +author: Marcel Amirault +type: other diff --git a/changelogs/unreleased/sh-log-rails-queue-duration.yml b/changelogs/unreleased/sh-log-rails-queue-duration.yml new file mode 100644 index 00000000000..89390aef108 --- /dev/null +++ b/changelogs/unreleased/sh-log-rails-queue-duration.yml @@ -0,0 +1,5 @@ +--- +title: Log queue duration in production_json.log +merge_request: 25075 +author: +type: other diff --git a/changelogs/unreleased/support-only-changes-on-mr-pipelines.yml b/changelogs/unreleased/support-only-changes-on-mr-pipelines.yml new file mode 100644 index 00000000000..fbab898b799 --- /dev/null +++ b/changelogs/unreleased/support-only-changes-on-mr-pipelines.yml @@ -0,0 +1,5 @@ +--- +title: 'Support `only: changes:` on MR pipelines' +merge_request: 24490 +author: Hiroyuki Sato +type: added diff --git a/config/initializers/lograge.rb b/config/initializers/lograge.rb index c897bc30e76..164954d1293 100644 --- a/config/initializers/lograge.rb +++ b/config/initializers/lograge.rb @@ -23,7 +23,8 @@ unless Sidekiq.server? remote_ip: event.payload[:remote_ip], user_id: event.payload[:user_id], username: event.payload[:username], - ua: event.payload[:ua] + ua: event.payload[:ua], + queue_duration: event.payload[:queue_duration] } gitaly_calls = Gitlab::GitalyClient.get_request_count diff --git a/danger/changelog/Dangerfile b/danger/changelog/Dangerfile index 530c6638653..63b2f6f5c5c 100644 --- a/danger/changelog/Dangerfile +++ b/danger/changelog/Dangerfile @@ -16,16 +16,12 @@ consider adding any of the %<labels>s labels. #{SEE_DOC} MSG -def ee? - ENV['CI_PROJECT_NAME'] == 'gitlab-ee' || File.exist?('../../CHANGELOG-EE.md') -end - def ee_changelog?(changelog_path) changelog_path =~ /unreleased-ee/ end def ce_port_changelog?(changelog_path) - ee? && !ee_changelog?(changelog_path) + helper.ee? && !ee_changelog?(changelog_path) end def check_changelog(path) diff --git a/danger/documentation/Dangerfile b/danger/documentation/Dangerfile index 188331cc87c..dd0e3f6deb6 100644 --- a/danger/documentation/Dangerfile +++ b/danger/documentation/Dangerfile @@ -1,17 +1,6 @@ # frozen_string_literal: true -# All the files/directories that should be reviewed by the Docs team. -DOCS_FILES = [ - 'doc/' -].freeze - -def docs_paths_requiring_review(files) - files.select do |file| - DOCS_FILES.any? { |pattern| file.start_with?(pattern) } - end -end - -docs_paths_to_review = docs_paths_requiring_review(helper.all_changed_files) +docs_paths_to_review = helper.changes_by_category[:documentation] unless docs_paths_to_review.empty? message 'This merge request adds or changes files that require a ' \ diff --git a/danger/plugins/helper.rb b/danger/plugins/helper.rb index f4eb9119266..581c0720083 100644 --- a/danger/plugins/helper.rb +++ b/danger/plugins/helper.rb @@ -1,34 +1,15 @@ # frozen_string_literal: true +require 'net/http' +require 'yaml' + +require_relative '../../lib/gitlab/danger/helper' + module Danger - # Common helper functions for our danger scripts - # If we find ourselves repeating code in our danger files, we might as well put them in here. + # Common helper functions for our danger scripts. See Gitlab::Danger::Helper + # for more details class Helper < Plugin - # Returns a list of all files that have been added, modified or renamed. - # `git.modified_files` might contain paths that already have been renamed, - # so we need to remove them from the list. - # - # Considering these changes: - # - # - A new_file.rb - # - D deleted_file.rb - # - M modified_file.rb - # - R renamed_file_before.rb -> renamed_file_after.rb - # - # it will return - # ``` - # [ 'new_file.rb', 'modified_file.rb', 'renamed_file_after.rb' ] - # ``` - # - # @return [Array<String>] - def all_changed_files - Set.new - .merge(git.added_files.to_a) - .merge(git.modified_files.to_a) - .merge(git.renamed_files.map { |x| x[:after] }) - .subtract(git.renamed_files.map { |x| x[:before] }) - .to_a - .sort - end + # Put the helper code somewhere it can be tested + include Gitlab::Danger::Helper end end diff --git a/danger/roulette/Dangerfile b/danger/roulette/Dangerfile new file mode 100644 index 00000000000..5c3d7a4ca49 --- /dev/null +++ b/danger/roulette/Dangerfile @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +MESSAGE = <<MARKDOWN +## Reviewer roulette + +Changes that require review have been detected! A merge request is normally +reviewed by both a reviewer and a maintainer in its primary category (e.g. +~frontend or ~backend), and by a maintainer in all other categories. +MARKDOWN + +CATEGORY_TABLE_HEADER = <<MARKDOWN + +To spread load more evenly across eligible reviewers, Danger has randomly picked +a candidate for each review slot. Feel free to override this selection if you +think someone else would be better-suited, or the chosen person is unavailable. + +Once you've decided who will review this merge request, mention them as you +normally would! Danger does not (yet?) automatically notify them for you. + +| Category | Reviewer | Maintainer | +| -------- | -------- | ---------- | +MARKDOWN + +UNKNOWN_FILES_MESSAGE = <<MARKDOWN + +These files couldn't be categorised, so Danger was unable to suggest a reviewer. +Please consider creating a merge request to +[add support](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/gitlab/danger/helper.rb) +for them. +MARKDOWN + +def spin(team, project, category) + reviewers = team.select { |member| member.reviewer?(project, category) } + maintainers = team.select { |member| member.maintainer?(project, category) } + + # TODO: filter out people who are currently not in the office + # TODO: take CODEOWNERS into account? + + reviewer = reviewers[rand(reviewers.size)] + maintainer = maintainers[rand(maintainers.size)] + + "| #{helper.label_for_category(category)} | #{reviewer&.markdown_name} | #{maintainer&.markdown_name} |" +end + +def build_list(items) + list = items.map { |filename| "* `#{filename}`" }.join("\n") + + if items.size > 10 + "\n<details>\n\n#{list}\n\n</details>" + else + list + end +end + +changes = helper.changes_by_category +categories = changes.keys - [:unknown] + +unless changes.empty? + team = + begin + helper.project_team + rescue => err + warn("Reviewer roulette failed to load team data: #{err.message}") + [] + end + + # Exclude the MR author from the team for selection purposes + team.delete_if { |teammate| teammate.username == gitlab.mr_author } + + project = helper.project_name + unknown = changes.fetch(:unknown, []) + + rows = categories.map { |category| spin(team, project, category) } + + markdown(MESSAGE) + markdown(CATEGORY_TABLE_HEADER + rows.join("\n")) unless rows.empty? + markdown(UNKNOWN_FILES_MESSAGE + build_list(unknown)) unless unknown.empty? +end diff --git a/doc/administration/logs.md b/doc/administration/logs.md index 698f4caab3a..36dee75bd44 100644 --- a/doc/administration/logs.md +++ b/doc/administration/logs.md @@ -23,12 +23,13 @@ requests from the API are logged to a separate file in `api_json.log`. Each line contains a JSON line that can be ingested by Elasticsearch, Splunk, etc. For example: ```json -{"method":"GET","path":"/gitlab/gitlab-ce/issues/1234","format":"html","controller":"Projects::IssuesController","action":"show","status":200,"duration":229.03,"view":174.07,"db":13.24,"time":"2017-08-08T20:15:54.821Z","params":[{"key":"param_key","value":"param_value"}],"remote_ip":"18.245.0.1","user_id":1,"username":"admin","gitaly_calls":76} +{"method":"GET","path":"/gitlab/gitlab-ce/issues/1234","format":"html","controller":"Projects::IssuesController","action":"show","status":200,"duration":229.03,"view":174.07,"db":13.24,"time":"2017-08-08T20:15:54.821Z","params":[{"key":"param_key","value":"param_value"}],"remote_ip":"18.245.0.1","user_id":1,"username":"admin","gitaly_calls":76,"queue_duration": 112.47} ``` In this example, you can see this was a GET request for a specific issue. Notice each line also contains performance data: -1. `duration`: the total time taken to retrieve the request +1. `duration`: total time in milliseconds taken to retrieve the request +1. `queue_duration`: total time in milliseconds that the request was queued inside GitLab Workhorse 1. `view`: total time taken inside the Rails views 1. `db`: total time to retrieve data from the database 1. `gitaly_calls`: total number of calls made to Gitaly @@ -91,6 +92,8 @@ This entry above shows an access to an internal endpoint to check whether an associated SSH key can download the project in question via a `git fetch` or `git clone`. In this example, we see: +1. `duration`: total time in milliseconds taken to retrieve the request +1. `queue_duration`: total time in milliseconds that the request was queued inside GitLab Workhorse 1. `method`: The HTTP method used to make the request 1. `path`: The relative path of the query 1. `params`: Key-value pairs passed in a query string or HTTP body. Sensitive parameters (e.g. passwords, tokens, etc.) are filtered out. diff --git a/doc/administration/pages/index.md b/doc/administration/pages/index.md index 10ae8c7dedf..5c809f25fbd 100644 --- a/doc/administration/pages/index.md +++ b/doc/administration/pages/index.md @@ -11,7 +11,7 @@ description: 'Learn how to administer GitLab Pages.' > - This guide is for Omnibus GitLab installations. If you have installed > GitLab from source, follow the [Pages source installation document](source.md). > - To learn how to use GitLab Pages, read the [user documentation][pages-userguide]. -> - Does NOT support subgroups. See [this issue](https://gitlab.com/gitlab-org/gitlab-ce/issues/30548) for more information and status. +> - Support for subgroup project's websites was [introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/30548) in GitLab 11.8. This document describes how to set up the _latest_ GitLab Pages feature. Make sure to read the [changelog](#changelog) if you are upgrading to a new GitLab diff --git a/doc/api/group_labels.md b/doc/api/group_labels.md index d4715dec192..3d4b099b49e 100644 --- a/doc/api/group_labels.md +++ b/doc/api/group_labels.md @@ -28,6 +28,7 @@ Example response: "id": 7, "name": "bug", "color": "#FF0000", + "text_color" : "#FFFFFF", "description": null, "open_issues_count": 0, "closed_issues_count": 0, @@ -38,6 +39,7 @@ Example response: "id": 4, "name": "feature", "color": "#228B22", + "text_color" : "#FFFFFF", "description": null, "open_issues_count": 0, "closed_issues_count": 0, @@ -73,6 +75,7 @@ Example response: "id": 9, "name": "Feature Proposal", "color": "#FFA500", + "text_color" : "#FFFFFF", "description": "Describes new ideas", "open_issues_count": 0, "closed_issues_count": 0, @@ -108,6 +111,7 @@ Example response: "id": 9, "name": "Feature Idea", "color": "#FFA500", + "text_color" : "#FFFFFF", "description": "Describes new ideas", "open_issues_count": 0, "closed_issues_count": 0, @@ -158,6 +162,7 @@ Example response: "id": 9, "name": "Feature Idea", "color": "#FFA500", + "text_color" : "#FFFFFF", "description": "Describes new ideas", "open_issues_count": 0, "closed_issues_count": 0, @@ -192,6 +197,7 @@ Example response: "id": 9, "name": "Feature Idea", "color": "#FFA500", + "text_color" : "#FFFFFF", "description": "Describes new ideas", "open_issues_count": 0, "closed_issues_count": 0, diff --git a/doc/api/group_milestones.md b/doc/api/group_milestones.md index 7be01ce9c6d..920acaf3e49 100644 --- a/doc/api/group_milestones.md +++ b/doc/api/group_milestones.md @@ -42,6 +42,7 @@ Example Response: "due_date": "2013-11-29", "start_date": "2013-11-10", "state": "active", + "percentage_complete" : 66, "updated_at": "2013-10-02T09:24:18Z", "created_at": "2013-10-02T09:24:18Z" } diff --git a/doc/api/labels.md b/doc/api/labels.md index aec1a2c7592..5b5b04bc1d7 100644 --- a/doc/api/labels.md +++ b/doc/api/labels.md @@ -24,6 +24,7 @@ Example response: "id" : 1, "name" : "bug", "color" : "#d9534f", + "text_color" : "#FFFFFF", "description": "Bug reported by user", "open_issues_count": 1, "closed_issues_count": 0, @@ -34,6 +35,7 @@ Example response: { "id" : 4, "color" : "#d9534f", + "text_color" : "#FFFFFF", "name" : "confirmed", "description": "Confirmed issue", "open_issues_count": 2, @@ -46,6 +48,7 @@ Example response: "id" : 7, "name" : "critical", "color" : "#d9534f", + "text_color" : "#FFFFFF", "description": "Critical issue. Need fix ASAP", "open_issues_count": 1, "closed_issues_count": 3, @@ -57,6 +60,7 @@ Example response: "id" : 8, "name" : "documentation", "color" : "#f0ad4e", + "text_color" : "#FFFFFF", "description": "Issue about documentation", "open_issues_count": 1, "closed_issues_count": 0, @@ -67,6 +71,7 @@ Example response: { "id" : 9, "color" : "#5cb85c", + "text_color" : "#FFFFFF", "name" : "enhancement", "description": "Enhancement proposal", "open_issues_count": 1, @@ -105,6 +110,7 @@ Example response: "id" : 10, "name" : "feature", "color" : "#5843AD", + "text_color" : "#FFFFFF", "description":null, "open_issues_count": 0, "closed_issues_count": 0, @@ -161,6 +167,7 @@ Example response: "id" : 8, "name" : "docs", "color" : "#8E44AD", + "text_color" : "#FFFFFF", "description": "Documentation", "open_issues_count": 1, "closed_issues_count": 0, @@ -196,6 +203,7 @@ Example response: "id" : 1, "name" : "bug", "color" : "#d9534f", + "text_color" : "#FFFFFF", "description": "Bug reported by user", "open_issues_count": 1, "closed_issues_count": 0, diff --git a/doc/api/milestones.md b/doc/api/milestones.md index fa8f8a0bcf0..21a390442bd 100644 --- a/doc/api/milestones.md +++ b/doc/api/milestones.md @@ -39,6 +39,7 @@ Example Response: "due_date": "2013-11-29", "start_date": "2013-11-10", "state": "active", + "percentage_complete" : 66, "updated_at": "2013-10-02T09:24:18Z", "created_at": "2013-10-02T09:24:18Z" } diff --git a/doc/api/releases/index.md b/doc/api/releases/index.md index 943109a3ea9..61f4ec130fa 100644 --- a/doc/api/releases/index.md +++ b/doc/api/releases/index.md @@ -14,7 +14,7 @@ GET /projects/:id/releases | Attribute | Type | Required | Description | | ------------- | -------------- | -------- | --------------------------------------- | -| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding). | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](../README.md#namespaced-path-encoding). | Example request: @@ -160,7 +160,7 @@ GET /projects/:id/releases/:tag_name | Attribute | Type | Required | Description | | ------------- | -------------- | -------- | --------------------------------------- | -| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding). | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](../README.md#namespaced-path-encoding). | | `tag_name` | string | yes | The tag where the release will be created from. | Example request: @@ -239,10 +239,10 @@ POST /projects/:id/releases | Attribute | Type | Required | Description | | ------------- | -------------- | -------- | --------------------------------------- | -| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding). | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](../README.md#namespaced-path-encoding). | | `name` | string | yes | The release name. | | `tag_name` | string | yes | The tag where the release will be created from. | -| `description` | string | yes | The description of the release. You can use [markdown](../user/markdown.md). | +| `description` | string | yes | The description of the release. You can use [markdown](../../user/markdown.md). | | `ref` | string | no | If `tag_name` doesn't exist, the release will be created from `ref`. It can be a commit SHA, another tag name, or a branch name. | | `assets:links`| array of hash | no | An array of assets links. | | `assets:links:name`| string | no (if `assets:links` specified, it's required) | The name of the link. | @@ -331,10 +331,10 @@ PUT /projects/:id/releases/:tag_name | Attribute | Type | Required | Description | | ------------- | -------------- | -------- | --------------------------------------- | -| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding). | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](../README.md#namespaced-path-encoding). | | `tag_name` | string | yes | The tag where the release will be created from. | | `name` | string | no | The release name. | -| `description` | string | no | The description of the release. You can use [markdown](../user/markdown.md). | +| `description` | string | no | The description of the release. You can use [markdown](../../user/markdown.md). | Example request: @@ -412,7 +412,7 @@ DELETE /projects/:id/releases/:tag_name | Attribute | Type | Required | Description | | ------------- | -------------- | -------- | --------------------------------------- | -| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding). | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](../README.md#namespaced-path-encoding). | | `tag_name` | string | yes | The tag where the release will be created from. | Example request: diff --git a/doc/api/releases/links.md b/doc/api/releases/links.md index ae99f3bd8b6..fd7b9d6e6e2 100644 --- a/doc/api/releases/links.md +++ b/doc/api/releases/links.md @@ -3,6 +3,7 @@ > [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/41766) in GitLab 11.7. Using this API you can manipulate GitLab's [Release](../../user/project/releases/index.md) links. For manipulating other Release assets, see [Release API](index.md). +GitLab supports links links to `http`, `https`, and `ftp` assets. ## Get links diff --git a/doc/api/services.md b/doc/api/services.md index 2a8ce39e570..5d5aa3e5b3e 100644 --- a/doc/api/services.md +++ b/doc/api/services.md @@ -412,7 +412,7 @@ Google GSuite team collaboration tool. Set Hangouts Chat service for a project. ``` -PUT /projects/:id/services/hangouts_chat +PUT /projects/:id/services/hangouts-chat ``` >**Note:** Specific event parameters (e.g. `push_events` flag) were [introduced in v10.4][11435] @@ -438,7 +438,7 @@ Parameters: Delete Hangouts Chat service for a project. ``` -DELETE /projects/:id/services/hangouts_chat +DELETE /projects/:id/services/hangouts-chat ``` ### Get Hangouts Chat service settings @@ -446,7 +446,7 @@ DELETE /projects/:id/services/hangouts_chat Get Hangouts Chat service settings for a project. ``` -GET /projects/:id/services/hangouts_chat +GET /projects/:id/services/hangouts-chat ``` ## Irker (IRC gateway) diff --git a/doc/ci/examples/deploy_spring_boot_to_cloud_foundry/index.md b/doc/ci/examples/deploy_spring_boot_to_cloud_foundry/index.md index 6499413baf0..474a481836a 100644 --- a/doc/ci/examples/deploy_spring_boot_to_cloud_foundry/index.md +++ b/doc/ci/examples/deploy_spring_boot_to_cloud_foundry/index.md @@ -112,7 +112,7 @@ on GitLab CI/CD. To set the environment variables, navigate to your project's  Once set up, GitLab CI/CD will deploy your app to CF at every push to your -repository's deafult branch. To see the build logs or watch your builds running +repository's default branch. To see the build logs or watch your builds running live, navigate to **CI/CD > Pipelines**. CAUTION: **Caution:** diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 984878b6c9b..8c8a31e9323 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -423,10 +423,28 @@ connected with merge requests yet, and because GitLab is creating pipelines before an user can create a merge request we don't know a target branch at this point. -Without a target branch, it is not possible to know what the common ancestor is, -thus we always create a job in that case. This feature works best for stable -branches like `master` because in that case GitLab uses the previous commit -that is present in a branch to compare against the latest SHA that was pushed. +#### Using `changes` with `merge_requests` + +With [pipelines for merge requests](../merge_request_pipelines/index.md), +make it possible to define if a job should be created base on files modified +in a merge request. + +For example: + +``` +docker build service one: + script: docker build -t my-service-one-image:$CI_COMMIT_REF_SLUG . + only: + refs: + - merge_requests + changes: + - Dockerfile + - service-one/**/* +``` + +In the scenario above, if you create or update a merge request that changes +either files in `service-one` folder or `Dockerfile`, GitLab creates and triggers +the `docker build service one` job. ## `tags` diff --git a/doc/development/README.md b/doc/development/README.md index d5829e31343..13646cbfe48 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -49,6 +49,7 @@ description: 'Learn how to contribute to GitLab.' - [Working with the GitHub importer](github_importer.md) - [Import/Export development documentation](import_export.md) - [Working with Merge Request diffs](diffs.md) +- [Kubernetes integration guidelines](kubernetes.md) - [Permissions](permissions.md) - [Prometheus metrics](prometheus_metrics.md) - [Guidelines for reusing abstractions](reusing_abstractions.md) diff --git a/doc/development/code_review.md b/doc/development/code_review.md index 25ea2211b64..4e5fb47e331 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -23,6 +23,11 @@ one of the [Merge request coaches][team]. If you need assistance with security scans or comments, feel free to include the Security Team (`@gitlab-com/gl-security`) in the review. +The `danger-review` CI job will randomly pick a reviewer and a maintainer for +each area of the codebase that your merge request seems to touch. It only makes +recommendations - feel free to override it if you think someone else is a better +fit! + Depending on the areas your merge request touches, it must be **approved** by one or more [maintainers](https://about.gitlab.com/handbook/engineering/workflow/code-review/#maintainer): diff --git a/doc/development/documentation/styleguide.md b/doc/development/documentation/styleguide.md index cda66447c2c..7a3a8f25c2d 100644 --- a/doc/development/documentation/styleguide.md +++ b/doc/development/documentation/styleguide.md @@ -112,7 +112,7 @@ table_display_block: true ## Emphasis - Use double asterisks (`**`) to mark a word or text in bold (`**bold**`). -- Use undescore (`_`) for text in italics (`_italic_`). +- Use underscore (`_`) for text in italics (`_italic_`). - Use greater than (`>`) for blockquotes. ## Punctuation diff --git a/doc/development/fe_guide/index.md b/doc/development/fe_guide/index.md index 3a3cb77f592..86b8972a69e 100644 --- a/doc/development/fe_guide/index.md +++ b/doc/development/fe_guide/index.md @@ -1,7 +1,7 @@ # Frontend Development Guidelines > **Notice:** -We are currently in the process of re-writing our development guide to make it easier to find information. The new guide is still WIP but viewable in [development/new_fe_guide](../new_fe_guide/index.md) +> We are currently in the process of re-writing our development guide to make it easier to find information. The new guide is still WIP but viewable in [development/new_fe_guide](../new_fe_guide/index.md) This document describes various guidelines to ensure consistency and quality across GitLab's frontend team. @@ -32,32 +32,41 @@ For our currently-supported browsers, see our [requirements][requirements]. --- ## [Development Process](development_process.md) + How we plan and execute the work on the frontend. ## [Architecture](architecture.md) + How we go about making fundamental design decisions in GitLab's frontend team or make changes to our frontend development guidelines. ## [Testing](../testing_guide/frontend_testing.md) + How we write frontend tests, run the GitLab test suite, and debug test related issues. ## [Design Patterns](design_patterns.md) + Common JavaScript design patterns in GitLab's codebase. ## [Vue.js Best Practices](vue.md) + Vue specific design patterns and practices. ## [Vuex](vuex.md) + Vuex specific design patterns and practices. ## [Axios](axios.md) + Axios specific practices and gotchas. ## [GraphQL](graphql.md) + How to use GraphQL ## [Icons and Illustrations](icons.md) + How we use SVG for our Icons and Illustrations. ## [Components](components.md) @@ -70,7 +79,7 @@ How we use UI components. ### [JavaScript Style Guide](style_guide_js.md) -We use eslint to enforce our JavaScript style guides. Our guide is based on +We use eslint to enforce our JavaScript style guides. Our guide is based on the excellent [Airbnb][airbnb-js-style-guide] style guide with a few small changes. @@ -81,23 +90,26 @@ Our SCSS conventions which are enforced through [scss-lint][scss-lint]. --- ## [Performance](performance.md) + Best practices for monitoring and maximizing frontend performance. --- ## [Security](security.md) + Frontend security practices. --- ## [Accessibility](accessibility.md) + Our accessibility standards and resources. ## [Internationalization (i18n) and Translations](../i18n/externalization.md) + Frontend internationalization support is described in [this document](../i18n/). The [externalization part of the guide](../i18n/externalization.md) explains the helpers/methods available. - [rails]: http://rubyonrails.org/ [haml]: http://haml.info/ [hamlit]: https://github.com/k0kubun/hamlit @@ -116,6 +128,7 @@ The [externalization part of the guide](../i18n/externalization.md) explains the --- ## [DropLab](droplab/droplab.md) + Our internal `DropLab` dropdown library. - [DropLab](droplab/droplab.md) diff --git a/doc/development/kubernetes.md b/doc/development/kubernetes.md new file mode 100644 index 00000000000..4b2d48903ac --- /dev/null +++ b/doc/development/kubernetes.md @@ -0,0 +1,126 @@ +# Kubernetes integration - development guidelines + +This document provides various guidelines when developing for GitLab's +[Kubernetes integration](../user/project/clusters/index.md). + +## Development + +### Architecture + +Some Kubernetes operations, such as creating restricted project +namespaces are performed on the GitLab Rails application. These +operations are performed using a [client library](#client-library). +These operations will carry an element of risk as the operations will be +run as the same user running the GitLab Rails application, see the +[security](#security) section below. + +Some Kubernetes operations, such as installing cluster applications are +performed on one-off pods on the Kubernetes cluster itself. These +installation pods are currently named `install-<application_name>` and +are created within the `gitlab-managed-apps` namespace. + +In terms of code organization, we generally add objects that represent +Kubernetes resources in +[`lib/gitlab/kubernetes`](https://gitlab.com/gitlab-org/gitlab-ce/tree/master/lib/gitlab/kubernetes). + +### Client library + +We use the [`kubeclient`](https://rubygems.org/gems/kubeclient) gem to +perform Kubernetes API calls. As the `kubeclient` gem does not support +different API Groups (e.g. `apis/rbac.authorization.k8s.io`) from a +single client, we have created a wrapper class, +[`Gitlab::Kubernetes::KubeClient`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/gitlab/kubernetes/kube_client.rb) +that will enable you to achieve this. + +Selected Kubernetes API groups are currently supported. Do add support +for new API groups or methods to +[`Gitlab::Kubernetes::KubeClient`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/gitlab/kubernetes/kube_client.rb) +if you need to use them. New API groups or API group versions can be +added to `SUPPORTED_API_GROUPS` - internally, this will create an +internal client for that group. New methods can be added as a delegation +to the relevant internal client. + +### Performance considerations + +All calls to the Kubernetes API must be in a background process. Do not +perform Kubernetes API calls within a web request as this will block +unicorn and can easily lead to a Denial Of Service (DoS) attack in GitLab as +the Kubernetes cluster response times are outside of our control. + +The easiest way to ensure your calls happen a background process is to +delegate any such work to happen in a [sidekiq +worker](sidekiq_style_guide.md). + +There are instances where you would like to make calls to Kubernetes and +return the response and as such a background worker does not seem to be +a good fit. For such cases you should make use of [reactive +caching](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/models/concerns/reactive_caching.rb). +For example: + +```ruby + def calculate_reactive_cache! + { pods: cluster.platform_kubernetes.kubeclient.get_pods } + end + + def pods + with_reactive_cache do |data| + data[:pods] + end + end +``` + +### Testing + +We have some Webmock stubs in +[`KubernetesHelpers`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/spec/support/helpers/kubernetes_helpers.rb) +which can help with mocking out calls to Kubernetes API in your tests. + +## Security + +### SSRF + +As URLs for Kubernetes clusters are user controlled it is easily +susceptible to Server Side Request Forgery (SSRF) attacks. You should +understand the mitigation strategies if you are adding more API calls to +a cluster. + +Mitigation strategies include: + +1. Not allowing redirects to attacker controller resources: + [`Kubeclient::KubeClient`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/gitlab/kubernetes/kube_client.rb#) + can be configured to disallow any redirects by passing in + `http_max_redirects: 0` as an option. +1. Not exposing error messages: by doing so, we + prevent attackers from triggering errors to expose results from + attacker controlled requests. For example, we do not expose (or store) + raw error messages: + + ```ruby + rescue Kubernetes::HttpError => e + # bad + # app.make_errored!("Kubernetes error: #{e.message}") + + # good + app.make_errored!("Kubernetes error: #{e.error_code}") + ``` + +## Debugging + +Logs related to the Kubernetes integration can be found in +[kubernetes.log](../administration/logs.md#kuberneteslog). On a local +GDK install, this will be present in `log/kubernetes.log`. + +Some services such as +[`Clusters::Applications::InstallService`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/services/clusters/applications/install_service.rb#L18) +rescues `StandardError` which can make it harder to debug issues in an +development environment. The current workaround is to temporarily +comment out the `rescue` in your local development source. + +You can also follow the installation pod logs to debug issues related to +installation. Once the installation/upgrade is underway, wait for the +pod to be created. Then run the following to obtain the pods logs as +they are written: + +```bash +kubectl logs <pod_name> --follow -n gitlab-managed-apps +``` diff --git a/doc/development/new_fe_guide/event_tracking.md b/doc/development/new_fe_guide/event_tracking.md new file mode 100644 index 00000000000..1958f1ce528 --- /dev/null +++ b/doc/development/new_fe_guide/event_tracking.md @@ -0,0 +1,74 @@ +# Event Tracking + +We use [Snowplow](https://github.com/snowplow/snowplow) for tracking custom events. + +## Generic tracking function + +In addition to Snowplow's built-in method for tracking page views, we use a generic tracking function which enables us to selectively apply listeners to events. + +The generic tracking function can be imported in EE-specific JS files as follows: + +```javascript +import { trackEvent } from `ee/stats`; +``` + +This gives the user access to the `trackEvent` method, which takes the following parameters: + +| parameter | type | description | required | +| ---------------- | ------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------- | +| `category` | string | Describes the page that you're capturing click events on. Unless infeasible, please use the Rails page attribute `document.body.dataset.page` by default. | true | +| `eventName` | string | Describes the action the user is taking. The first word should always describe the action. For example, clicks should be `click` and activations should be `activate`. Use underscores to describe what was acted on. For example, activating a form field would be `activate_form_input`. Clicking on a dropdown is `click_dropdown`. | true | +| `additionalData` | object | Additional data such as `label`, `property`, and `value` as described [in our Feature Instrumentation taxonomy](https://about.gitlab.com/handbook/product/feature-instrumentation/#taxonomy). | false | + +Read more about instrumentation and the taxonomy in the [Product Handbook](https://about.gitlab.com/handbook/product/feature-instrumentation). + +### Tracking in `.js` and `.vue` files + +The most simple use case is to add tracking programmatically to an event of interest in Javascript. + +The following example demonstrates how to track a click on a button in Javascript by calling the `trackEvent` method explicitly: + +```javascript +import { trackEvent } from `ee/stats`; + +trackEvent('dashboard:projects:index', 'click_button', { + label: 'create_from_template', + property: 'template_preview', + value: 'rails', +}); +``` + +### Tracking in HAML templates + +Sometimes we want to track clicks for multiple elements on a page. Creating event handlers for all elements could soon turn into a tedious task. + +There's a more convenient solution to this problem. When working with HAML templates, we can add `data-track-*` attributes to elements of interest. This way, all elements that have both `data-track-label` and `data-track-event` attributes assigned get marked for event tracking. All we have to do is call the `bindTrackableContainer` method on a container which allows for better scoping. + +Below is an example of `data-track-*` attributes assigned to a button in HAML: + +```ruby +%button.btn{ data: { track_label: "create_from_template", track_property: "template_preview", track_event: "click_button", track_value: "my-template" } } +``` + +By calling `bindTrackableContainer('.my-container')`, click handlers get bound to all elements located in `.my-container` provided that they have the necessary `data-track-*` attributes assigned to them. + +```javascript +import Stats from 'ee/stats'; + +document.addEventListener('DOMContentLoaded', () => { + Stats.bindTrackableContainer('.my-container', 'category'); +}); +``` + +The second parameter in `bindTrackableContainer` is optional. If omitted, the value of `document.body.dataset.page` will be used as category instead. + +Below is a list of supported `data-track-*` attributes: + +| attribute | description | required | +| --------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------- | +| `data-track-label` | The `label` in `trackEvent` | true | +| `data-track-event` | The `eventName` in `trackEvent` | true | +| `data-track-property` | The `property` in `trackEvent`. If omitted, an empty string will be used as a default value. | false | +| `data-track-value` | The `value` in `trackEvent`. If omitted, this will be `target.value` or empty string. For checkboxes, the default value being tracked will be the element's checked attribute if `data-track-value` is omitted. | false | + +Since Snowplow is an Enterprise Edition feature, it's necessary to create a CE backport when adding `data-track-*` attributes to HAML templates in most cases. diff --git a/doc/development/new_fe_guide/index.md b/doc/development/new_fe_guide/index.md index bfcca9cec7b..5fd5af252ef 100644 --- a/doc/development/new_fe_guide/index.md +++ b/doc/development/new_fe_guide/index.md @@ -27,6 +27,10 @@ Learn about all the internal JavaScript modules that make up our frontend. Style guides to keep our code consistent. +## [Event Tracking with Snowplow](event_tracking.md) + +How we use Snowplow to track custom events. + ## [Tips](tips.md) Tips from our frontend team to develop more efficiently and effectively. diff --git a/doc/user/profile/account/two_factor_authentication.md b/doc/user/profile/account/two_factor_authentication.md index 83bc79925e1..0a7c5832a2d 100644 --- a/doc/user/profile/account/two_factor_authentication.md +++ b/doc/user/profile/account/two_factor_authentication.md @@ -18,7 +18,7 @@ the second factor of authentication. Once enabled, in addition to supplying your password to login, you'll be prompted to activate your U2F device (usually by pressing a button on it), and it will perform secure authentication on your behalf. -The U2F workflow is only supported by Google Chrome at this point, so we _strongly_ recommend +The U2F workflow is only [supported by](https://caniuse.com/#search=U2F) Google Chrome, Opera and Firefox at this point, so we _strongly_ recommend that you set up both methods of two-factor authentication, so you can still access your account from other browsers. diff --git a/doc/user/project/pages/index.md b/doc/user/project/pages/index.md index 11f6165fcb4..2de3fb7e080 100644 --- a/doc/user/project/pages/index.md +++ b/doc/user/project/pages/index.md @@ -66,7 +66,7 @@ publish any website written directly in plain HTML, CSS, and JavaScript.</p> If you're using GitLab.com, your website will be publicly available to the internet. If you're using self-managed instances (Core, Starter, Premium, or Ultimate), your websites will be published on your own server, according to the -[Pages admin settings](../../../administration/pages/index.md) chosen by your sysdamin, +[Pages admin settings](../../../administration/pages/index.md) chosen by your sysadmin, who can opt for making them public or internal to your server. ### How it works diff --git a/doc/user/project/web_ide/index.md b/doc/user/project/web_ide/index.md index 9c095da5a4b..3e85e97d7a5 100644 --- a/doc/user/project/web_ide/index.md +++ b/doc/user/project/web_ide/index.md @@ -50,7 +50,7 @@ review the list of changed files. Click on each file to review the changes and click the tick icon to stage the file. Once you have staged some changes, you can add a commit message and commit the -staged changes. Unstaged changes will not be commited. +staged changes. Unstaged changes will not be committed.  diff --git a/lib/api/entities.rb b/lib/api/entities.rb index beb8ce349b4..0ef56067b95 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -369,8 +369,9 @@ module API end class Commit < Grape::Entity - expose :id, :short_id, :title, :created_at + expose :id, :short_id, :created_at expose :parent_ids + expose :full_title, as: :title expose :safe_message, as: :message expose :author_name, :author_email, :authored_date expose :committer_name, :committer_email, :committed_date @@ -501,6 +502,9 @@ module API expose :state, :created_at, :updated_at expose :due_date expose :start_date + expose :percentage_complete do |milestone, options| + milestone.percent_complete(options[:current_user]) + end expose :web_url do |milestone, _options| Gitlab::UrlBuilder.build(milestone) @@ -1003,7 +1007,7 @@ module API end class LabelBasic < Grape::Entity - expose :id, :name, :color, :description + expose :id, :name, :color, :description, :text_color end class Label < LabelBasic diff --git a/lib/api/milestone_responses.rb b/lib/api/milestone_responses.rb index a0ca39b69d4..4c68c568aaa 100644 --- a/lib/api/milestone_responses.rb +++ b/lib/api/milestone_responses.rb @@ -35,19 +35,19 @@ module API milestones = filter_by_iid(milestones, params[:iids]) if params[:iids].present? milestones = filter_by_search(milestones, params[:search]) if params[:search] - present paginate(milestones), with: Entities::Milestone + present paginate(milestones), with: Entities::Milestone, current_user: current_user end def get_milestone_for(parent) milestone = parent.milestones.find(params[:milestone_id]) - present milestone, with: Entities::Milestone + present milestone, with: Entities::Milestone, current_user: current_user end def create_milestone_for(parent) milestone = ::Milestones::CreateService.new(parent, current_user, declared_params).execute if milestone.valid? - present milestone, with: Entities::Milestone + present milestone, with: Entities::Milestone, current_user: current_user else render_api_error!("Failed to create milestone #{milestone.errors.messages}", 400) end @@ -60,7 +60,7 @@ module API milestone = ::Milestones::UpdateService.new(parent, current_user, milestone_params).execute(milestone) if milestone.valid? - present milestone, with: Entities::Milestone + present milestone, with: Entities::Milestone, current_user: current_user else render_api_error!("Failed to update milestone #{milestone.errors.messages}", 400) end diff --git a/lib/api/services.rb b/lib/api/services.rb index 36bdba2d765..163c7505a65 100644 --- a/lib/api/services.rb +++ b/lib/api/services.rb @@ -368,8 +368,9 @@ module API name: :webhook, type: String, desc: 'The Hangouts Chat webhook. e.g. https://chat.googleapis.com/v1/spaces…' - } - ], + }, + CHAT_NOTIFICATION_EVENTS + ].flatten, 'irker' => [ { required: true, diff --git a/lib/api/wikis.rb b/lib/api/wikis.rb index ef0e3decc2c..994074ddc67 100644 --- a/lib/api/wikis.rb +++ b/lib/api/wikis.rb @@ -11,9 +11,7 @@ module API } end - params :wiki_page_params do - requires :content, type: String, desc: 'Content of a wiki page' - requires :title, type: String, desc: 'Title of a wiki page' + params :common_wiki_page_params do optional :format, type: String, values: ProjectWiki::MARKUPS.values.map(&:to_s), @@ -54,7 +52,9 @@ module API success Entities::WikiPage end params do - use :wiki_page_params + requires :title, type: String, desc: 'Title of a wiki page' + requires :content, type: String, desc: 'Content of a wiki page' + use :common_wiki_page_params end post ':id/wikis' do authorize! :create_wiki, user_project @@ -72,7 +72,10 @@ module API success Entities::WikiPage end params do - use :wiki_page_params + optional :title, type: String, desc: 'Title of a wiki page' + optional :content, type: String, desc: 'Content of a wiki page' + use :common_wiki_page_params + at_least_one_of :content, :title, :format end put ':id/wikis/:slug' do authorize! :create_wiki, user_project diff --git a/lib/gitlab/ci/build/policy/changes.rb b/lib/gitlab/ci/build/policy/changes.rb index 1663c875426..9c705a1cd3e 100644 --- a/lib/gitlab/ci/build/policy/changes.rb +++ b/lib/gitlab/ci/build/policy/changes.rb @@ -10,7 +10,7 @@ module Gitlab end def satisfied_by?(pipeline, seed) - return true unless pipeline.branch_updated? + return true if pipeline.modified_paths.nil? pipeline.modified_paths.any? do |path| @globs.any? do |glob| diff --git a/lib/gitlab/danger/helper.rb b/lib/gitlab/danger/helper.rb new file mode 100644 index 00000000000..80760e41c7d --- /dev/null +++ b/lib/gitlab/danger/helper.rb @@ -0,0 +1,131 @@ +# frozen_string_literal: true +require 'net/http' +require 'json' + +require_relative 'teammate' + +module Gitlab + module Danger + module Helper + ROULETTE_DATA_URL = URI.parse('https://about.gitlab.com/roulette.json').freeze + + # Returns a list of all files that have been added, modified or renamed. + # `git.modified_files` might contain paths that already have been renamed, + # so we need to remove them from the list. + # + # Considering these changes: + # + # - A new_file.rb + # - D deleted_file.rb + # - M modified_file.rb + # - R renamed_file_before.rb -> renamed_file_after.rb + # + # it will return + # ``` + # [ 'new_file.rb', 'modified_file.rb', 'renamed_file_after.rb' ] + # ``` + # + # @return [Array<String>] + def all_changed_files + Set.new + .merge(git.added_files.to_a) + .merge(git.modified_files.to_a) + .merge(git.renamed_files.map { |x| x[:after] }) + .subtract(git.renamed_files.map { |x| x[:before] }) + .to_a + .sort + end + + def ee? + ENV['CI_PROJECT_NAME'] == 'gitlab-ee' || File.exist?('../../CHANGELOG-EE.md') + end + + def project_name + ee? ? 'gitlab-ee' : 'gitlab-ce' + end + + # Looks up the current list of GitLab team members and parses it into a + # useful form + # + # @return [Array<Teammate>] + def team + @team ||= + begin + rsp = Net::HTTP.get_response(ROULETTE_DATA_URL) + raise "Failed to read #{ROULETTE_DATA_URL}: #{rsp.code} #{rsp.message}" unless + rsp.is_a?(Net::HTTPSuccess) + + data = JSON.parse(rsp.body) + data.map { |hash| ::Gitlab::Danger::Teammate.new(hash) } + rescue JSON::ParserError + raise "Failed to parse JSON response from #{ROULETTE_DATA_URL}" + end + end + + # Like +team+, but only returns teammates in the current project, based on + # project_name. + # + # @return [Array<Teammate>] + def project_team + team.select { |member| member.in_project?(project_name) } + end + + # @return [Hash<String,Array<String>>] + def changes_by_category + all_changed_files.each_with_object(Hash.new { |h, k| h[k] = [] }) do |file, hash| + hash[category_for_file(file)] << file + end + end + + # Determines the category a file is in, e.g., `:frontend` or `:backend` + # @return[Symbol] + def category_for_file(file) + _, category = CATEGORIES.find { |regexp, _| regexp.match?(file) } + + category || :unknown + end + + # Returns the GFM for a category label, making its best guess if it's not + # a category we know about. + # + # @return[String] + def label_for_category(category) + CATEGORY_LABELS.fetch(category, "~#{category}") + end + + CATEGORY_LABELS = { + docs: "~Documentation", + qa: "~QA" + }.freeze + + # rubocop:disable Style/RegexpLiteral + CATEGORIES = { + %r{\Adoc/} => :docs, + %r{\A(CONTRIBUTING|LICENSE|MAINTENANCE|PHILOSOPHY|PROCESS|README)(\.md)?\z} => :docs, + + %r{\A(ee/)?app/(assets|views)/} => :frontend, + %r{\A(ee/)?public/} => :frontend, + %r{\A(ee/)?spec/javascripts/} => :frontend, + %r{\A(ee/)?vendor/assets/} => :frontend, + %r{\A(jest\.config\.js|package\.json|yarn\.lock)\z} => :frontend, + + %r{\A(ee/)?app/(?!assets|views)[^/]+} => :backend, + %r{\A(ee/)?(bin|config|danger|generator_templates|lib|rubocop|scripts)/} => :backend, + %r{\A(ee/)?spec/(?!javascripts)[^/]+} => :backend, + %r{\A(ee/)?vendor/(?!assets)[^/]+} => :backend, + %r{\A(ee/)?vendor/(languages\.yml|licenses\.csv)\z} => :backend, + %r{\A(Dangerfile|Gemfile|Gemfile.lock|Procfile|Rakefile)\z} => :backend, + %r{\A[A-Z_]+_VERSION\z} => :backend, + + %r{\A(ee/)?db/} => :database, + %r{\A(ee/)?qa/} => :qa, + + # Fallbacks in case the above patterns miss anything + %r{\.rb\z} => :backend, + %r{\.(md|txt)\z} => :docs, + %r{\.js\z} => :frontend + }.freeze + # rubocop:enable Style/RegexpLiteral + end + end +end diff --git a/lib/gitlab/danger/teammate.rb b/lib/gitlab/danger/teammate.rb new file mode 100644 index 00000000000..4b822aa86c5 --- /dev/null +++ b/lib/gitlab/danger/teammate.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +module Gitlab + module Danger + class Teammate + attr_reader :name, :username, :projects + + def initialize(options = {}) + @name = options['name'] + @username = options['username'] + @projects = options['projects'] + end + + def markdown_name + "[#{name}](https://gitlab.com/#{username}) (`@#{username}`)" + end + + def in_project?(name) + projects&.has_key?(name) + end + + # Traintainers also count as reviewers + def reviewer?(project, category) + capabilities(project) == "reviewer #{category}" || traintainer?(project, category) + end + + def traintainer?(project, category) + capabilities(project) == "trainee_maintainer #{category}" + end + + def maintainer?(project, category) + capabilities(project) == "maintainer #{category}" + end + + private + + def capabilities(project) + projects.fetch(project, '') + end + end + end +end diff --git a/lib/gitlab/middleware/rails_queue_duration.rb b/lib/gitlab/middleware/rails_queue_duration.rb index 96c6a0a7d28..a147e165262 100644 --- a/lib/gitlab/middleware/rails_queue_duration.rb +++ b/lib/gitlab/middleware/rails_queue_duration.rb @@ -7,6 +7,8 @@ module Gitlab module Middleware class RailsQueueDuration + GITLAB_RAILS_QUEUE_DURATION_KEY = 'GITLAB_RAILS_QUEUE_DURATION' + def initialize(app) @app = app end @@ -19,6 +21,7 @@ module Gitlab duration = Time.now.to_f * 1_000 - proxy_start.to_f / 1_000_000 trans.set(:rails_queue_duration, duration) metric_rails_queue_duration_seconds.observe(trans.labels, duration / 1_000) + env[GITLAB_RAILS_QUEUE_DURATION_KEY] = duration.round(2) end @app.call(env) diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index 6bfcf83f388..a65f4a8639c 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -90,8 +90,14 @@ module Gitlab todos: count(Todo), uploads: count(Upload), web_hooks: count(WebHook) - }.merge(services_usage).merge(approximate_counts) - } + } + .merge(services_usage) + .merge(approximate_counts) + }.tap do |data| + if Feature.enabled?(:group_overview_security_dashboard) + data[:counts][:user_preferences] = user_preferences_usage + end + end end # rubocop: enable CodeReuse/ActiveRecord @@ -159,6 +165,10 @@ module Gitlab } end + def user_preferences_usage + {} # augmented in EE + end + def count(relation, fallback: -1) relation.count rescue ActiveRecord::StatementInvalid diff --git a/lib/gitlab/wiki_file_finder.rb b/lib/gitlab/wiki_file_finder.rb index 5303b3582ab..e9be6db50da 100644 --- a/lib/gitlab/wiki_file_finder.rb +++ b/lib/gitlab/wiki_file_finder.rb @@ -2,8 +2,6 @@ module Gitlab class WikiFileFinder < FileFinder - BATCH_SIZE = 100 - attr_reader :repository def initialize(project, ref) @@ -19,7 +17,7 @@ module Gitlab safe_query = Regexp.new(safe_query, Regexp::IGNORECASE) filenames = repository.ls_files(ref) - filenames.grep(safe_query).first(BATCH_SIZE) + filenames.grep(safe_query) end end end diff --git a/spec/controllers/concerns/send_file_upload_spec.rb b/spec/controllers/concerns/send_file_upload_spec.rb index a07113a6156..cf3b24f50a3 100644 --- a/spec/controllers/concerns/send_file_upload_spec.rb +++ b/spec/controllers/concerns/send_file_upload_spec.rb @@ -52,6 +52,23 @@ describe SendFileUpload do end end + context 'with inline image' do + let(:filename) { 'test.png' } + let(:params) { { disposition: 'inline', attachment: filename } } + + it 'sends a file with inline disposition' do + # Notice the filename= is omitted from the disposition; this is because + # Rails 5 will append this header in send_file + expected_params = { + filename: 'test.png', + disposition: "inline; filename*=UTF-8''test.png" + } + expect(controller).to receive(:send_file).with(uploader.path, expected_params) + + subject + end + end + context 'with attachment' do let(:filename) { 'test.js' } let(:params) { { attachment: filename } } diff --git a/spec/controllers/help_controller_spec.rb b/spec/controllers/help_controller_spec.rb index 5cb284e7e2d..dca67c18caa 100644 --- a/spec/controllers/help_controller_spec.rb +++ b/spec/controllers/help_controller_spec.rb @@ -37,6 +37,46 @@ describe HelpController do expect(assigns[:help_index]).to eq '[external](https://some.external.link)' end end + + context 'when relative url with external on same line' do + it 'prefix it with /help/' do + stub_readme("[API](api/README.md) [external](https://some.external.link)") + + get :index + + expect(assigns[:help_index]).to eq '[API](/help/api/README.md) [external](https://some.external.link)' + end + end + + context 'when relative url with http:// in query' do + it 'prefix it with /help/' do + stub_readme("[API](api/README.md?go=https://example.com/)") + + get :index + + expect(assigns[:help_index]).to eq '[API](/help/api/README.md?go=https://example.com/)' + end + end + + context 'when mailto URL' do + it 'do not change it' do + stub_readme("[report bug](mailto:bugs@example.com)") + + get :index + + expect(assigns[:help_index]).to eq '[report bug](mailto:bugs@example.com)' + end + end + + context 'when protocol-relative link' do + it 'do not change it' do + stub_readme("[protocol-relative](//example.com)") + + get :index + + expect(assigns[:help_index]).to eq '[protocol-relative](//example.com)' + end + end end describe 'GET #show' do diff --git a/spec/features/dashboard/todos/todos_spec.rb b/spec/features/dashboard/todos/todos_spec.rb index 51f158d3045..fd8677feab5 100644 --- a/spec/features/dashboard/todos/todos_spec.rb +++ b/spec/features/dashboard/todos/todos_spec.rb @@ -126,7 +126,7 @@ describe 'Dashboard Todos' do it 'shows you added a todo message' do page.within('.js-todos-all') do - expect(page).to have_content("You added a todo for issue #{issue.to_reference(full: true)}") + expect(page).to have_content("You added a todo for issue #{issue.to_reference(full: true)}") expect(page).not_to have_content('to yourself') end end @@ -140,7 +140,7 @@ describe 'Dashboard Todos' do it 'shows you mentioned yourself message' do page.within('.js-todos-all') do - expect(page).to have_content("You mentioned yourself on issue #{issue.to_reference(full: true)}") + expect(page).to have_content("You mentioned yourself on issue #{issue.to_reference(full: true)}") expect(page).not_to have_content('to yourself') end end @@ -154,7 +154,7 @@ describe 'Dashboard Todos' do it 'shows you directly addressed yourself message' do page.within('.js-todos-all') do - expect(page).to have_content("You directly addressed yourself on issue #{issue.to_reference(full: true)}") + expect(page).to have_content("You directly addressed yourself on issue #{issue.to_reference(full: true)}") expect(page).not_to have_content('to yourself') end end @@ -170,7 +170,7 @@ describe 'Dashboard Todos' do it 'shows you set yourself as an approver message' do page.within('.js-todos-all') do - expect(page).to have_content("You set yourself as an approver for merge request #{merge_request.to_reference(full: true)}") + expect(page).to have_content("You set yourself as an approver for merge request #{merge_request.to_reference(full: true)}") expect(page).not_to have_content('to yourself') end end diff --git a/spec/fixtures/api/schemas/public_api/v4/group_labels.json b/spec/fixtures/api/schemas/public_api/v4/group_labels.json index f6c327abfdd..fbde45f2904 100644 --- a/spec/fixtures/api/schemas/public_api/v4/group_labels.json +++ b/spec/fixtures/api/schemas/public_api/v4/group_labels.json @@ -6,6 +6,7 @@ "id" : { "type": "integer" }, "name" : { "type": "string "}, "color" : { "type": "string "}, + "text_color" : { "type": "string "}, "description" : { "type": "string "}, "open_issues_count" : { "type": "integer "}, "closed_issues_count" : { "type": "integer "}, diff --git a/spec/fixtures/api/schemas/public_api/v4/milestone.json b/spec/fixtures/api/schemas/public_api/v4/milestone.json index 6ca2e88ae91..971f7980f46 100644 --- a/spec/fixtures/api/schemas/public_api/v4/milestone.json +++ b/spec/fixtures/api/schemas/public_api/v4/milestone.json @@ -8,6 +8,7 @@ "title": { "type": "string" }, "description": { "type": ["string", "null"] }, "state": { "type": "string" }, + "percentage_complete": { "type": "integer" }, "created_at": { "type": "date" }, "updated_at": { "type": "date" }, "start_date": { "type": "date" }, diff --git a/spec/javascripts/diffs/store/utils_spec.js b/spec/javascripts/diffs/store/utils_spec.js index c5e413a29d8..baf6e111f9f 100644 --- a/spec/javascripts/diffs/store/utils_spec.js +++ b/spec/javascripts/diffs/store/utils_spec.js @@ -14,7 +14,7 @@ import { MERGE_REQUEST_NOTEABLE_TYPE } from '~/notes/constants'; import diffFileMockData from '../mock_data/diff_file'; import { noteableDataMock } from '../../notes/mock_data'; -const getDiffFileMock = () => Object.assign({}, diffFileMockData); +const getDiffFileMock = () => JSON.parse(JSON.stringify(diffFileMockData)); describe('DiffsStoreUtils', () => { describe('findDiffFile', () => { @@ -80,30 +80,44 @@ describe('DiffsStoreUtils', () => { }); describe('addContextLines', () => { - it('should add context lines properly with bottom parameter', () => { + it('should add context lines', () => { const diffFile = getDiffFileMock(); const inlineLines = diffFile.highlighted_diff_lines; const parallelLines = diffFile.parallel_diff_lines; const lineNumbers = { oldLineNumber: 3, newLineNumber: 5 }; - const contextLines = [{ lineNumber: 42 }]; - const options = { inlineLines, parallelLines, contextLines, lineNumbers, bottom: true }; + const contextLines = [{ lineNumber: 42, line_code: '123' }]; + const options = { inlineLines, parallelLines, contextLines, lineNumbers }; const inlineIndex = utils.findIndexInInlineLines(inlineLines, lineNumbers); const parallelIndex = utils.findIndexInParallelLines(parallelLines, lineNumbers); const normalizedParallelLine = { left: options.contextLines[0], right: options.contextLines[0], + line_code: '123', }; utils.addContextLines(options); - expect(inlineLines[inlineLines.length - 1]).toEqual(contextLines[0]); - expect(parallelLines[parallelLines.length - 1]).toEqual(normalizedParallelLine); + expect(inlineLines[inlineIndex]).toEqual(contextLines[0]); + expect(parallelLines[parallelIndex]).toEqual(normalizedParallelLine); + }); + + it('should add context lines properly with bottom parameter', () => { + const diffFile = getDiffFileMock(); + const inlineLines = diffFile.highlighted_diff_lines; + const parallelLines = diffFile.parallel_diff_lines; + const lineNumbers = { oldLineNumber: 3, newLineNumber: 5 }; + const contextLines = [{ lineNumber: 42, line_code: '123' }]; + const options = { inlineLines, parallelLines, contextLines, lineNumbers, bottom: true }; + const normalizedParallelLine = { + left: options.contextLines[0], + right: options.contextLines[0], + line_code: '123', + }; - delete options.bottom; utils.addContextLines(options); - expect(inlineLines[inlineIndex]).toEqual(contextLines[0]); - expect(parallelLines[parallelIndex]).toEqual(normalizedParallelLine); + expect(inlineLines[inlineLines.length - 1]).toEqual(contextLines[0]); + expect(parallelLines[parallelLines.length - 1]).toEqual(normalizedParallelLine); }); }); diff --git a/spec/javascripts/notes/components/noteable_note_spec.js b/spec/javascripts/notes/components/noteable_note_spec.js index 8ade6fc2ced..9420713ceca 100644 --- a/spec/javascripts/notes/components/noteable_note_spec.js +++ b/spec/javascripts/notes/components/noteable_note_spec.js @@ -1,86 +1,138 @@ -import $ from 'jquery'; import _ from 'underscore'; -import Vue from 'vue'; +import { shallowMount, createLocalVue } from '@vue/test-utils'; import createStore from '~/notes/stores'; import issueNote from '~/notes/components/noteable_note.vue'; +import NoteHeader from '~/notes/components/note_header.vue'; +import UserAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue'; +import NoteActions from '~/notes/components/note_actions.vue'; +import NoteBody from '~/notes/components/note_body.vue'; import { noteableDataMock, notesDataMock, note } from '../mock_data'; describe('issue_note', () => { let store; - let vm; + let wrapper; beforeEach(() => { - const Component = Vue.extend(issueNote); - store = createStore(); store.dispatch('setNoteableData', noteableDataMock); store.dispatch('setNotesData', notesDataMock); - vm = new Component({ + const localVue = createLocalVue(); + wrapper = shallowMount(issueNote, { store, propsData: { note, }, - }).$mount(); + sync: false, + localVue, + }); }); afterEach(() => { - vm.$destroy(); + wrapper.destroy(); }); it('should render user information', () => { - expect(vm.$el.querySelector('.user-avatar-link img').getAttribute('src')).toEqual( - note.author.avatar_url, - ); + const { author } = note; + const avatar = wrapper.find(UserAvatarLink); + const avatarProps = avatar.props(); + + expect(avatarProps.linkHref).toBe(author.path); + expect(avatarProps.imgSrc).toBe(author.avatar_url); + expect(avatarProps.imgAlt).toBe(author.name); + expect(avatarProps.imgSize).toBe(40); }); it('should render note header content', () => { - const el = vm.$el.querySelector('.note-header .note-header-author-name'); + const noteHeader = wrapper.find(NoteHeader); + const noteHeaderProps = noteHeader.props(); - expect(el.textContent.trim()).toEqual(note.author.name); + expect(noteHeaderProps.author).toEqual(note.author); + expect(noteHeaderProps.createdAt).toEqual(note.created_at); + expect(noteHeaderProps.noteId).toEqual(note.id); }); it('should render note actions', () => { - expect(vm.$el.querySelector('.note-actions')).toBeDefined(); + const { author } = note; + const noteActions = wrapper.find(NoteActions); + const noteActionsProps = noteActions.props(); + + expect(noteActionsProps.authorId).toBe(author.id); + expect(noteActionsProps.noteId).toBe(note.id); + expect(noteActionsProps.noteUrl).toBe(note.noteable_note_url); + expect(noteActionsProps.accessLevel).toBe(note.human_access); + expect(noteActionsProps.canEdit).toBe(note.current_user.can_edit); + expect(noteActionsProps.canAwardEmoji).toBe(note.current_user.can_award_emoji); + expect(noteActionsProps.canDelete).toBe(note.current_user.can_edit); + expect(noteActionsProps.canReportAsAbuse).toBe(true); + expect(noteActionsProps.canResolve).toBe(false); + expect(noteActionsProps.reportAbusePath).toBe(note.report_abuse_path); + expect(noteActionsProps.resolvable).toBe(false); + expect(noteActionsProps.isResolved).toBe(false); + expect(noteActionsProps.isResolving).toBe(false); + expect(noteActionsProps.resolvedBy).toEqual({}); }); it('should render issue body', () => { - expect(vm.$el.querySelector('.note-text').innerHTML).toEqual(note.note_html); + const noteBody = wrapper.find(NoteBody); + const noteBodyProps = noteBody.props(); + + expect(noteBodyProps.note).toEqual(note); + expect(noteBodyProps.line).toBe(null); + expect(noteBodyProps.canEdit).toBe(note.current_user.can_edit); + expect(noteBodyProps.isEditing).toBe(false); + expect(noteBodyProps.helpPagePath).toBe(''); }); it('prevents note preview xss', done => { const imgSrc = 'data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7'; const noteBody = `<img src="${imgSrc}" onload="alert(1)" />`; const alertSpy = spyOn(window, 'alert'); - vm.updateNote = () => new Promise($.noop); + store.hotUpdate({ + actions: { + updateNote() {}, + }, + }); + const noteBodyComponent = wrapper.find(NoteBody); - vm.formUpdateHandler(noteBody, null, $.noop); + noteBodyComponent.vm.$emit('handleFormUpdate', noteBody, null, () => {}); setTimeout(() => { expect(alertSpy).not.toHaveBeenCalled(); - expect(vm.note.note_html).toEqual(_.escape(noteBody)); + expect(wrapper.vm.note.note_html).toEqual(_.escape(noteBody)); done(); }, 0); }); describe('cancel edit', () => { it('restores content of updated note', done => { - const noteBody = 'updated note text'; - vm.updateNote = () => Promise.resolve(); - - vm.formUpdateHandler(noteBody, null, $.noop); - - setTimeout(() => { - expect(vm.note.note_html).toEqual(noteBody); - - vm.formCancelHandler(); - - setTimeout(() => { - expect(vm.note.note_html).toEqual(noteBody); - - done(); - }); + const updatedText = 'updated note text'; + store.hotUpdate({ + actions: { + updateNote() {}, + }, }); + const noteBody = wrapper.find(NoteBody); + noteBody.vm.resetAutoSave = () => {}; + + noteBody.vm.$emit('handleFormUpdate', updatedText, null, () => {}); + + wrapper.vm + .$nextTick() + .then(() => { + const noteBodyProps = noteBody.props(); + + expect(noteBodyProps.note.note_html).toBe(updatedText); + noteBody.vm.$emit('cancelForm'); + }) + .then(() => wrapper.vm.$nextTick()) + .then(() => { + const noteBodyProps = noteBody.props(); + + expect(noteBodyProps.note.note_html).toBe(note.note_html); + }) + .then(done) + .catch(done.fail); }); }); }); diff --git a/spec/javascripts/vue_shared/components/user_avatar/user_avatar_link_spec.js b/spec/javascripts/vue_shared/components/user_avatar/user_avatar_link_spec.js index f2472fd377c..80aa75847ae 100644 --- a/spec/javascripts/vue_shared/components/user_avatar/user_avatar_link_spec.js +++ b/spec/javascripts/vue_shared/components/user_avatar/user_avatar_link_spec.js @@ -1,13 +1,14 @@ import _ from 'underscore'; import Vue from 'vue'; import UserAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue'; +import { TEST_HOST } from 'spec/test_constants'; describe('User Avatar Link Component', function() { beforeEach(function() { this.propsData = { - linkHref: 'myavatarurl.com', + linkHref: `${TEST_HOST}/myavatarurl.com`, imgSize: 99, - imgSrc: 'myavatarurl.com', + imgSrc: `${TEST_HOST}/myavatarurl.com`, imgAlt: 'mydisplayname', imgCssClasses: 'myextraavatarclass', tooltipText: 'tooltip text', @@ -37,11 +38,18 @@ describe('User Avatar Link Component', function() { }); it('should render <a> as a child element', function() { - expect(this.userAvatarLink.$el.tagName).toBe('A'); + const link = this.userAvatarLink.$el; + + expect(link.tagName).toBe('A'); + expect(link.href).toBe(this.propsData.linkHref); }); - it('should have <img> as a child element', function() { - expect(this.userAvatarLink.$el.querySelector('img')).not.toBeNull(); + it('renders imgSrc with imgSize as image', function() { + const { imgSrc, imgSize } = this.propsData; + const image = this.userAvatarLink.$el.querySelector('img'); + + expect(image).not.toBeNull(); + expect(image.src).toBe(`${imgSrc}?width=${imgSize}`); }); it('should return necessary props as defined', function() { diff --git a/spec/lib/gitlab/ci/build/policy/changes_spec.rb b/spec/lib/gitlab/ci/build/policy/changes_spec.rb index 5fee37bb43e..dc3329061d1 100644 --- a/spec/lib/gitlab/ci/build/policy/changes_spec.rb +++ b/spec/lib/gitlab/ci/build/policy/changes_spec.rb @@ -73,9 +73,9 @@ describe Gitlab::Ci::Build::Policy::Changes do expect(policy).not_to be_satisfied_by(pipeline, seed) end - context 'when pipelines does not run for a branch update' do + context 'when modified paths can not be evaluated' do before do - pipeline.before_sha = Gitlab::Git::BLANK_SHA + allow(pipeline).to receive(:modified_paths) { nil } end it 'is always satisfied' do @@ -115,5 +115,57 @@ describe Gitlab::Ci::Build::Policy::Changes do expect(policy).not_to be_satisfied_by(pipeline, seed) end end + + context 'when branch is created' do + let(:pipeline) do + create(:ci_empty_pipeline, project: project, + ref: 'feature', + source: source, + sha: '0b4bc9a4', + before_sha: Gitlab::Git::BLANK_SHA, + merge_request: merge_request) + end + + let(:ci_build) do + build(:ci_build, pipeline: pipeline, project: project, ref: 'feature') + end + + let(:seed) { double('build seed', to_resource: ci_build) } + + context 'when source is merge request' do + let(:source) { :merge_request } + + let(:merge_request) do + create(:merge_request, + source_project: project, + source_branch: 'feature', + target_project: project, + target_branch: 'master') + end + + it 'is satified by changes in the merge request' do + policy = described_class.new(%w[files/ruby/feature.rb]) + + expect(policy).to be_satisfied_by(pipeline, seed) + end + + it 'is not satified by changes not in the merge request' do + policy = described_class.new(%w[foo.rb]) + + expect(policy).not_to be_satisfied_by(pipeline, seed) + end + end + + context 'when source is push' do + let(:source) { :push } + let(:merge_request) { nil } + + it 'is always satified' do + policy = described_class.new(%w[foo.rb]) + + expect(policy).to be_satisfied_by(pipeline, seed) + end + end + end end end diff --git a/spec/lib/gitlab/danger/helper_spec.rb b/spec/lib/gitlab/danger/helper_spec.rb new file mode 100644 index 00000000000..75080aacd96 --- /dev/null +++ b/spec/lib/gitlab/danger/helper_spec.rb @@ -0,0 +1,294 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rspec-parameterized' +require 'webmock/rspec' + +require 'gitlab/danger/helper' + +describe Gitlab::Danger::Helper do + using RSpec::Parameterized::TableSyntax + + class FakeDanger + include Gitlab::Danger::Helper + + attr_reader :git + + def initialize(git:) + @git = git + end + end + + let(:teammate_json) do + <<~JSON + [ + { + "username": "in-gitlab-ce", + "name": "CE maintainer", + "projects":{ "gitlab-ce": "maintainer backend" } + }, + { + "username": "in-gitlab-ee", + "name": "EE reviewer", + "projects":{ "gitlab-ee": "reviewer frontend" } + } + ] + JSON + end + + let(:ce_teammate_matcher) do + satisfy do |teammate| + teammate.username == 'in-gitlab-ce' && + teammate.name == 'CE maintainer' && + teammate.projects == { 'gitlab-ce' => 'maintainer backend' } + end + end + + let(:ee_teammate_matcher) do + satisfy do |teammate| + teammate.username == 'in-gitlab-ee' && + teammate.name == 'EE reviewer' && + teammate.projects == { 'gitlab-ee' => 'reviewer frontend' } + end + end + + let(:fake_git) { double('fake-git') } + + subject(:helper) { FakeDanger.new(git: fake_git) } + + describe '#all_changed_files' do + subject { helper.all_changed_files } + + it 'interprets a list of changes from the danger git plugin' do + expect(fake_git).to receive(:added_files) { %w[a b c.old] } + expect(fake_git).to receive(:modified_files) { %w[d e] } + expect(fake_git) + .to receive(:renamed_files) + .at_least(:once) + .and_return([{ before: 'c.old', after: 'c.new' }]) + + is_expected.to contain_exactly('a', 'b', 'c.new', 'd', 'e') + end + end + + describe '#ee?' do + subject { helper.ee? } + + it 'returns true if CI_PROJECT_NAME if set to gitlab-ee' do + stub_env('CI_PROJECT_NAME', 'gitlab-ee') + expect(File).not_to receive(:exist?) + + is_expected.to be_truthy + end + + it 'delegates to CHANGELOG-EE.md existence if CI_PROJECT_NAME is set to something else' do + stub_env('CI_PROJECT_NAME', 'something else') + expect(File).to receive(:exist?).with('../../CHANGELOG-EE.md') { true } + + is_expected.to be_truthy + end + + it 'returns true if CHANGELOG-EE.md exists' do + stub_env('CI_PROJECT_NAME', nil) + expect(File).to receive(:exist?).with('../../CHANGELOG-EE.md') { true } + + is_expected.to be_truthy + end + + it "returns false if CHANGELOG-EE.md doesn't exist" do + stub_env('CI_PROJECT_NAME', nil) + expect(File).to receive(:exist?).with('../../CHANGELOG-EE.md') { false } + + is_expected.to be_falsy + end + end + + describe '#project_name' do + subject { helper.project_name } + + it 'returns gitlab-ee if ee? returns true' do + expect(helper).to receive(:ee?) { true } + + is_expected.to eq('gitlab-ee') + end + + it 'returns gitlab-ce if ee? returns false' do + expect(helper).to receive(:ee?) { false } + + is_expected.to eq('gitlab-ce') + end + end + + describe '#team' do + subject(:team) { helper.team } + + context 'HTTP failure' do + before do + WebMock + .stub_request(:get, 'https://about.gitlab.com/roulette.json') + .to_return(status: 404) + end + + it 'raises a pretty error' do + expect { team }.to raise_error(/Failed to read/) + end + end + + context 'JSON failure' do + before do + WebMock + .stub_request(:get, 'https://about.gitlab.com/roulette.json') + .to_return(body: 'INVALID JSON') + end + + it 'raises a pretty error' do + expect { team }.to raise_error(/Failed to parse/) + end + end + + context 'success' do + before do + WebMock + .stub_request(:get, 'https://about.gitlab.com/roulette.json') + .to_return(body: teammate_json) + end + + it 'returns an array of teammates' do + is_expected.to contain_exactly(ce_teammate_matcher, ee_teammate_matcher) + end + + it 'memoizes the result' do + expect(team.object_id).to eq(helper.team.object_id) + end + end + end + + describe '#project_team' do + subject { helper.project_team } + + before do + WebMock + .stub_request(:get, 'https://about.gitlab.com/roulette.json') + .to_return(body: teammate_json) + end + + it 'filters team by project_name' do + expect(helper) + .to receive(:project_name) + .at_least(:once) + .and_return('gitlab-ce') + + is_expected.to contain_exactly(ce_teammate_matcher) + end + end + + describe '#changes_by_category' do + it 'categorizes changed files' do + expect(fake_git).to receive(:added_files) { %w[foo foo.md foo.rb foo.js db/foo qa/foo] } + allow(fake_git).to receive(:modified_files) { [] } + allow(fake_git).to receive(:renamed_files) { [] } + + expect(helper.changes_by_category).to eq( + backend: %w[foo.rb], + database: %w[db/foo], + docs: %w[foo.md], + frontend: %w[foo.js], + qa: %w[qa/foo], + unknown: %w[foo] + ) + end + end + + describe '#category_for_file' do + where(:path, :expected_category) do + 'doc/foo' | :docs + 'CONTRIBUTING.md' | :docs + 'LICENSE' | :docs + 'MAINTENANCE.md' | :docs + 'PHILOSOPHY.md' | :docs + 'PROCESS.md' | :docs + 'README.md' | :docs + + 'ee/doc/foo' | :unknown + 'ee/README' | :unknown + + 'app/assets/foo' | :frontend + 'app/views/foo' | :frontend + 'public/foo' | :frontend + 'spec/javascripts/foo' | :frontend + 'vendor/assets/foo' | :frontend + 'jest.config.js' | :frontend + 'package.json' | :frontend + 'yarn.lock' | :frontend + + 'ee/app/assets/foo' | :frontend + 'ee/app/views/foo' | :frontend + 'ee/spec/javascripts/foo' | :frontend + + 'app/models/foo' | :backend + 'bin/foo' | :backend + 'config/foo' | :backend + 'danger/foo' | :backend + 'lib/foo' | :backend + 'rubocop/foo' | :backend + 'scripts/foo' | :backend + 'spec/foo' | :backend + 'spec/foo/bar' | :backend + + 'ee/app/foo' | :backend + 'ee/bin/foo' | :backend + 'ee/spec/foo' | :backend + 'ee/spec/foo/bar' | :backend + + 'generator_templates/foo' | :backend + 'vendor/languages.yml' | :backend + 'vendor/licenses.csv' | :backend + + 'Dangerfile' | :backend + 'Gemfile' | :backend + 'Gemfile.lock' | :backend + 'Procfile' | :backend + 'Rakefile' | :backend + 'FOO_VERSION' | :backend + + 'ee/FOO_VERSION' | :unknown + + 'db/foo' | :database + 'qa/foo' | :qa + + 'ee/db/foo' | :database + 'ee/qa/foo' | :qa + + 'FOO' | :unknown + 'foo' | :unknown + + 'foo/bar.rb' | :backend + 'foo/bar.js' | :frontend + 'foo/bar.txt' | :docs + 'foo/bar.md' | :docs + end + + with_them do + subject { helper.category_for_file(path) } + + it { is_expected.to eq(expected_category) } + end + end + + describe '#label_for_category' do + where(:category, :expected_label) do + :backend | '~backend' + :database | '~database' + :docs | '~Documentation' + :foo | '~foo' + :frontend | '~frontend' + :qa | '~QA' + end + + with_them do + subject { helper.label_for_category(category) } + + it { is_expected.to eq(expected_label) } + end + end +end diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index 4f5993ba226..d3eae80cc56 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -124,9 +124,15 @@ describe Gitlab::UsageData do todos uploads web_hooks + user_preferences )) end + it 'does not gather user preferences usage data when the feature is disabled' do + stub_feature_flags(group_overview_security_dashboard: false) + expect(subject[:counts].keys).not_to include(:user_preferences) + end + it 'gathers projects data correctly' do count_data = subject[:counts] diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 72a0df96a80..460b5c8cd31 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -1172,8 +1172,26 @@ describe Ci::Pipeline, :mailer do pipeline.update_column(:before_sha, Gitlab::Git::BLANK_SHA) end - it 'raises an error' do - expect { pipeline.modified_paths }.to raise_error(ArgumentError) + it 'returns nil' do + expect(pipeline.modified_paths).to be_nil + end + end + + context 'when source is merge request' do + let(:pipeline) do + create(:ci_pipeline, source: :merge_request, merge_request: merge_request) + end + + let(:merge_request) do + create(:merge_request, + source_project: project, + source_branch: 'feature', + target_project: project, + target_branch: 'master') + end + + it 'returns merge request modified paths' do + expect(pipeline.modified_paths).to match(merge_request.modified_paths) end end end diff --git a/spec/models/releases/link_spec.rb b/spec/models/releases/link_spec.rb index 06ed1438688..4dd26c976cc 100644 --- a/spec/models/releases/link_spec.rb +++ b/spec/models/releases/link_spec.rb @@ -77,4 +77,28 @@ describe Releases::Link do it { is_expected.to be_truthy } end + + describe 'supported protocols' do + where(:protocol) do + %w(http https ftp) + end + + with_them do + let(:link) { build(:release_link, url: protocol + '://assets.com/download') } + + it 'will be valid' do + expect(link).to be_valid + end + end + end + + describe 'unsupported protocol' do + context 'for torrent' do + let(:link) { build(:release_link, url: 'torrent://assets.com/download') } + + it 'will be invalid' do + expect(link).to be_invalid + end + end + end end diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index 6b9bc6eda6a..bc892523cf8 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -237,7 +237,7 @@ describe API::Commits do end describe 'create' do - let(:message) { 'Created file' } + let(:message) { 'Created a new file with a very very looooooooooooooooooooooooooooooooooooooooooooooong commit message' } let(:invalid_c_params) do { branch: 'master', diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb index 49eea2e362b..606fa9185d8 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -20,7 +20,7 @@ describe API::Labels do create(:labeled_merge_request, labels: [priority_label], author: user, source_project: project ) expected_keys = %w( - id name color description + id name color text_color description open_issues_count closed_issues_count open_merge_requests_count subscribed priority ) @@ -43,6 +43,7 @@ describe API::Labels do expect(label1_response['open_merge_requests_count']).to eq(0) expect(label1_response['name']).to eq(label1.name) expect(label1_response['color']).to be_present + expect(label1_response['text_color']).to be_present expect(label1_response['description']).to be_nil expect(label1_response['priority']).to be_nil expect(label1_response['subscribed']).to be_falsey @@ -52,6 +53,7 @@ describe API::Labels do expect(group_label_response['open_merge_requests_count']).to eq(0) expect(group_label_response['name']).to eq(group_label.name) expect(group_label_response['color']).to be_present + expect(group_label_response['text_color']).to be_present expect(group_label_response['description']).to be_nil expect(group_label_response['priority']).to be_nil expect(group_label_response['subscribed']).to be_falsey @@ -61,6 +63,7 @@ describe API::Labels do expect(priority_label_response['open_merge_requests_count']).to eq(1) expect(priority_label_response['name']).to eq(priority_label.name) expect(priority_label_response['color']).to be_present + expect(priority_label_response['text_color']).to be_present expect(priority_label_response['description']).to be_nil expect(priority_label_response['priority']).to eq(3) expect(priority_label_response['subscribed']).to be_falsey diff --git a/spec/requests/api/wikis_spec.rb b/spec/requests/api/wikis_spec.rb index 6109829aad1..d1b58aac104 100644 --- a/spec/requests/api/wikis_spec.rb +++ b/spec/requests/api/wikis_spec.rb @@ -100,6 +100,8 @@ describe API::Wikis do shared_examples_for 'updates wiki page' do it 'updates the wiki page' do + put(api(url, user), params: payload) + expect(response).to have_gitlab_http_status(200) expect(json_response.size).to eq(4) expect(json_response.keys).to match_array(expected_keys_with_content) @@ -107,6 +109,16 @@ describe API::Wikis do expect(json_response['slug']).to eq(payload[:title].tr(' ', '-')) expect(json_response['title']).to eq(payload[:title]) end + + [:title, :content, :format].each do |part| + it "it updates with wiki with missing #{part}" do + payload.delete(part) + + put(api(url, user), params: payload) + + expect(response).to have_gitlab_http_status(200) + end + end end shared_examples_for '403 Forbidden' do @@ -528,8 +540,6 @@ describe API::Wikis do context 'when user is developer' do before do project.add_developer(user) - - put(api(url, user), params: payload) end include_examples 'updates wiki page' @@ -537,6 +547,10 @@ describe API::Wikis do context 'when page is not existing' do let(:url) { "/projects/#{project.id}/wikis/unknown" } + before do + put(api(url, user), params: payload) + end + include_examples '404 Wiki Page Not Found' end end @@ -544,8 +558,6 @@ describe API::Wikis do context 'when user is maintainer' do before do project.add_maintainer(user) - - put(api(url, user), params: payload) end include_examples 'updates wiki page' @@ -553,6 +565,10 @@ describe API::Wikis do context 'when page is not existing' do let(:url) { "/projects/#{project.id}/wikis/unknown" } + before do + put(api(url, user), params: payload) + end + include_examples '404 Wiki Page Not Found' end end @@ -572,8 +588,6 @@ describe API::Wikis do context 'when user is developer' do before do project.add_developer(user) - - put(api(url, user), params: payload) end include_examples 'updates wiki page' @@ -581,6 +595,10 @@ describe API::Wikis do context 'when page is not existing' do let(:url) { "/projects/#{project.id}/wikis/unknown" } + before do + put(api(url, user), params: payload) + end + include_examples '404 Wiki Page Not Found' end end @@ -588,8 +606,6 @@ describe API::Wikis do context 'when user is maintainer' do before do project.add_maintainer(user) - - put(api(url, user), params: payload) end include_examples 'updates wiki page' @@ -597,6 +613,10 @@ describe API::Wikis do context 'when page is not existing' do let(:url) { "/projects/#{project.id}/wikis/unknown" } + before do + put(api(url, user), params: payload) + end + include_examples '404 Wiki Page Not Found' end end @@ -605,10 +625,6 @@ describe API::Wikis do context 'when wiki belongs to a group project' do let(:project) { create(:project, :wiki_repo, namespace: group) } - before do - put(api(url, user), params: payload) - end - include_examples 'updates wiki page' end end diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index 48f1d696ff6..1645b67c329 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -311,7 +311,14 @@ describe Notes::CreateService do end it 'converts existing note to DiscussionNote' do - expect { subject }.to change { existing_note.reload.type }.from(nil).to('DiscussionNote') + expect do + existing_note + + Timecop.freeze(Time.now + 1.minute) { subject } + + existing_note.reload + end.to change { existing_note.type }.from(nil).to('DiscussionNote') + .and change { existing_note.updated_at } end end end diff --git a/spec/services/task_list_toggle_service_spec.rb b/spec/services/task_list_toggle_service_spec.rb index 7c5480d382f..b1260cf740a 100644 --- a/spec/services/task_list_toggle_service_spec.rb +++ b/spec/services/task_list_toggle_service_spec.rb @@ -12,6 +12,10 @@ describe TaskListToggleService do 1. [X] Item 1 - [ ] Sub-item 1 + + - [ ] loose list + + with an embedded paragraph EOT end @@ -26,16 +30,22 @@ describe TaskListToggleService do </li> </ul> <p data-sourcepos="4:1-4:11" dir="auto">A paragraph</p> - <ol data-sourcepos="6:1-7:19" class="task-list" dir="auto"> - <li data-sourcepos="6:1-7:19" class="task-list-item"> - <input type="checkbox" class="task-list-item-checkbox" disabled checked> Item 1 - <ul data-sourcepos="7:4-7:19" class="task-list"> - <li data-sourcepos="7:4-7:19" class="task-list-item"> - <input type="checkbox" class="task-list-item-checkbox" disabled> Sub-item 1 + <ol data-sourcepos="6:1-8:0" class="task-list" dir="auto"> + <li data-sourcepos="6:1-8:0" class="task-list-item"> + <input type="checkbox" class="task-list-item-checkbox" checked="" disabled=""> Item 1 + <ul data-sourcepos="7:4-8:0" class="task-list"> + <li data-sourcepos="7:4-8:0" class="task-list-item"> + <input type="checkbox" class="task-list-item-checkbox" disabled=""> Sub-item 1 </li> </ul> </li> </ol> + <ul data-sourcepos="9:1-11:28" class="task-list" dir="auto"> + <li data-sourcepos="9:1-11:28" class="task-list-item"> + <p data-sourcepos="9:3-9:16"><input type="checkbox" class="task-list-item-checkbox" disabled=""> loose list</p> + <p data-sourcepos="11:3-11:28">with an embedded paragraph</p> + </li> + </ul> EOT end @@ -59,6 +69,16 @@ describe TaskListToggleService do expect(toggler.updated_markdown_html).to include('disabled> Item 1') end + it 'checks task in loose list' do + toggler = described_class.new(markdown, markdown_html, + toggle_as_checked: true, + line_source: '- [ ] loose list', line_number: 9) + + expect(toggler.execute).to be_truthy + expect(toggler.updated_markdown.lines[8]).to eq "- [x] loose list\n" + expect(toggler.updated_markdown_html).to include('disabled checked> loose list') + end + it 'returns false if line_source does not match the text' do toggler = described_class.new(markdown, markdown_html, toggle_as_checked: false, diff --git a/spec/support/api/milestones_shared_examples.rb b/spec/support/api/milestones_shared_examples.rb index 5f709831ce1..b426fadb001 100644 --- a/spec/support/api/milestones_shared_examples.rb +++ b/spec/support/api/milestones_shared_examples.rb @@ -8,12 +8,17 @@ shared_examples_for 'group and project milestones' do |route_definition| describe "GET #{route_definition}" do it 'returns milestones list' do + create(:issue, project: project, milestone: milestone) + create(:closed_issue, project: project, milestone: milestone) + create(:closed_issue, project: project, milestone: milestone) + get api(route, user) expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.first['title']).to eq(milestone.title) + expect(json_response.first['percentage_complete']).to eq(66) end it 'returns a 401 error if user not authenticated' do diff --git a/spec/support/helpers/stub_feature_flags.rb b/spec/support/helpers/stub_feature_flags.rb index 4061a8d1bc9..48258692304 100644 --- a/spec/support/helpers/stub_feature_flags.rb +++ b/spec/support/helpers/stub_feature_flags.rb @@ -1,11 +1,30 @@ module StubFeatureFlags # Stub Feature flags with `flag_name: true/false` # - # @param [Hash] features where key is feature name and value is boolean whether enabled or not + # @param [Hash] features where key is feature name and value is boolean whether enabled or not. + # Alternatively, you can specify Hash to enable the flag on a specific thing. + # + # Examples + # - `stub_feature_flags(ci_live_trace: false)` ... Disable `ci_live_trace` + # feature flag globally. + # - `stub_feature_flags(ci_live_trace: { enabled: false, thing: project })` ... + # Disable `ci_live_trace` feature flag on the specified project. def stub_feature_flags(features) - features.each do |feature_name, enabled| - allow(Feature).to receive(:enabled?).with(feature_name, any_args) { enabled } - allow(Feature).to receive(:enabled?).with(feature_name.to_s, any_args) { enabled } + features.each do |feature_name, option| + if option.is_a?(Hash) + enabled, thing = option.values_at(:enabled, :thing) + else + enabled = option + thing = nil + end + + if thing + allow(Feature).to receive(:enabled?).with(feature_name, thing, any_args) { enabled } + allow(Feature).to receive(:enabled?).with(feature_name.to_s, thing, any_args) { enabled } + else + allow(Feature).to receive(:enabled?).with(feature_name, any_args) { enabled } + allow(Feature).to receive(:enabled?).with(feature_name.to_s, any_args) { enabled } + end end end end diff --git a/spec/support/shared_examples/models/with_uploads_shared_examples.rb b/spec/support/shared_examples/models/with_uploads_shared_examples.rb index 1d11b855459..43033a2d256 100644 --- a/spec/support/shared_examples/models/with_uploads_shared_examples.rb +++ b/spec/support/shared_examples/models/with_uploads_shared_examples.rb @@ -44,26 +44,6 @@ shared_examples_for 'model with uploads' do |supports_fileuploads| model_object.destroy end end - - describe 'destroy strategy depending on feature flag' do - let!(:upload) { create(:upload, uploader: FileUploader, model: model_object) } - - it 'does not destroy uploads by default' do - expect(model_object).to receive(:delete_uploads) - expect(model_object).not_to receive(:destroy_uploads) - - model_object.destroy - end - - it 'uses before destroy callback if feature flag is disabled' do - stub_feature_flags(fast_destroy_uploads: false) - - expect(model_object).to receive(:destroy_uploads) - expect(model_object).not_to receive(:delete_uploads) - - model_object.destroy - end - end end end end |