diff options
author | Shinya Maeda <shinya@gitlab.com> | 2017-10-04 02:59:11 +0900 |
---|---|---|
committer | Shinya Maeda <shinya@gitlab.com> | 2017-10-04 02:59:11 +0900 |
commit | 65b4627d3eda27844b093c981d05499a86e7235a (patch) | |
tree | 187d64d5271f4f9409eef1d947312a8f7ef80e69 | |
parent | 6d4e28295863fb1969c4785b3c8463c12cafb52f (diff) | |
parent | bd970a51d3a44346bd8ffd769a85a3c4cc66db0f (diff) | |
download | gitlab-ce-65b4627d3eda27844b093c981d05499a86e7235a.tar.gz |
Merge branch 'master' into feature/sm/35954-create-kubernetes-cluster-on-gke-from-k8s-service
142 files changed, 2035 insertions, 833 deletions
diff --git a/.gitignore b/.gitignore index 3baf640a9c3..4933575332b 100644 --- a/.gitignore +++ b/.gitignore @@ -63,4 +63,5 @@ eslint-report.html /.gitlab_workhorse_secret /webpack-report/ /locale/**/LC_MESSAGES +/locale/**/*.time_stamp /.rspec diff --git a/GITLAB_PAGES_VERSION b/GITLAB_PAGES_VERSION index 4b9fcbec101..a918a2aa18d 100644 --- a/GITLAB_PAGES_VERSION +++ b/GITLAB_PAGES_VERSION @@ -1 +1 @@ -0.5.1 +0.6.0 @@ -398,7 +398,7 @@ group :ed25519 do end # Gitaly GRPC client -gem 'gitaly-proto', '~> 0.38.0', require: 'gitaly' +gem 'gitaly-proto', '~> 0.39.0', require: 'gitaly' gem 'toml-rb', '~> 0.3.15', require: false diff --git a/Gemfile.lock b/Gemfile.lock index fe54dd9d06c..a0ad2716c01 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -273,7 +273,7 @@ GEM po_to_json (>= 1.0.0) rails (>= 3.2.0) gherkin-ruby (0.3.2) - gitaly-proto (0.38.0) + gitaly-proto (0.39.0) google-protobuf (~> 3.1) grpc (~> 1.0) github-linguist (4.7.6) @@ -1027,7 +1027,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.2.0) - gitaly-proto (~> 0.38.0) + gitaly-proto (~> 0.39.0) github-linguist (~> 4.7.0) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-markup (~> 1.6.2) diff --git a/PROCESS.md b/PROCESS.md index ed4e84dd0b6..5e65bb59246 100644 --- a/PROCESS.md +++ b/PROCESS.md @@ -197,6 +197,11 @@ month. When we say 'the most recent monthly release', this can refer to either the version currently running on GitLab.com, or the most recent version available in the package repositories. +A regression issue should be labeled with the appropriate [subject label](../CONTRIBUTING.md#subject-labels-wiki-container-registry-ldap-api-etc) +and [team label](../CONTRIBUTING.md#team-labels-ci-discussion-edge-platform-etc), +just like any other issue, to help GitLab team members focus on issues that are +relevant to [their area of responsibility](https://about.gitlab.com/handbook/engineering/workflow/#choosing-something-to-work-on). + ## Release retrospective and kickoff - [Retrospective](https://about.gitlab.com/handbook/engineering/workflow/#retrospective) diff --git a/app/assets/javascripts/copy_as_gfm.js b/app/assets/javascripts/copy_as_gfm.js index e3e2c798570..93b0cbf4209 100644 --- a/app/assets/javascripts/copy_as_gfm.js +++ b/app/assets/javascripts/copy_as_gfm.js @@ -298,7 +298,7 @@ class CopyAsGFM { const documentFragment = getSelectedFragment(); if (!documentFragment) return; - const el = transformer(documentFragment.cloneNode(true)); + const el = transformer(documentFragment.cloneNode(true), e.currentTarget); if (!el) return; e.preventDefault(); @@ -338,55 +338,64 @@ class CopyAsGFM { } static transformGFMSelection(documentFragment) { - const gfmEls = documentFragment.querySelectorAll('.md, .wiki'); - switch (gfmEls.length) { + const gfmElements = documentFragment.querySelectorAll('.md, .wiki'); + switch (gfmElements.length) { case 0: { return documentFragment; } case 1: { - return gfmEls[0]; + return gfmElements[0]; } default: { - const allGfmEl = document.createElement('div'); + const allGfmElement = document.createElement('div'); - for (let i = 0; i < gfmEls.length; i += 1) { - const lineEl = gfmEls[i]; - allGfmEl.appendChild(lineEl); - allGfmEl.appendChild(document.createTextNode('\n\n')); + for (let i = 0; i < gfmElements.length; i += 1) { + const gfmElement = gfmElements[i]; + allGfmElement.appendChild(gfmElement); + allGfmElement.appendChild(document.createTextNode('\n\n')); } - return allGfmEl; + return allGfmElement; } } } - static transformCodeSelection(documentFragment) { - const lineEls = documentFragment.querySelectorAll('.line'); + static transformCodeSelection(documentFragment, target) { + let lineSelector = '.line'; - let codeEl; - if (lineEls.length > 1) { - codeEl = document.createElement('pre'); - codeEl.className = 'code highlight'; + if (target) { + const lineClass = ['left-side', 'right-side'].filter(name => target.classList.contains(name))[0]; + if (lineClass) { + lineSelector = `.line_content.${lineClass} ${lineSelector}`; + } + } + + const lineElements = documentFragment.querySelectorAll(lineSelector); + + let codeElement; + if (lineElements.length > 1) { + codeElement = document.createElement('pre'); + codeElement.className = 'code highlight'; - const lang = lineEls[0].getAttribute('lang'); + const lang = lineElements[0].getAttribute('lang'); if (lang) { - codeEl.setAttribute('lang', lang); + codeElement.setAttribute('lang', lang); } } else { - codeEl = document.createElement('code'); + codeElement = document.createElement('code'); } - if (lineEls.length > 0) { - for (let i = 0; i < lineEls.length; i += 1) { - const lineEl = lineEls[i]; - codeEl.appendChild(lineEl); - codeEl.appendChild(document.createTextNode('\n')); + if (lineElements.length > 0) { + for (let i = 0; i < lineElements.length; i += 1) { + const lineElement = lineElements[i]; + codeElement.appendChild(lineElement); + codeElement.appendChild(document.createTextNode('\n')); } } else { - codeEl.appendChild(documentFragment); + codeElement.appendChild(documentFragment); } - return codeEl; + return codeElement; } static nodeToGFM(node, respectWhitespaceParam = false) { diff --git a/app/assets/javascripts/diff.js b/app/assets/javascripts/diff.js index 6a008112203..ae8338f5fd2 100644 --- a/app/assets/javascripts/diff.js +++ b/app/assets/javascripts/diff.js @@ -24,7 +24,8 @@ class Diff { if (!isBound) { $(document) .on('click', '.js-unfold', this.handleClickUnfold.bind(this)) - .on('click', '.diff-line-num a', this.handleClickLineNum.bind(this)); + .on('click', '.diff-line-num a', this.handleClickLineNum.bind(this)) + .on('mousedown', 'td.line_content.parallel', this.handleParallelLineDown.bind(this)); isBound = true; } @@ -100,6 +101,18 @@ class Diff { this.highlightSelectedLine(); } + handleParallelLineDown(e) { + const line = $(e.currentTarget); + const table = line.closest('table'); + + table.removeClass('left-side-selected right-side-selected'); + + const lineClass = ['left-side', 'right-side'].filter(name => line.hasClass(name))[0]; + if (lineClass) { + table.addClass(`${lineClass}-selected`); + } + } + diffViewType() { return $('.inline-parallel-buttons a.active').data('view-type'); } diff --git a/app/assets/javascripts/locale/index.js b/app/assets/javascripts/locale/index.js index 6a5084efeb8..af718e894cf 100644 --- a/app/assets/javascripts/locale/index.js +++ b/app/assets/javascripts/locale/index.js @@ -1,5 +1,7 @@ import Jed from 'jed'; +import sprintf from './sprintf'; + /** This is required to require all the translation folders in the current directory this saves us having to do this manually & keep up to date with new languages @@ -66,4 +68,5 @@ export { lang }; export { gettext as __ }; export { ngettext as n__ }; export { pgettext as s__ }; +export { sprintf }; export default locale; diff --git a/app/assets/javascripts/locale/sprintf.js b/app/assets/javascripts/locale/sprintf.js new file mode 100644 index 00000000000..003cbe820d6 --- /dev/null +++ b/app/assets/javascripts/locale/sprintf.js @@ -0,0 +1,26 @@ +import _ from 'underscore'; + +/** + Very limited implementation of sprintf supporting only named parameters. + + @param input (translated) text with parameters (e.g. '%{num_users} users use us') + @param parameters object mapping parameter names to values (e.g. { num_users: 5 }) + @param escapeParameters whether parameter values should be escaped (see http://underscorejs.org/#escape) + @returns {String} the text with parameters replaces (e.g. '5 users use us') + + @see https://ruby-doc.org/core-2.3.3/Kernel.html#method-i-sprintf + @see https://gitlab.com/gitlab-org/gitlab-ce/issues/37992 +**/ +export default (input, parameters, escapeParameters = true) => { + let output = input; + + if (parameters) { + Object.keys(parameters).forEach((parameterName) => { + const parameterValue = parameters[parameterName]; + const escapedParameterValue = escapeParameters ? _.escape(parameterValue) : parameterValue; + output = output.replace(new RegExp(`%{${parameterName}}`, 'g'), escapedParameterValue); + }); + } + + return output; +} diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_status_icon.js b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_status_icon.js index 703f3a56a34..4998a47b691 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_status_icon.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_status_icon.js @@ -27,7 +27,7 @@ export default { <button v-if="showDisabledButton" type="button" - class="btn btn-success btn-sm" + class="js-disabled-merge-button btn btn-success btn-sm" disabled="true"> Merge </button> diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_closed.js b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_closed.js index 4078aad7f83..b25cc3443ef 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_closed.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_closed.js @@ -16,9 +16,9 @@ export default { <div class="media-body"> <mr-widget-author-and-time actionText="Closed by" - :author="mr.closedBy" - :dateTitle="mr.updatedAt" - :dateReadable="mr.closedAt" + :author="mr.closedEvent.author" + :dateTitle="mr.closedEvent.updatedAt" + :dateReadable="mr.closedEvent.formattedUpdatedAt" /> <section class="mr-info-list"> <p> diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_conflicts.js b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_conflicts.js index f9cb79a0bc1..dc252f8a9b7 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_conflicts.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_conflicts.js @@ -10,27 +10,37 @@ export default { }, template: ` <div class="mr-widget-body media"> - <status-icon status="failed" showDisabledButton /> + <status-icon + status="failed" + showDisabledButton /> <div class="media-body space-children"> - <span class="bold"> - There are merge conflicts<span v-if="!mr.canMerge">.</span> - <span v-if="!mr.canMerge"> - Resolve these conflicts or ask someone with write access to this repository to merge it locally - </span> + <span + v-if="mr.shouldBeRebased" + class="bold"> + Fast-forward merge is not possible. + To merge this request, first rebase locally. </span> - <a - v-if="mr.canMerge && mr.conflictResolutionPath" - :href="mr.conflictResolutionPath" - class="btn btn-default btn-xs js-resolve-conflicts-button"> - Resolve conflicts - </a> - <a - v-if="mr.canMerge" - class="btn btn-default btn-xs js-merge-locally-button" - data-toggle="modal" - href="#modal_merge_info"> - Merge locally - </a> + <template v-else> + <span class="bold"> + There are merge conflicts<span v-if="!mr.canMerge">.</span> + <span v-if="!mr.canMerge"> + Resolve these conflicts or ask someone with write access to this repository to merge it locally + </span> + </span> + <a + v-if="mr.canMerge && mr.conflictResolutionPath" + :href="mr.conflictResolutionPath" + class="js-resolve-conflicts-button btn btn-default btn-xs"> + Resolve conflicts + </a> + <a + v-if="mr.canMerge" + class="js-merge-locally-button btn btn-default btn-xs" + data-toggle="modal" + href="#modal_merge_info"> + Merge locally + </a> + </template> </div> </div> `, diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.js b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.js index e452260a4d0..74fc52796a0 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.js @@ -69,9 +69,9 @@ export default { <div class="space-children"> <mr-widget-author-and-time actionText="Merged by" - :author="mr.mergedBy" - :dateTitle="mr.updatedAt" - :dateReadable="mr.mergedAt" /> + :author="mr.mergedEvent.author" + :date-title="mr.mergedEvent.updatedAt" + :date-readable="mr.mergedEvent.formattedUpdatedAt" /> <a v-if="mr.canRevertInCurrentMR" v-tooltip diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js index ad709da51ee..f83d3ca00dd 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js @@ -284,10 +284,16 @@ export default { :mr="mr" :is-merge-button-disabled="isMergeButtonDisabled" /> + <span + v-if="mr.ffOnlyEnabled" + class="js-fast-forward-message"> + Fast-forward merge without a merge commit + </span> <button + v-else @click="toggleCommitMessageEditor" :disabled="isMergeButtonDisabled" - class="btn btn-default btn-xs" + class="js-modify-commit-message-button btn btn-default btn-xs" type="button"> Modify commit message </button> diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js index 29464662578..e554082149b 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js @@ -37,10 +37,8 @@ export default class MergeRequestStore { } this.updatedAt = data.updated_at; - this.mergedAt = MergeRequestStore.getEventDate(data.merge_event); - this.closedAt = MergeRequestStore.getEventDate(data.closed_event); - this.mergedBy = MergeRequestStore.getAuthorObject(data.merge_event); - this.closedBy = MergeRequestStore.getAuthorObject(data.closed_event); + this.mergedEvent = MergeRequestStore.getEventObject(data.merge_event); + this.closedEvent = MergeRequestStore.getEventObject(data.closed_event); this.setToMWPSBy = MergeRequestStore.getAuthorObject({ author: data.merge_user || {} }); this.mergeUserId = data.merge_user_id; this.currentUserId = gon.current_user_id; @@ -57,6 +55,8 @@ export default class MergeRequestStore { this.onlyAllowMergeIfPipelineSucceeds = data.only_allow_merge_if_pipeline_succeeds || false; this.mergeWhenPipelineSucceeds = data.merge_when_pipeline_succeeds || false; this.mergePath = data.merge_path; + this.ffOnlyEnabled = data.ff_only_enabled; + this.shouldBeRebased = !!data.should_be_rebased; this.statusPath = data.status_path; this.emailPatchesPath = data.email_patches_path; this.plainDiffPath = data.plain_diff_path; @@ -118,6 +118,14 @@ export default class MergeRequestStore { } } + static getEventObject(event) { + return { + author: MergeRequestStore.getAuthorObject(event), + updatedAt: gl.utils.formatDate(MergeRequestStore.getEventUpdatedAtDate(event)), + formattedUpdatedAt: MergeRequestStore.getEventDate(event), + }; + } + static getAuthorObject(event) { if (!event) { return {}; @@ -131,6 +139,14 @@ export default class MergeRequestStore { }; } + static getEventUpdatedAtDate(event) { + if (!event) { + return ''; + } + + return event.updated_at; + } + static getEventDate(event) { const timeagoInstance = new Timeago(); @@ -138,7 +154,7 @@ export default class MergeRequestStore { return ''; } - return timeagoInstance.format(event.updated_at); + return timeagoInstance.format(MergeRequestStore.getEventUpdatedAtDate(event)); } } diff --git a/app/assets/javascripts/vue_shared/translate.js b/app/assets/javascripts/vue_shared/translate.js index f83c4b00761..2c7886ec308 100644 --- a/app/assets/javascripts/vue_shared/translate.js +++ b/app/assets/javascripts/vue_shared/translate.js @@ -2,6 +2,7 @@ import { __, n__, s__, + sprintf, } from '../locale'; export default (Vue) => { @@ -37,6 +38,7 @@ export default (Vue) => { @returns {String} Translated context based text **/ s__, + sprintf, }, }); }; diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index c0d8e6c328c..5c397470629 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -873,6 +873,13 @@ min-width: 100%; } } + + header.navbar-gitlab-new .header-content .dropdown { + .dropdown-menu { + left: 0; + min-width: 100%; + } + } } @include new-style-dropdown('.breadcrumbs-list .dropdown '); diff --git a/app/assets/stylesheets/framework/lists.scss b/app/assets/stylesheets/framework/lists.scss index 0fb19344510..badc7b0eba3 100644 --- a/app/assets/stylesheets/framework/lists.scss +++ b/app/assets/stylesheets/framework/lists.scss @@ -229,6 +229,10 @@ ul.content-list { .label-default { color: $gl-text-color-secondary; } + + .avatar-cell { + align-self: flex-start; + } } .panel > .content-list > li { diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index e4bd783c8bc..fb23343b966 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -77,6 +77,18 @@ word-wrap: break-word; } } + + &.left-side-selected { + td.line_content.parallel.right-side { + @include user-select(none); + } + } + + &.right-side-selected { + td.line_content.parallel.left-side { + @include user-select(none); + } + } } tr.line_holder.parallel { diff --git a/app/controllers/concerns/notes_actions.rb b/app/controllers/concerns/notes_actions.rb index 18fd8eb114d..915f32b4c33 100644 --- a/app/controllers/concerns/notes_actions.rb +++ b/app/controllers/concerns/notes_actions.rb @@ -15,9 +15,9 @@ module NotesActions notes = notes_finder.execute .inc_relations_for_view - .reject { |n| n.cross_reference_not_visible_for?(current_user) } notes = prepare_notes_for_rendering(notes) + notes = notes.reject { |n| n.cross_reference_not_visible_for?(current_user) } notes_json[:notes] = if noteable.discussions_rendered_on_frontend? diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index a3ec79a56d9..ee6e6f80cdd 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -16,7 +16,7 @@ class Projects::IssuesController < Projects::ApplicationController before_action :authorize_create_issue!, only: [:new, :create] # Allow modify issue - before_action :authorize_update_issue!, only: [:edit, :update, :move] + before_action :authorize_update_issue!, only: [:update, :move] # Allow create a new branch and empty WIP merge request from current issue before_action :authorize_create_merge_request!, only: [:create_merge_request] @@ -63,10 +63,6 @@ class Projects::IssuesController < Projects::ApplicationController respond_with(@issue) end - def edit - respond_with(@issue) - end - def show @noteable = @issue @note = @project.notes.new(noteable: @issue) @@ -126,10 +122,6 @@ class Projects::IssuesController < Projects::ApplicationController @issue = Issues::UpdateService.new(project, current_user, update_params).execute(issue) respond_to do |format| - format.html do - recaptcha_check_with_fallback { render :edit } - end - format.json do render_issue_json end diff --git a/app/controllers/projects/wikis_controller.rb b/app/controllers/projects/wikis_controller.rb index 968d880886c..a8ebdf5a4a9 100644 --- a/app/controllers/projects/wikis_controller.rb +++ b/app/controllers/projects/wikis_controller.rb @@ -18,16 +18,12 @@ class Projects::WikisController < Projects::ApplicationController response.headers['Content-Security-Policy'] = "default-src 'none'" response.headers['X-Content-Security-Policy'] = "default-src 'none'" - if file.on_disk? - send_file file.on_disk_path, disposition: 'inline' - else - send_data( - file.raw_data, - type: file.mime_type, - disposition: 'inline', - filename: file.name - ) - end + send_data( + file.raw_data, + type: file.mime_type, + disposition: 'inline', + filename: file.name + ) else return render('empty') unless can?(current_user, :create_wiki, @project) @page = WikiPage.new(@project_wiki) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index b13034d3333..a738ca9f361 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -344,6 +344,7 @@ class ProjectsController < Projects::ApplicationController :tag_list, :visibility_level, :template_name, + :merge_method, project_feature_attributes: %i[ builds_access_level diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 28f591a4e22..4e4a66e8a02 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -33,19 +33,21 @@ module DiffHelper end def diff_match_line(old_pos, new_pos, text: '', view: :inline, bottom: false) - content = content_tag :td, text, class: "line_content match #{view == :inline ? '' : view}" - cls = ['diff-line-num', 'unfold', 'js-unfold'] - cls << 'js-unfold-bottom' if bottom + content_line_class = %w[line_content match] + content_line_class << 'parallel' if view == :parallel + + line_num_class = %w[diff-line-num unfold js-unfold] + line_num_class << 'js-unfold-bottom' if bottom html = '' if old_pos - html << content_tag(:td, '...', class: cls + ['old_line'], data: { linenumber: old_pos }) - html << content unless view == :inline + html << content_tag(:td, '...', class: [*line_num_class, 'old_line'], data: { linenumber: old_pos }) + html << content_tag(:td, text, class: [*content_line_class, 'left-side']) if view == :parallel end if new_pos - html << content_tag(:td, '...', class: cls + ['new_line'], data: { linenumber: new_pos }) - html << content + html << content_tag(:td, '...', class: [*line_num_class, 'new_line'], data: { linenumber: new_pos }) + html << content_tag(:td, text, class: [*content_line_class, ('right-side' if view == :parallel)]) end html.html_safe diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 8d9a30397a9..e85b83daf9e 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -524,6 +524,14 @@ class MergeRequest < ActiveRecord::Base true end + def ff_merge_possible? + project.repository.ancestor?(target_branch_sha, diff_head_sha) + end + + def should_be_rebased? + project.ff_merge_must_be_possible? && !ff_merge_possible? + end + def can_cancel_merge_when_pipeline_succeeds?(current_user) can_be_merged_by?(current_user) || self.author == current_user end diff --git a/app/models/project.rb b/app/models/project.rb index fd171cc7941..8223e658e9d 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -64,6 +64,7 @@ class Project < ActiveRecord::Base # Storage specific hooks after_initialize :use_hashed_storage + after_create :check_repository_absence! after_create :ensure_storage_path_exists after_save :ensure_storage_path_exists, if: :namespace_id_changed? @@ -229,7 +230,7 @@ class Project < ActiveRecord::Base validates :import_url, importable_url: true, if: [:external_import?, :import_url_changed?] validates :star_count, numericality: { greater_than_or_equal_to: 0 } validate :check_limit, on: :create - validate :check_repository_path_availability, on: [:create, :update], if: ->(project) { !project.persisted? || project.renamed? } + validate :check_repository_path_availability, on: :update, if: ->(project) { project.renamed? } validate :avatar_type, if: ->(project) { project.avatar.present? && project.avatar_changed? } validates :avatar, file_size: { maximum: 200.kilobytes.to_i } @@ -1026,7 +1027,9 @@ class Project < ActiveRecord::Base expires_full_path_cache # we need to clear cache to validate renames correctly - if gitlab_shell.exists?(repository_storage_path, "#{disk_path}.git") + # Check if repository with same path already exists on disk we can + # skip this for the hashed storage because the path does not change + if legacy_storage? && repository_with_same_path_already_exists? errors.add(:base, 'There is already a repository with that name on disk') return false end @@ -1567,6 +1570,34 @@ class Project < ActiveRecord::Base persisted? && path_changed? end + def merge_method + if self.merge_requests_ff_only_enabled + :ff + elsif self.merge_requests_rebase_enabled + :rebase_merge + else + :merge + end + end + + def merge_method=(method) + case method.to_s + when "ff" + self.merge_requests_ff_only_enabled = true + self.merge_requests_rebase_enabled = true + when "rebase_merge" + self.merge_requests_ff_only_enabled = false + self.merge_requests_rebase_enabled = true + when "merge" + self.merge_requests_ff_only_enabled = false + self.merge_requests_rebase_enabled = false + end + end + + def ff_merge_must_be_possible? + self.merge_requests_ff_only_enabled || self.merge_requests_rebase_enabled + end + def migrate_to_hashed_storage! return if hashed_storage? @@ -1614,6 +1645,19 @@ class Project < ActiveRecord::Base Gitlab::ReferenceCounter.new(gl_repository(is_wiki: true)).value end + def check_repository_absence! + return if skip_disk_validation + + if repository_storage_path.blank? || repository_with_same_path_already_exists? + errors.add(:base, 'There is already a repository with that name on disk') + throw :abort + end + end + + def repository_with_same_path_already_exists? + gitlab_shell.exists?(repository_storage_path, "#{disk_path}.git") + end + # set last_activity_at to the same as created_at def set_last_activity_at update_column(:last_activity_at, self.created_at) diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb index c4cc1c1cf22..bb7be29ef66 100644 --- a/app/models/project_wiki.rb +++ b/app/models/project_wiki.rb @@ -54,12 +54,15 @@ class ProjectWiki [Gitlab.config.gitlab.relative_url_root, '/', @project.full_path, '/wikis'].join('') end - # Returns the Gollum::Wiki object. + # Returns the Gitlab::Git::Wiki object. def wiki @wiki ||= begin - Gollum::Wiki.new(path_to_repo) - rescue Rugged::OSError - create_repo! + gl_repository = Gitlab::GlRepository.gl_repository(project, true) + raw_repository = Gitlab::Git::Repository.new(project.repository_storage, disk_path + '.git', gl_repository) + + create_repo!(raw_repository) unless raw_repository.exists? + + Gitlab::Git::Wiki.new(raw_repository) end end @@ -86,20 +89,14 @@ class ProjectWiki # Returns an initialized WikiPage instance or nil def find_page(title, version = nil) page_title, page_dir = page_title_and_dir(title) - if page = wiki.page(page_title, version, page_dir) + + if page = wiki.page(title: page_title, version: version, dir: page_dir) WikiPage.new(self, page, true) - else - nil end end - def find_file(name, version = nil, try_on_disk = true) - version = wiki.ref if version.nil? # Gollum::Wiki#file ? - if wiki_file = wiki.file(name, version, try_on_disk) - wiki_file - else - nil - end + def find_file(name, version = nil) + wiki.file(name, version) end def create_page(title, content, format = :markdown, message = nil) @@ -108,7 +105,7 @@ class ProjectWiki wiki.write_page(title, format.to_sym, content, commit) update_project_activity - rescue Gollum::DuplicatePageError => e + rescue Gitlab::Git::Wiki::DuplicatePageError => e @error_message = "Duplicate page: #{e.message}" return false end @@ -116,13 +113,13 @@ class ProjectWiki def update_page(page, content:, title: nil, format: :markdown, message: nil) commit = commit_details(:updated, message, page.title) - wiki.update_page(page, title || page.name, format.to_sym, content, commit) + wiki.update_page(page.path, title || page.name, format.to_sym, content, commit) update_project_activity end def delete_page(page, message = nil) - wiki.delete_page(page, commit_details(:deleted, message, page.title)) + wiki.delete_page(page.path, commit_details(:deleted, message, page.title)) update_project_activity end @@ -145,20 +142,8 @@ class ProjectWiki wiki.class.default_ref end - def create_repo! - if init_repo(disk_path) - wiki = Gollum::Wiki.new(path_to_repo) - else - raise CouldNotCreateWikiError - end - - repository.after_create - - wiki - end - def ensure_repository - create_repo! unless repository_exists? + raise CouldNotCreateWikiError unless wiki.repository_exists? end def hook_attrs @@ -173,24 +158,24 @@ class ProjectWiki private - def init_repo(disk_path) + def create_repo!(raw_repository) gitlab_shell.add_repository(project.repository_storage, disk_path) + + raise CouldNotCreateWikiError unless raw_repository.exists? + + repository.after_create end def commit_details(action, message = nil, title = nil) commit_message = message || default_message(action, title) - { email: @user.email, name: @user.name, message: commit_message } + Gitlab::Git::Wiki::CommitDetails.new(@user.name, @user.email, commit_message) end def default_message(action, title) "#{@user.username} #{action} page: #{title}" end - def path_to_repo - @path_to_repo ||= File.join(project.repository_storage_path, "#{disk_path}.git") - end - def update_project_activity @project.touch(:last_activity_at, :last_repository_updated_at) end diff --git a/app/models/repository.rb b/app/models/repository.rb index a0f57f1e54d..d47dc9a05cd 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -850,6 +850,25 @@ class Repository end end + def ff_merge(user, source, target_branch, merge_request: nil) + our_commit = rugged.branches[target_branch].target + their_commit = + if source.is_a?(Gitlab::Git::Commit) + source.raw_commit + else + rugged.lookup(source) + end + + raise 'Invalid merge target' if our_commit.nil? + raise 'Invalid merge source' if their_commit.nil? + + with_branch(user, target_branch) do |start_commit| + merge_request&.update(in_progress_merge_commit_sha: their_commit.oid) + + their_commit.oid + end + end + def revert( user, commit, branch_name, message, start_branch_name: nil, start_project: project) diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index f2315bb3dbb..5f710961f95 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -50,7 +50,7 @@ class WikiPage # The Gitlab ProjectWiki instance. attr_reader :wiki - # The raw Gollum::Page instance. + # The raw Gitlab::Git::WikiPage instance. attr_reader :page # The attributes Hash used for storing and validating @@ -75,7 +75,7 @@ class WikiPage if @attributes[:slug].present? @attributes[:slug] else - wiki.wiki.preview_page(title, '', format).url_path + wiki.wiki.preview_slug(title, format) end end @@ -131,7 +131,7 @@ class WikiPage def versions return [] unless persisted? - @page.versions + wiki.wiki.page_versions(@page.path) end def commit @@ -264,8 +264,8 @@ class WikiPage end page_title, page_dir = wiki.page_title_and_dir(page_details) - gollum_wiki = wiki.wiki - @page = gollum_wiki.paged(page_title, page_dir) + gitlab_git_wiki = wiki.wiki + @page = gitlab_git_wiki.page(title: page_title, dir: page_dir) set_attributes @persisted = errors.blank? diff --git a/app/serializers/merge_request_entity.rb b/app/serializers/merge_request_entity.rb index 07650ce6f20..7d3c752b8e6 100644 --- a/app/serializers/merge_request_entity.rb +++ b/app/serializers/merge_request_entity.rb @@ -13,6 +13,11 @@ class MergeRequestEntity < IssuableEntity expose :target_branch expose :target_project_id + expose :should_be_rebased?, as: :should_be_rebased + expose :ff_only_enabled do |merge_request| + merge_request.project.merge_requests_ff_only_enabled + end + # Events expose :merge_event, using: EventEntity expose :closed_event, using: EventEntity diff --git a/app/services/merge_requests/ff_merge_service.rb b/app/services/merge_requests/ff_merge_service.rb new file mode 100644 index 00000000000..ba6853b835a --- /dev/null +++ b/app/services/merge_requests/ff_merge_service.rb @@ -0,0 +1,24 @@ +module MergeRequests + # MergeService class + # + # Do git fast-forward merge and in case of success + # mark merge request as merged and execute all hooks and notifications + # Executed when you do fast-forward merge via GitLab UI + # + class FfMergeService < MergeRequests::MergeService + private + + def commit + repository.ff_merge(current_user, + source, + merge_request.target_branch, + merge_request: merge_request) + rescue Gitlab::Git::HooksService::PreReceiveError => e + raise MergeError, e.message + rescue StandardError => e + raise MergeError, "Something went wrong during merge: #{e.message}" + ensure + merge_request.update(in_progress_merge_commit_sha: nil) + end + end +end diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index bf26859dd6d..a110abf8256 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -11,6 +11,11 @@ module MergeRequests attr_reader :merge_request, :source def execute(merge_request) + if project.merge_requests_ff_only_enabled && !self.is_a?(FfMergeService) + FfMergeService.new(project, current_user, params).execute(merge_request) + return + end + @merge_request = merge_request unless @merge_request.mergeable? diff --git a/app/views/projects/_merge_request_fast_forward_settings.html.haml b/app/views/projects/_merge_request_fast_forward_settings.html.haml new file mode 100644 index 00000000000..9d357293a2f --- /dev/null +++ b/app/views/projects/_merge_request_fast_forward_settings.html.haml @@ -0,0 +1,13 @@ +- form = local_assigns.fetch(:form) +- project = local_assigns.fetch(:project) + +.radio + = label_tag :project_merge_method_ff do + = form.radio_button :merge_method, :ff, class: "js-merge-method-radio" + %strong Fast-forward merge + %br + %span.descr + No merge commits are created and all merges are fast-forwarded, which means that merging is only allowed if the branch could be fast-forwarded. + %br + %span.descr + When fast-forward merge is not possible, the user must first rebase locally. diff --git a/app/views/projects/_merge_request_rebase_settings.html.haml b/app/views/projects/_merge_request_rebase_settings.html.haml new file mode 100644 index 00000000000..c52e09573a6 --- /dev/null +++ b/app/views/projects/_merge_request_rebase_settings.html.haml @@ -0,0 +1,13 @@ +- form = local_assigns.fetch(:form) + +.radio + = label_tag :project_merge_method_rebase_merge do + = form.radio_button :merge_method, :rebase_merge, class: "js-merge-method-radio" + %strong Merge commit with semi-linear history + %br + %span.descr + A merge commit is created for every merge, but merging is only allowed if fast-forward merge is possible. + This way you could make sure that if this merge request would build, after merging to target branch it would also build. + %br + %span.descr + When fast-forward merge is not possible, the user must first rebase locally. diff --git a/app/views/projects/_merge_request_settings.html.haml b/app/views/projects/_merge_request_settings.html.haml index cc5afa943cf..fd0c419cdac 100644 --- a/app/views/projects/_merge_request_settings.html.haml +++ b/app/views/projects/_merge_request_settings.html.haml @@ -1,3 +1,18 @@ - form = local_assigns.fetch(:form) +.form-group + = label_tag :merge_method_merge, class: 'label-light' do + Merge method + .radio + = label_tag :project_merge_method_merge do + = form.radio_button :merge_method, :merge, class: "js-merge-method-radio" + %strong Merge commit + %br + %span.descr + A merge commit is created for every merge, and merging is allowed as long as there are no conflicts. + + = render 'merge_request_rebase_settings', form: form + + = render 'merge_request_fast_forward_settings', project: @project, form: form + = render 'projects/merge_request_merge_settings', form: form diff --git a/app/views/projects/blob/diff.html.haml b/app/views/projects/blob/diff.html.haml index d1d448f0d4c..ea7a71792a3 100644 --- a/app/views/projects/blob/diff.html.haml +++ b/app/views/projects/blob/diff.html.haml @@ -5,25 +5,24 @@ = diff_match_line @form.since, @form.since, text: @match_line, view: diff_view - @lines.each_with_index do |line, index| - - line_new = index + @form.since - - line_old = line_new - @form.offset - - line_content = capture do - %td.line_content.noteable_line{ class: line_class }==#{' ' * @form.indent}#{line} - %tr.line_holder.diff-expanded{ id: line_old, class: line_class } + - line_number_new = index + @form.since + - line_number_old = line_number_new - @form.offset + - line[0, 0] = ' ' * @form.indent + %tr.line_holder.diff-expanded{ id: line_number_old, class: line_class } - case diff_view - when :inline - %td.old_line.diff-line-num{ data: { linenumber: line_old } } - %a{ href: "#", data: { linenumber: line_old }, disabled: true } - %td.new_line.diff-line-num{ data: { linenumber: line_new } } - %a{ href: "#", data: { linenumber: line_new }, disabled: true } - = line_content + %td.old_line.diff-line-num{ data: { linenumber: line_number_old } } + %a{ href: "#", data: { linenumber: line_number_old }, disabled: true } + %td.new_line.diff-line-num{ data: { linenumber: line_number_new } } + %a{ href: "#", data: { linenumber: line_number_new }, disabled: true } + %td.line_content.noteable_line{ class: line_class }= line - when :parallel - %td.old_line.diff-line-num{ data: { linenumber: line_old } } - %a{ href: "##{line_old}", data: { linenumber: line_old }, disabled: true } - = line_content - %td.new_line.diff-line-num{ data: { linenumber: line_new } } - %a{ href: "##{line_new}", data: { linenumber: line_new }, disabled: true } - = line_content + %td.old_line.diff-line-num{ data: { linenumber: line_number_old } } + %a{ href: "##{line_number_old}", data: { linenumber: line_number_old }, disabled: true } + %td.line_content.noteable_line.left-side{ class: line_class }= line + %td.new_line.diff-line-num{ data: { linenumber: line_number_new } } + %a{ href: "##{line_number_new}", data: { linenumber: line_number_new }, disabled: true } + %td.line_content.noteable_line.right-side{ class: line_class }= line - if @form.unfold? && @form.bottom? && @form.to < @blob.lines.size %tr.line_holder{ id: @form.to, class: line_class } diff --git a/app/views/projects/diffs/_parallel_view.html.haml b/app/views/projects/diffs/_parallel_view.html.haml index 56d63250714..1f0ca211074 100644 --- a/app/views/projects/diffs/_parallel_view.html.haml +++ b/app/views/projects/diffs/_parallel_view.html.haml @@ -14,20 +14,20 @@ = diff_match_line left.old_pos, nil, text: left.text, view: :parallel - when 'old-nonewline', 'new-nonewline' %td.old_line.diff-line-num - %td.line_content.match= left.text + %td.line_content.match.left-side= left.text - else - left_line_code = diff_file.line_code(left) - left_position = diff_file.position(left) - %td.old_line.diff-line-num.js-avatar-container{ id: left_line_code, class: left.type, data: { linenumber: left.old_pos } } + %td.old_line.diff-line-num.js-avatar-container{ class: left.type, data: { linenumber: left.old_pos } } = add_diff_note_button(left_line_code, left_position, 'old') %a{ href: "##{left_line_code}", data: { linenumber: left.old_pos } } - discussion_left = discussions_left.try(:first) - if discussion_left && discussion_left.resolvable? %diff-note-avatars{ "discussion-id" => discussion_left.id } - %td.line_content.parallel.noteable_line{ class: left.type }= diff_line_content(left.text) + %td.line_content.parallel.noteable_line.left-side{ id: left_line_code, class: left.type }= diff_line_content(left.text) - else %td.old_line.diff-line-num.empty-cell - %td.line_content.parallel + %td.line_content.parallel.left-side - if right - case right.type @@ -35,20 +35,20 @@ = diff_match_line nil, right.new_pos, text: left.text, view: :parallel - when 'old-nonewline', 'new-nonewline' %td.new_line.diff-line-num - %td.line_content.match= right.text + %td.line_content.match.right-side= right.text - else - right_line_code = diff_file.line_code(right) - right_position = diff_file.position(right) - %td.new_line.diff-line-num.js-avatar-container{ id: right_line_code, class: right.type, data: { linenumber: right.new_pos } } + %td.new_line.diff-line-num.js-avatar-container{ class: right.type, data: { linenumber: right.new_pos } } = add_diff_note_button(right_line_code, right_position, 'new') %a{ href: "##{right_line_code}", data: { linenumber: right.new_pos } } - discussion_right = discussions_right.try(:first) - if discussion_right && discussion_right.resolvable? %diff-note-avatars{ "discussion-id" => discussion_right.id } - %td.line_content.parallel.noteable_line{ class: right.type }= diff_line_content(right.text) + %td.line_content.parallel.noteable_line.right-side{ id: right_line_code, class: right.type }= diff_line_content(right.text) - else %td.old_line.diff-line-num.empty-cell - %td.line_content.parallel + %td.line_content.parallel.right-side - if discussions_left || discussions_right = render "discussions/parallel_diff_discussion", discussions_left: discussions_left, discussions_right: discussions_right diff --git a/app/views/projects/issues/edit.html.haml b/app/views/projects/issues/edit.html.haml deleted file mode 100644 index 1b7d878c38c..00000000000 --- a/app/views/projects/issues/edit.html.haml +++ /dev/null @@ -1,7 +0,0 @@ -- page_title "Edit", "#{@issue.title} (#{@issue.to_reference})", "Issues" - -%h3.page-title - Edit Issue ##{@issue.iid} -%hr - -= render "form" diff --git a/app/views/projects/wikis/history.html.haml b/app/views/projects/wikis/history.html.haml index bc1ab5065e4..9ee09262324 100644 --- a/app/views/projects/wikis/history.html.haml +++ b/app/views/projects/wikis/history.html.haml @@ -29,13 +29,13 @@ commit.id, index == 0) do = truncate_sha(commit.id) %td - = commit.author.name + = commit.author_name %td = commit.message %td #{time_ago_with_tooltip(version.authored_date)} %td %strong - = @page.page.wiki.page(@page.page.name, commit.id).try(:format) + = version.format = render 'sidebar' diff --git a/app/views/projects/wikis/show.html.haml b/app/views/projects/wikis/show.html.haml index 62c18cc4582..de15fc99eda 100644 --- a/app/views/projects/wikis/show.html.haml +++ b/app/views/projects/wikis/show.html.haml @@ -11,7 +11,7 @@ .nav-text %h2.wiki-page-title= @page.title.capitalize %span.wiki-last-edit-by - = (_("Last edited by %{name}") % { name: "<strong>#{@page.commit.author.name}</strong>" }).html_safe + = (_("Last edited by %{name}") % { name: "<strong>#{@page.commit.author_name}</strong>" }).html_safe #{time_ago_with_tooltip(@page.commit.authored_date)} .nav-controls diff --git a/changelogs/unreleased/36670-remove-edit-form.yml b/changelogs/unreleased/36670-remove-edit-form.yml new file mode 100644 index 00000000000..4e80b685f67 --- /dev/null +++ b/changelogs/unreleased/36670-remove-edit-form.yml @@ -0,0 +1,5 @@ +--- +title: Remove the ability to visit the issue edit form directly +merge_request: 14523 +author: +type: removed diff --git a/changelogs/unreleased/38202-cannot-rename-a-hashed-project.yml b/changelogs/unreleased/38202-cannot-rename-a-hashed-project.yml new file mode 100644 index 00000000000..768e296fcd7 --- /dev/null +++ b/changelogs/unreleased/38202-cannot-rename-a-hashed-project.yml @@ -0,0 +1,6 @@ +--- +title: Does not check if an invariant hashed storage path exists on disk when renaming + projects. +merge_request: 14428 +author: +type: fixed diff --git a/changelogs/unreleased/38502-fix-nav-dropdown-close-animation.yml b/changelogs/unreleased/38502-fix-nav-dropdown-close-animation.yml new file mode 100644 index 00000000000..974adb9ed28 --- /dev/null +++ b/changelogs/unreleased/38502-fix-nav-dropdown-close-animation.yml @@ -0,0 +1,5 @@ +--- +title: Fix navigation dropdown close animation on mobile screens +merge_request: 14649 +author: +type: fixed diff --git a/changelogs/unreleased/38635-fix-gitlab-check-git-ssh-config.yml b/changelogs/unreleased/38635-fix-gitlab-check-git-ssh-config.yml new file mode 100644 index 00000000000..49d0671233a --- /dev/null +++ b/changelogs/unreleased/38635-fix-gitlab-check-git-ssh-config.yml @@ -0,0 +1,5 @@ +--- +title: Whitelist authorized_keys.lock in the gitlab:check rake task +merge_request: 14624 +author: +type: fixed diff --git a/changelogs/unreleased/add-labels-template-index.yml b/changelogs/unreleased/add-labels-template-index.yml new file mode 100644 index 00000000000..5f66c4ce181 --- /dev/null +++ b/changelogs/unreleased/add-labels-template-index.yml @@ -0,0 +1,5 @@ +--- +title: Add (partial) index on Labels.template +merge_request: +author: +type: other diff --git a/changelogs/unreleased/close-issue-by-implements.yml b/changelogs/unreleased/close-issue-by-implements.yml new file mode 100644 index 00000000000..fe36ce3f7aa --- /dev/null +++ b/changelogs/unreleased/close-issue-by-implements.yml @@ -0,0 +1,5 @@ +--- +title: "Add \"implements\" to the default issue closing message regex" +merge_request: 14612 +author: Guilherme Vieira +type: added diff --git a/changelogs/unreleased/commit-row-avatar-align-top.yml b/changelogs/unreleased/commit-row-avatar-align-top.yml new file mode 100644 index 00000000000..aa5ab770bd8 --- /dev/null +++ b/changelogs/unreleased/commit-row-avatar-align-top.yml @@ -0,0 +1,5 @@ +--- +title: Fixed commit avatars being centered vertically +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/dm-copy-parallel-diff.yml b/changelogs/unreleased/dm-copy-parallel-diff.yml new file mode 100644 index 00000000000..96a65007661 --- /dev/null +++ b/changelogs/unreleased/dm-copy-parallel-diff.yml @@ -0,0 +1,5 @@ +--- +title: Only copy old/new code when selecting left/right side of parallel diff +merge_request: +author: +type: added diff --git a/changelogs/unreleased/docs-add-summary-about-project-archiving.yml b/changelogs/unreleased/docs-add-summary-about-project-archiving.yml new file mode 100644 index 00000000000..cc1b48a682d --- /dev/null +++ b/changelogs/unreleased/docs-add-summary-about-project-archiving.yml @@ -0,0 +1,5 @@ +--- +title: Add documentation to summarise project archiving +merge_request: 14650 +author: +type: other diff --git a/changelogs/unreleased/ff_port_from_ee.yml b/changelogs/unreleased/ff_port_from_ee.yml new file mode 100644 index 00000000000..e1cb7804a47 --- /dev/null +++ b/changelogs/unreleased/ff_port_from_ee.yml @@ -0,0 +1,5 @@ +--- +title: Move Custom merge methods from EE +merge_request: +author: +type: added diff --git a/changelogs/unreleased/fix-kubectl-180.yml b/changelogs/unreleased/fix-kubectl-180.yml new file mode 100644 index 00000000000..beb71cecd57 --- /dev/null +++ b/changelogs/unreleased/fix-kubectl-180.yml @@ -0,0 +1,5 @@ +--- +title: 'Kubernetes integration: ensure v1.8.0 compatibility' +merge_request: 14635 +author: +type: fixed diff --git a/changelogs/unreleased/gem-sm-bump-google-api-client-gem-from-0-8-6-to-0-13-6.yml b/changelogs/unreleased/gem-sm-bump-google-api-client-gem-from-0-8-6-to-0-13-6.yml new file mode 100644 index 00000000000..13ec113167f --- /dev/null +++ b/changelogs/unreleased/gem-sm-bump-google-api-client-gem-from-0-8-6-to-0-13-6.yml @@ -0,0 +1,5 @@ +--- +title: Bump google-api-client Gem from 0.8.6 to 0.13.6 +merge_request: +author: +type: other diff --git a/changelogs/unreleased/mr-widget-merged-date-tooltip.yml b/changelogs/unreleased/mr-widget-merged-date-tooltip.yml new file mode 100644 index 00000000000..ea22993ff52 --- /dev/null +++ b/changelogs/unreleased/mr-widget-merged-date-tooltip.yml @@ -0,0 +1,5 @@ +--- +title: Fixed merge request widget merged & closed date tooltip text +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/update-pages-0-6.yml b/changelogs/unreleased/update-pages-0-6.yml new file mode 100644 index 00000000000..507bb4d78e9 --- /dev/null +++ b/changelogs/unreleased/update-pages-0-6.yml @@ -0,0 +1,5 @@ +--- +title: Update GitLab Pages to v0.6.0 +merge_request: 14630 +author: +type: other diff --git a/changelogs/unreleased/winh-sprintf.yml b/changelogs/unreleased/winh-sprintf.yml new file mode 100644 index 00000000000..f8ae5932ae4 --- /dev/null +++ b/changelogs/unreleased/winh-sprintf.yml @@ -0,0 +1,5 @@ +--- +title: Add basic sprintf implementation to JavaScript +merge_request: 14506 +author: +type: other diff --git a/config/environments/test.rb b/config/environments/test.rb index 278144b8943..1edb6fd39b8 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -16,7 +16,7 @@ Rails.application.configure do config.cache_classes = ENV['CACHE_CLASSES'] == 'true' # Configure static asset server for tests with Cache-Control for performance - config.assets.digest = false + config.assets.compile = false if ENV['CI'] config.serve_static_files = true config.static_cache_control = "public, max-age=3600" diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 9b496822e93..c5c50c216e1 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -89,7 +89,7 @@ production: &base # This happens when the commit is pushed or merged into the default branch of a project. # When not specified the default issue_closing_pattern as specified below will be used. # Tip: you can test your closing pattern at http://rubular.com. - # issue_closing_pattern: '((?:[Cc]los(?:e[sd]?|ing)|[Ff]ix(?:e[sd]|ing)?|[Rr]esolv(?:e[sd]?|ing))(:?) +(?:(?:issues? +)?%{issue_ref}(?:(?:, *| +and +)?)|([A-Z][A-Z0-9_]+-\d+))+)' + # issue_closing_pattern: '((?:[Cc]los(?:e[sd]?|ing)|[Ff]ix(?:e[sd]|ing)?|[Rr]esolv(?:e[sd]?|ing)|[Ii]mplement(?:s|ed|ing)?)(:?) +(?:(?:issues? +)?%{issue_ref}(?:(?:, *| +and +)?)|([A-Z][A-Z0-9_]+-\d+))+)' ## Default project features settings default_projects_features: diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 27c1ecc7b23..a23b3208dab 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -257,7 +257,7 @@ Settings.gitlab['signup_enabled'] ||= true if Settings.gitlab['signup_enabled']. Settings.gitlab['password_authentication_enabled'] ||= true if Settings.gitlab['password_authentication_enabled'].nil? Settings.gitlab['restricted_visibility_levels'] = Settings.__send__(:verify_constant_array, Gitlab::VisibilityLevel, Settings.gitlab['restricted_visibility_levels'], []) Settings.gitlab['username_changing_enabled'] = true if Settings.gitlab['username_changing_enabled'].nil? -Settings.gitlab['issue_closing_pattern'] = '((?:[Cc]los(?:e[sd]?|ing)|[Ff]ix(?:e[sd]|ing)?|[Rr]esolv(?:e[sd]?|ing))(:?) +(?:(?:issues? +)?%{issue_ref}(?:(?:, *| +and +)?)|([A-Z][A-Z0-9_]+-\d+))+)' if Settings.gitlab['issue_closing_pattern'].nil? +Settings.gitlab['issue_closing_pattern'] = '((?:[Cc]los(?:e[sd]?|ing)|[Ff]ix(?:e[sd]|ing)?|[Rr]esolv(?:e[sd]?|ing)|[Ii]mplement(?:s|ed|ing)?)(:?) +(?:(?:issues? +)?%{issue_ref}(?:(?:, *| +and +)?)|([A-Z][A-Z0-9_]+-\d+))+)' if Settings.gitlab['issue_closing_pattern'].nil? Settings.gitlab['default_projects_features'] ||= {} Settings.gitlab['webhook_timeout'] ||= 10 Settings.gitlab['max_attachment_size'] ||= 10 diff --git a/config/initializers/grpc.rb b/config/initializers/grpc.rb new file mode 100644 index 00000000000..b96962fe7db --- /dev/null +++ b/config/initializers/grpc.rb @@ -0,0 +1,11 @@ +require 'logger' + +GRPC_LOGGER = Logger.new(Rails.root.join('log/grpc.log')) +GRPC_LOGGER.level = ENV['GRPC_LOG_LEVEL'].presence || 'WARN' +GRPC_LOGGER.progname = 'GRPC' + +module GRPC + def self.logger + GRPC_LOGGER + end +end diff --git a/db/migrate/20141126120926_add_merge_request_rebase_enabled_to_projects.rb b/db/migrate/20141126120926_add_merge_request_rebase_enabled_to_projects.rb new file mode 100644 index 00000000000..3dafdf0fde4 --- /dev/null +++ b/db/migrate/20141126120926_add_merge_request_rebase_enabled_to_projects.rb @@ -0,0 +1,17 @@ +# rubocop:disable all +class AddMergeRequestRebaseEnabledToProjects < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column_with_default(:projects, :merge_requests_rebase_enabled, :boolean, default: false) + end + + def down + remove_column(:projects, :merge_requests_rebase_enabled) + end +end diff --git a/db/migrate/20150827121444_add_fast_forward_option_to_project.rb b/db/migrate/20150827121444_add_fast_forward_option_to_project.rb new file mode 100644 index 00000000000..014f5b2f372 --- /dev/null +++ b/db/migrate/20150827121444_add_fast_forward_option_to_project.rb @@ -0,0 +1,17 @@ +# rubocop:disable all +class AddFastForwardOptionToProject < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + disable_ddl_transaction! + + def add + add_column_with_default(:projects, :merge_requests_ff_only_enabled, :boolean, default: false) + end + + def down + remove_column(:projects, :merge_requests_ff_only_enabled) + end +end diff --git a/db/migrate/20170927122209_add_partial_index_for_labels_template.rb b/db/migrate/20170927122209_add_partial_index_for_labels_template.rb new file mode 100644 index 00000000000..c3e5077ba20 --- /dev/null +++ b/db/migrate/20170927122209_add_partial_index_for_labels_template.rb @@ -0,0 +1,45 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddPartialIndexForLabelsTemplate < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index", "remove_concurrent_index" or + # "add_column_with_default" you must disable the use of transactions + # as these methods can not run in an existing transaction. + # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure + # that either of them is the _only_ method called in the migration, + # any other changes should go in a separate migration. + # This ensures that upon failure _only_ the index creation or removing fails + # and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + + disable_ddl_transaction! + + # Note this is a partial index in Postgres but MySQL will ignore the + # partial index clause. By making it an index on "template" this + # means the index will still accomplish the same goal of optimizing + # a query with "where template = true" on MySQL -- it'll just take + # more space. In this case the number of records with template=true + # is expected to be very small (small enough to display on a single + # web page) so it's ok to filter or sort them without the index + # anyways. + + def up + add_concurrent_index "labels", ["template"], where: "template" + end + + def down + remove_concurrent_index "labels", ["template"], where: "template" + end +end diff --git a/db/schema.rb b/db/schema.rb index fedd28bff7d..9fc8191751c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -759,6 +759,7 @@ ActiveRecord::Schema.define(version: 20170928100231) do add_index "labels", ["group_id", "project_id", "title"], name: "index_labels_on_group_id_and_project_id_and_title", unique: true, using: :btree add_index "labels", ["project_id"], name: "index_labels_on_project_id", using: :btree + add_index "labels", ["template"], name: "index_labels_on_template", where: "template", using: :btree add_index "labels", ["title"], name: "index_labels_on_title", using: :btree add_index "labels", ["type", "project_id"], name: "index_labels_on_type_and_project_id", using: :btree @@ -1246,6 +1247,8 @@ ActiveRecord::Schema.define(version: 20170928100231) do t.integer "storage_version", limit: 2 t.boolean "resolve_outdated_diff_discussions" t.boolean "repository_read_only" + t.boolean "merge_requests_ff_only_enabled", default: false + t.boolean "merge_requests_rebase_enabled", default: false, null: false end add_index "projects", ["ci_id"], name: "index_projects_on_ci_id", using: :btree diff --git a/doc/administration/gitaly/index.md b/doc/administration/gitaly/index.md index 40099dcc967..e3b10119090 100644 --- a/doc/administration/gitaly/index.md +++ b/doc/administration/gitaly/index.md @@ -32,6 +32,14 @@ prometheus_listen_addr = "localhost:9236" Changes to `/home/git/gitaly/config.toml` are applied when you run `service gitlab restart`. +## Client-side GRPC logs + +Gitaly uses the [gRPC](https://grpc.io/) RPC framework. The Ruby gRPC +client has its own log file which may contain useful information when +you are seeing Gitaly errors. You can control the log level of the +gRPC client with the `GRPC_LOG_LEVEL` environment variable. The +default level is `WARN`. + ## Running Gitaly on its own server > This is an optional way to deploy Gitaly which can benefit GitLab diff --git a/doc/development/i18n_guide.md b/doc/development/i18n_guide.md index bd0ef39ca62..29c8941a8f7 100644 --- a/doc/development/i18n_guide.md +++ b/doc/development/i18n_guide.md @@ -183,13 +183,20 @@ aren't in the message with id `1 pipeline`. ### Interpolation -- In Ruby/HAML: +- In Ruby/HAML (see [sprintf]): ```ruby _("Hello %{name}") % { name: 'Joe' } ``` -- In JavaScript: Not supported at this moment. +- In JavaScript: Only named parameters are supported (see also [#37992]): + + ```javascript + __("Hello %{name}") % { name: 'Joe' } + ``` + +[sprintf]: http://ruby-doc.org/core/Kernel.html#method-i-sprintf +[#37992]: https://gitlab.com/gitlab-org/gitlab-ce/issues/37992 ### Plurals diff --git a/doc/development/ux_guide/animation.md b/doc/development/ux_guide/animation.md index 5dae4bcc905..d190ee1b0ff 100644 --- a/doc/development/ux_guide/animation.md +++ b/doc/development/ux_guide/animation.md @@ -39,6 +39,12 @@ When information is updating in place, a quick, subtle animation is needed. The ![Quick update animation](img/animation-quickupdate.gif) +### Skeleton loading + +Skeleton loading is explained in the [component section](components.html#skeleton-loading) of the UX guide. It includes a horizontally pulsating animation that shows motion as if it's growing. It's timing is a slower `linear 1s`. + +![Skeleton loading animation](img/skeleton-loading.gif) + ### Moving transitions When elements move on screen, there should be a quick animation so it is clear to users what moved where. The timing of this animation differs based on the amount of movement and change. Consider animations between `200ms` and `400ms`. @@ -51,7 +57,9 @@ View the [interactive example](http://codepen.io/awhildy/full/ALyKPE/) here. ![Reorder animation](img/animation-reorder.gif) #### Autoscroll the page + Another example of a moving transition is when you have to autoscroll the page to keep an active element visible. View the [interactive example](http://codepen.io/awhildy/full/PbxgVo/) here. -![Autoscroll animation](img/animation-autoscroll.gif)
\ No newline at end of file + +![Autoscroll animation](img/animation-autoscroll.gif) diff --git a/doc/development/ux_guide/components.md b/doc/development/ux_guide/components.md index ac7c1b6207d..986b796437b 100644 --- a/doc/development/ux_guide/components.md +++ b/doc/development/ux_guide/components.md @@ -204,6 +204,25 @@ Cover blocks are generally used to create a heading element for a page, such as --- +## Skeleton loading + +Skeleton loading is a way to convey to the user what kind of content is currently being loaded. It's a paradigm with which content can independently and asynchronously be loaded, while still adhering to the structure and look of the completely loaded view. + +### Requirements + +* A skeleton should represent an organism in a recognisable way +* Atom elements within organisms (for reference see this article on [atomic design methodology](http://atomicdesign.bradfrost.com/chapter-2/)) may be represented in a maximum of 3 repetitions, if applicable. +* Skeletons should only be presented in grayscale using the HEX colors: `#fafafa` or `#ffffff` (except for shadows) +* Animate the grey atoms in a pulsating way to show motion, as if "loading". The pulse animation transitions colors horizontally from left to right, starting with `#f2f2f2` to `#fafafa`. + +![Skeleton loading animation](img/skeleton-loading.gif) + +### Usage + +Skeleton loading can replace any existing UI elements for the period in which they are loaded and should aim for maintaining a similar structure visually. + +--- + ## Panels > TODO: Catalog how we are currently using panels and rationalize how they relate to alerts diff --git a/doc/development/ux_guide/img/skeleton-loading.gif b/doc/development/ux_guide/img/skeleton-loading.gif Binary files differnew file mode 100644 index 00000000000..5877139171d --- /dev/null +++ b/doc/development/ux_guide/img/skeleton-loading.gif diff --git a/doc/user/project/merge_requests/fast_forward_merge.md b/doc/user/project/merge_requests/fast_forward_merge.md new file mode 100644 index 00000000000..085170d9f03 --- /dev/null +++ b/doc/user/project/merge_requests/fast_forward_merge.md @@ -0,0 +1,35 @@ +# Fast-forward merge requests + +Retain a linear Git history and a way to accept merge requests without +creating merge commits. + +## Overview + +When the fast-forward merge ([`--ff-only`][ffonly]) setting is enabled, no merge +commits will be created and all merges are fast-forwarded, which means that +merging is only allowed if the branch could be fast-forwarded. + +When a fast-forward merge is not possible, the user must rebase the branch manually. + +## Use cases + +Sometimes, a workflow policy might mandate a clean commit history without +merge commits. In such cases, the fast-forward merge is the perfect candidate. + +## Enabling fast-forward merges + +1. Navigate to your project's **Settings** and search for the 'Merge method' +1. Select the **Fast-forward merge** option +1. Hit **Save changes** for the changes to take effect + +Now, when you visit the merge request page, you will be able to accept it +**only if a fast-forward merge is possible**. + +![Fast forward merge request](img/ff_merge_mr.png) + +If the target branch is ahead of the source branch, you need to rebase the +source branch locally before you will be able to do a fast-forward merge. + +![Fast forward merge rebase locally](img/ff_merge_rebase_locally.png) + +[ffonly]: https://git-scm.com/docs/git-merge#git-merge---ff-only diff --git a/doc/user/project/merge_requests/img/ff_merge_mr.png b/doc/user/project/merge_requests/img/ff_merge_mr.png Binary files differnew file mode 100644 index 00000000000..241cc990343 --- /dev/null +++ b/doc/user/project/merge_requests/img/ff_merge_mr.png diff --git a/doc/user/project/merge_requests/img/ff_merge_rebase_locally.png b/doc/user/project/merge_requests/img/ff_merge_rebase_locally.png Binary files differnew file mode 100644 index 00000000000..fb412296efc --- /dev/null +++ b/doc/user/project/merge_requests/img/ff_merge_rebase_locally.png diff --git a/doc/user/project/merge_requests/index.md b/doc/user/project/merge_requests/index.md index 26c6277d33a..8e081b4f0b8 100644 --- a/doc/user/project/merge_requests/index.md +++ b/doc/user/project/merge_requests/index.md @@ -23,12 +23,14 @@ With GitLab merge requests, you can: - Organize your issues and merge requests consistently throughout the project with [labels](../../project/labels.md) - Add a time estimation and the time spent with that merge request with [Time Tracking](../../../workflow/time_tracking.html#time-tracking) - [Resolve merge conflicts from the UI](#resolve-conflicts) +- Enable [fast-forward merge requests](#fast-forward-merge-requests) +- Enable [semi-linear history merge requests](#semi-linear-history-merge-requests) as another security layer to guarantee the pipeline is passing in the target branch + With **[GitLab Enterprise Edition][ee]**, you can also: - View the deployment process across projects with [Multi-Project Pipeline Graphs](https://docs.gitlab.com/ee/ci/multi_project_pipeline_graphs.html#multi-project-pipeline-graphs) (available only in GitLab Enterprise Edition Premium) - Request [approvals](https://docs.gitlab.com/ee/user/project/merge_requests/merge_request_approvals.html) from your managers (available in GitLab Enterprise Edition Starter) -- Enable [fast-forward merge requests](https://docs.gitlab.com/ee/user/project/merge_requests/fast_forward_merge.html) (available in GitLab Enterprise Edition Starter) - [Squash and merge](https://docs.gitlab.com/ee/user/project/merge_requests/squash_and_merge.html) for a cleaner commit history (available in GitLab Enterprise Edition Starter) - Enable [semi-linear history merge requests](https://docs.gitlab.com/ee/user/project/merge_requests/index.html#semi-linear-history-merge-requests) as another security layer to guarantee the pipeline is passing in the target branch (available in GitLab Enterprise Edition Starter) - Analise the impact of your changes with [Code Quality reports](https://docs.gitlab.com/ee/user/project/merge_requests/code_quality_diff.html) (available in GitLab Enterprise Edition Starter) @@ -89,6 +91,22 @@ in a merged merge requests or a commit. [Learn more about cherry-picking changes.](cherry_pick_changes.md) +## Semi-linear history merge requests + +A merge commit is created for every merge, but the branch is only merged if +a fast-forward merge is possible. This ensures that if the merge request build +succeeded, the target branch build will also succeed after merging. + +Navigate to a project's settings, select the **Merge commit with semi-linear +history** option under **Merge Requests: Merge method** and save your changes. + +## Fast-forward merge requests + +If you prefer a linear Git history and a way to accept merge requests without +creating merge commits, you can configure this on a per-project basis. + +[Read more about fast-forward merge requests.](fast_forward_merge.md) + ## Merge when pipeline succeeds When reviewing a merge request that looks ready to merge but still has one or @@ -254,4 +272,4 @@ git checkout origin/merge-requests/1 ``` [protected branches]: ../protected_branches.md -[ee]: https://about.gitlab.com/gitlab-ee/ "GitLab Enterprise Edition"
\ No newline at end of file +[ee]: https://about.gitlab.com/gitlab-ee/ "GitLab Enterprise Edition" diff --git a/doc/user/project/settings/index.md b/doc/user/project/settings/index.md index 22c343dc027..a234a647b77 100644 --- a/doc/user/project/settings/index.md +++ b/doc/user/project/settings/index.md @@ -23,7 +23,7 @@ Add an [issue description template](../description_templates.md#description-temp Set up your project's merge request settings: -- Set up the merge request method (merge commit, [fast-forward merge](https://docs.gitlab.com/ee/user/project/merge_requests/fast_forward_merge.html#fast-forward-merge-requests)). _Fast-forward is available in [GitLab Enterprise Edition Starter](https://about.gitlab.com/gitlab-ee/)._ +- Set up the merge request method (merge commit, [fast-forward merge](../merge_requests/fast_forward_merge.html)). - Merge request [description templates](../description_templates.md#description-templates). - Enable [merge request approvals](https://docs.gitlab.com/ee/user/project/merge_requests/merge_request_approvals.html#merge-request-approvals), _available in [GitLab Enterprise Edition Starter](https://about.gitlab.com/gitlab-ee/)_. - Enable [merge only of pipeline succeeds](../merge_requests/merge_when_pipeline_succeeds.md). @@ -42,3 +42,11 @@ Learn how to [export a project](import_export.md#importing-the-project) in GitLa ### Advanced settings Here you can run housekeeping, archive, rename, transfer, or remove a project. + +#### Archiving a project + +>**Note:** Only Project Owners and Admin users have the permission to archive a project + +It's possible to mark a project as archived via the Project Settings. An archived project will be hidden by default in the project listings. + +An archived project can be fully restored and will therefore retain it's repository and all associated resources whilst in an archived state. diff --git a/doc/workflow/README.md b/doc/workflow/README.md index 673e08287a3..6b2aba47f54 100644 --- a/doc/workflow/README.md +++ b/doc/workflow/README.md @@ -36,6 +36,7 @@ - [Revert changes in the UI](../user/project/merge_requests/revert_changes.md) - [Merge requests versions](../user/project/merge_requests/versions.md) - ["Work In Progress" merge requests](../user/project/merge_requests/work_in_progress_merge_requests.md) + - [Fast-forward merge requests](../user/project/merge_requests/fast_forward_merge.md) - [Manage large binaries with Git LFS](lfs/manage_large_binaries_with_git_lfs.md) - [Importing from SVN, GitHub, Bitbucket, etc](importing/README.md) - [Todos](todos.md) diff --git a/features/project/ff_merge_requests.feature b/features/project/ff_merge_requests.feature new file mode 100644 index 00000000000..995e52f9332 --- /dev/null +++ b/features/project/ff_merge_requests.feature @@ -0,0 +1,24 @@ +Feature: Project Ff Merge Requests + Background: + Given I sign in as a user + And I own project "Shop" + And project "Shop" have "Bug NS-05" open merge request with diffs inside + And merge request "Bug NS-05" is mergeable + + @javascript + Scenario: I do ff-only merge for rebased branch + Given ff merge enabled + And merge request "Bug NS-05" is rebased + When I visit merge request page "Bug NS-05" + Then I should see ff-only merge button + When I accept this merge request + Then I should see merged request + + @javascript + Scenario: I do ff-only merge for merged branch + Given ff merge enabled + And merge request "Bug NS-05" merged target + When I visit merge request page "Bug NS-05" + Then I should see ff-only merge button + When I accept this merge request + Then I should see merged request diff --git a/features/steps/project/ff_merge_requests.rb b/features/steps/project/ff_merge_requests.rb new file mode 100644 index 00000000000..d68fe71e16e --- /dev/null +++ b/features/steps/project/ff_merge_requests.rb @@ -0,0 +1,65 @@ +class Spinach::Features::ProjectFfMergeRequests < Spinach::FeatureSteps + include SharedAuthentication + include SharedIssuable + include SharedProject + include SharedNote + include SharedPaths + include SharedMarkdown + include SharedDiffNote + include SharedUser + include WaitForRequests + + step 'project "Shop" have "Bug NS-05" open merge request with diffs inside' do + create(:merge_request_with_diffs, + title: "Bug NS-05", + source_project: project, + target_project: project, + author: project.users.first) + end + + step 'I should see ff-only merge button' do + expect(page).to have_content "Fast-forward merge without a merge commit" + expect(page).to have_button 'Merge' + end + + step 'merge request "Bug NS-05" is mergeable' do + merge_request.mark_as_mergeable + end + + step 'I accept this merge request' do + page.within '.mr-state-widget' do + click_button "Merge" + end + end + + step 'I should see merged request' do + page.within '.status-box' do + expect(page).to have_content "Merged" + wait_for_requests + end + end + + step 'ff merge enabled' do + project = merge_request.target_project + project.merge_requests_ff_only_enabled = true + project.save! + end + + step 'merge request "Bug NS-05" is rebased' do + merge_request.source_branch = 'flatten-dir' + merge_request.target_branch = 'improve/awesome' + merge_request.reload_diff + merge_request.save! + end + + step 'merge request "Bug NS-05" merged target' do + merge_request.source_branch = 'merged-target' + merge_request.target_branch = 'improve/awesome' + merge_request.reload_diff + merge_request.save! + end + + def merge_request + @merge_request ||= MergeRequest.find_by!(title: "Bug NS-05") + end +end diff --git a/features/steps/shared/diff_note.rb b/features/steps/shared/diff_note.rb index 2c59ec5bb06..c872bd6f861 100644 --- a/features/steps/shared/diff_note.rb +++ b/features/steps/shared/diff_note.rb @@ -232,7 +232,7 @@ module SharedDiffNote end def click_parallel_diff_line(code, line_type) - find(".line_holder.parallel .diff-line-num[id='#{code}']").trigger 'mouseover' + find(".line_holder.parallel td[id='#{code}']").find(:xpath, 'preceding-sibling::*[1][self::td]').trigger 'mouseover' find(".line_holder.parallel button[data-line-code='#{code}']").trigger 'click' end end diff --git a/lib/banzai/filter/sanitization_filter.rb b/lib/banzai/filter/sanitization_filter.rb index 88b17e12576..d8c8deea628 100644 --- a/lib/banzai/filter/sanitization_filter.rb +++ b/lib/banzai/filter/sanitization_filter.rb @@ -73,8 +73,9 @@ module Banzai return unless node.has_attribute?('href') begin + node['href'] = node['href'].strip uri = Addressable::URI.parse(node['href']) - uri.scheme = uri.scheme.strip.downcase if uri.scheme + uri.scheme = uri.scheme.downcase if uri.scheme node.remove_attribute('href') if UNSAFE_PROTOCOLS.include?(uri.scheme) rescue Addressable::URI::InvalidURIError diff --git a/lib/gitlab/git/user.rb b/lib/gitlab/git/user.rb index cb1af5f3b7c..da74719ae87 100644 --- a/lib/gitlab/git/user.rb +++ b/lib/gitlab/git/user.rb @@ -7,6 +7,11 @@ module Gitlab new(gitlab_user.username, gitlab_user.name, gitlab_user.email, Gitlab::GlId.gl_id(gitlab_user)) end + # TODO support the username field in Gitaly https://gitlab.com/gitlab-org/gitaly/issues/628 + def self.from_gitaly(gitaly_user) + new('', gitaly_user.name, gitaly_user.email, gitaly_user.gl_id) + end + def initialize(username, name, email, gl_id) @username = username @name = name diff --git a/lib/gitlab/git/wiki.rb b/lib/gitlab/git/wiki.rb new file mode 100644 index 00000000000..d651c931a38 --- /dev/null +++ b/lib/gitlab/git/wiki.rb @@ -0,0 +1,115 @@ +module Gitlab + module Git + class Wiki + DuplicatePageError = Class.new(StandardError) + + CommitDetails = Struct.new(:name, :email, :message) do + def to_h + { name: name, email: email, message: message } + end + end + + def self.default_ref + 'master' + end + + # Initialize with a Gitlab::Git::Repository instance + def initialize(repository) + @repository = repository + end + + def repository_exists? + @repository.exists? + end + + def write_page(name, format, content, commit_details) + assert_type!(format, Symbol) + assert_type!(commit_details, CommitDetails) + + gollum_wiki.write_page(name, format, content, commit_details.to_h) + + nil + rescue Gollum::DuplicatePageError => e + raise Gitlab::Git::Wiki::DuplicatePageError, e.message + end + + def delete_page(page_path, commit_details) + assert_type!(commit_details, CommitDetails) + + gollum_wiki.delete_page(gollum_page_by_path(page_path), commit_details.to_h) + nil + end + + def update_page(page_path, title, format, content, commit_details) + assert_type!(format, Symbol) + assert_type!(commit_details, CommitDetails) + + gollum_wiki.update_page(gollum_page_by_path(page_path), title, format, content, commit_details.to_h) + nil + end + + def pages + gollum_wiki.pages.map { |gollum_page| new_page(gollum_page) } + end + + def page(title:, version: nil, dir: nil) + if version + version = Gitlab::Git::Commit.find(@repository, version).id + end + + gollum_page = gollum_wiki.page(title, version, dir) + return unless gollum_page + + new_page(gollum_page) + end + + def file(name, version) + version ||= self.class.default_ref + gollum_file = gollum_wiki.file(name, version) + return unless gollum_file + + Gitlab::Git::WikiFile.new(gollum_file) + end + + def page_versions(page_path) + current_page = gollum_page_by_path(page_path) + current_page.versions.map do |gollum_git_commit| + gollum_page = gollum_wiki.page(current_page.title, gollum_git_commit.id) + new_version(gollum_page, gollum_git_commit.id) + end + end + + def preview_slug(title, format) + gollum_wiki.preview_page(title, '', format).url_path + end + + private + + def gollum_wiki + @gollum_wiki ||= Gollum::Wiki.new(@repository.path) + end + + def gollum_page_by_path(page_path) + page_name = Gollum::Page.canonicalize_filename(page_path) + page_dir = File.split(page_path).first + + gollum_wiki.paged(page_name, page_dir) + end + + def new_page(gollum_page) + Gitlab::Git::WikiPage.new(gollum_page, new_version(gollum_page, gollum_page.version.id)) + end + + def new_version(gollum_page, commit_id) + commit = Gitlab::Git::Commit.find(@repository, commit_id) + Gitlab::Git::WikiPageVersion.new(commit, gollum_page&.format) + end + + def assert_type!(object, klass) + unless object.is_a?(klass) + raise ArgumentError, "expected a #{klass}, got #{object.inspect}" + end + end + end + end +end diff --git a/lib/gitlab/git/wiki_file.rb b/lib/gitlab/git/wiki_file.rb new file mode 100644 index 00000000000..527f2a44dea --- /dev/null +++ b/lib/gitlab/git/wiki_file.rb @@ -0,0 +1,19 @@ +module Gitlab + module Git + class WikiFile + attr_reader :mime_type, :raw_data, :name + + # This class is meant to be serializable so that it can be constructed + # by Gitaly and sent over the network to GitLab. + # + # Because Gollum::File is not serializable we must get all the data from + # 'gollum_file' during initialization, and NOT store it in an instance + # variable. + def initialize(gollum_file) + @mime_type = gollum_file.mime_type + @raw_data = gollum_file.raw_data + @name = gollum_file.name + end + end + end +end diff --git a/lib/gitlab/git/wiki_page.rb b/lib/gitlab/git/wiki_page.rb new file mode 100644 index 00000000000..a06bac4414f --- /dev/null +++ b/lib/gitlab/git/wiki_page.rb @@ -0,0 +1,39 @@ +module Gitlab + module Git + class WikiPage + attr_reader :url_path, :title, :format, :path, :version, :raw_data, :name, :text_data, :historical + + # This class is meant to be serializable so that it can be constructed + # by Gitaly and sent over the network to GitLab. + # + # Because Gollum::Page is not serializable we must get all the data from + # 'gollum_page' during initialization, and NOT store it in an instance + # variable. + # + # Note that 'version' is a WikiPageVersion instance which it itself + # serializable. That means it's OK to store 'version' in an instance + # variable. + def initialize(gollum_page, version) + @url_path = gollum_page.url_path + @title = gollum_page.title + @format = gollum_page.format + @path = gollum_page.path + @raw_data = gollum_page.raw_data + @name = gollum_page.name + @historical = gollum_page.historical? + + @version = version + end + + def historical? + @historical + end + + def text_data + return @text_data if defined?(@text_data) + + @text_data = @raw_data && Gitlab::EncodingHelper.encode!(@raw_data.dup) + end + end + end +end diff --git a/lib/gitlab/git/wiki_page_version.rb b/lib/gitlab/git/wiki_page_version.rb new file mode 100644 index 00000000000..55f1afedcab --- /dev/null +++ b/lib/gitlab/git/wiki_page_version.rb @@ -0,0 +1,19 @@ +module Gitlab + module Git + class WikiPageVersion + attr_reader :commit, :format + + # This class is meant to be serializable so that it can be constructed + # by Gitaly and sent over the network to GitLab. + # + # Both 'commit' (a Gitlab::Git::Commit) and 'format' (a string) are + # serializable. + def initialize(commit, format) + @commit = commit + @format = format + end + + delegate :message, :sha, :id, :author_name, :authored_date, to: :commit + end + end +end diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index e75e0500ed8..87b300dcf7e 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -233,6 +233,8 @@ module Gitlab end def self.encode(s) + return "" if s.nil? + s.dup.force_encoding(Encoding::ASCII_8BIT) end diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index 36da63fd586..a2b50f2507e 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -274,7 +274,7 @@ module Gitlab repository: @gitaly_repo, left_commit_id: from_id, right_commit_id: to_id, - paths: options.fetch(:paths, []).map { |path| GitalyClient.encode(path) } + paths: options.fetch(:paths, []).compact.map { |path| GitalyClient.encode(path) } } end diff --git a/lib/gitlab/kubernetes.rb b/lib/gitlab/kubernetes.rb index cdbdfa10d0e..da43bd0af4b 100644 --- a/lib/gitlab/kubernetes.rb +++ b/lib/gitlab/kubernetes.rb @@ -113,7 +113,7 @@ module Gitlab def kubeconfig_embed_ca_pem(config, ca_pem) cluster = config.dig(:clusters, 0, :cluster) - cluster[:'certificate-authority-data'] = Base64.encode64(ca_pem) + cluster[:'certificate-authority-data'] = Base64.strict_encode64(ca_pem) end end end diff --git a/lib/gitlab/url_sanitizer.rb b/lib/gitlab/url_sanitizer.rb index 4e1ec1402ea..1caa791c1be 100644 --- a/lib/gitlab/url_sanitizer.rb +++ b/lib/gitlab/url_sanitizer.rb @@ -1,7 +1,9 @@ module Gitlab class UrlSanitizer + ALLOWED_SCHEMES = %w[http https ssh git].freeze + def self.sanitize(content) - regexp = URI::Parser.new.make_regexp(%w(http https ssh git)) + regexp = URI::Parser.new.make_regexp(ALLOWED_SCHEMES) content.gsub(regexp) { |url| new(url).masked_url } rescue Addressable::URI::InvalidURIError @@ -11,9 +13,9 @@ module Gitlab def self.valid?(url) return false unless url.present? - Addressable::URI.parse(url.strip) + uri = Addressable::URI.parse(url.strip) - true + ALLOWED_SCHEMES.include?(uri.scheme) rescue Addressable::URI::InvalidURIError false end diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb index 45f246242f1..f200c694562 100644 --- a/lib/gitlab/workhorse.rb +++ b/lib/gitlab/workhorse.rb @@ -89,6 +89,13 @@ module Gitlab params = repository.archive_metadata(ref, Gitlab.config.gitlab.repository_downloads_path, format) raise "Repository or ref not found" if params.empty? + if Gitlab::GitalyClient.feature_enabled?(:workhorse_archive) + params.merge!( + 'GitalyServer' => gitaly_server_hash(repository), + 'GitalyRepository' => repository.gitaly_repository.to_h + ) + end + [ SEND_DATA_HEADER, "git-archive:#{encode(params)}" diff --git a/lib/system_check/app/git_user_default_ssh_config_check.rb b/lib/system_check/app/git_user_default_ssh_config_check.rb index 7b486d78cf0..dfa8b8b3f5b 100644 --- a/lib/system_check/app/git_user_default_ssh_config_check.rb +++ b/lib/system_check/app/git_user_default_ssh_config_check.rb @@ -5,6 +5,7 @@ module SystemCheck # whitelisted as it may change the SSH client's behaviour dramatically. WHITELIST = %w[ authorized_keys + authorized_keys.lock authorized_keys2 known_hosts ].freeze diff --git a/qa/qa/page/project/show.rb b/qa/qa/page/project/show.rb index 56a270d8fcc..68d9597c4d2 100644 --- a/qa/qa/page/project/show.rb +++ b/qa/qa/page/project/show.rb @@ -5,8 +5,8 @@ module QA def choose_repository_clone_http find('#clone-dropdown').click - page.within('#clone-dropdown') do - find('span', text: 'HTTP').click + page.within('.clone-options-dropdown') do + click_link('HTTP') end end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index b4a22a46b51..053bd73fee3 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -207,162 +207,6 @@ describe Projects::IssuesController do end end - describe 'PUT #update' do - before do - sign_in(user) - project.team << [user, :developer] - end - - it_behaves_like 'update invalid issuable', Issue - - context 'changing the assignee' do - it 'limits the attributes exposed on the assignee' do - assignee = create(:user) - project.add_developer(assignee) - - put :update, - namespace_id: project.namespace.to_param, - project_id: project, - id: issue.iid, - issue: { assignee_ids: [assignee.id] }, - format: :json - body = JSON.parse(response.body) - - expect(body['assignees'].first.keys) - .to match_array(%w(id name username avatar_url state web_url)) - end - end - - context 'Akismet is enabled' do - let(:project) { create(:project_empty_repo, :public) } - - before do - stub_application_setting(recaptcha_enabled: true) - allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true) - end - - context 'when an issue is not identified as spam' do - before do - allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false) - allow_any_instance_of(AkismetService).to receive(:spam?).and_return(false) - end - - it 'normally updates the issue' do - expect { update_issue(title: 'Foo') }.to change { issue.reload.title }.to('Foo') - end - end - - context 'when an issue is identified as spam' do - before do - allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) - end - - context 'when captcha is not verified' do - def update_spam_issue - update_issue(title: 'Spam Title', description: 'Spam lives here') - end - - before do - allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false) - end - - it 'rejects an issue recognized as a spam' do - expect(Gitlab::Recaptcha).to receive(:load_configurations!).and_return(true) - expect { update_spam_issue }.not_to change { issue.reload.title } - end - - it 'rejects an issue recognized as a spam when recaptcha disabled' do - stub_application_setting(recaptcha_enabled: false) - - expect { update_spam_issue }.not_to change { issue.reload.title } - end - - it 'creates a spam log' do - update_spam_issue - - spam_logs = SpamLog.all - - expect(spam_logs.count).to eq(1) - expect(spam_logs.first.title).to eq('Spam Title') - expect(spam_logs.first.recaptcha_verified).to be_falsey - end - - context 'as HTML' do - it 'renders verify template' do - update_spam_issue - - expect(response).to render_template(:verify) - end - end - - context 'as JSON' do - before do - update_issue({ title: 'Spam Title', description: 'Spam lives here' }, format: :json) - end - - it 'renders json errors' do - expect(json_response) - .to eql("errors" => ["Your issue has been recognized as spam. Please, change the content or solve the reCAPTCHA to proceed."]) - end - - it 'returns 422 status' do - expect(response).to have_http_status(422) - end - end - end - - context 'when captcha is verified' do - let(:spammy_title) { 'Whatever' } - let!(:spam_logs) { create_list(:spam_log, 2, user: user, title: spammy_title) } - - def update_verified_issue - update_issue({ title: spammy_title }, - { spam_log_id: spam_logs.last.id, - recaptcha_verification: true }) - end - - before do - allow_any_instance_of(described_class).to receive(:verify_recaptcha) - .and_return(true) - end - - it 'redirect to issue page' do - update_verified_issue - - expect(response) - .to redirect_to(project_issue_path(project, issue)) - end - - it 'accepts an issue after recaptcha is verified' do - expect { update_verified_issue }.to change { issue.reload.title }.to(spammy_title) - end - - it 'marks spam log as recaptcha_verified' do - expect { update_verified_issue }.to change { SpamLog.last.recaptcha_verified }.from(false).to(true) - end - - it 'does not mark spam log as recaptcha_verified when it does not belong to current_user' do - spam_log = create(:spam_log) - - expect { update_issue(spam_log_id: spam_log.id, recaptcha_verification: true) } - .not_to change { SpamLog.last.recaptcha_verified } - end - end - end - - def update_issue(issue_params = {}, additional_params = {}) - params = { - namespace_id: project.namespace.to_param, - project_id: project, - id: issue.iid, - issue: issue_params - }.merge(additional_params) - - put :update, params - end - end - end - describe 'POST #move' do before do sign_in(user) @@ -533,6 +377,146 @@ describe Projects::IssuesController do end end + describe 'PUT #update' do + def update_issue(issue_params: {}, additional_params: {}, id: nil) + id ||= issue.iid + params = { + namespace_id: project.namespace.to_param, + project_id: project, + id: id, + issue: { title: 'New title' }.merge(issue_params), + format: :json + }.merge(additional_params) + + put :update, params + end + + def go(id:) + update_issue(id: id) + end + + before do + sign_in(user) + project.team << [user, :developer] + end + + it_behaves_like 'restricted action', success: 200 + it_behaves_like 'update invalid issuable', Issue + + context 'changing the assignee' do + it 'limits the attributes exposed on the assignee' do + assignee = create(:user) + project.add_developer(assignee) + + update_issue(issue_params: { assignee_ids: [assignee.id] }) + + body = JSON.parse(response.body) + + expect(body['assignees'].first.keys) + .to match_array(%w(id name username avatar_url state web_url)) + end + end + + context 'Akismet is enabled' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + stub_application_setting(recaptcha_enabled: true) + allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true) + end + + context 'when an issue is not identified as spam' do + before do + allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false) + allow_any_instance_of(AkismetService).to receive(:spam?).and_return(false) + end + + it 'normally updates the issue' do + expect { update_issue(issue_params: { title: 'Foo' }) }.to change { issue.reload.title }.to('Foo') + end + end + + context 'when an issue is identified as spam' do + before do + allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) + end + + context 'when captcha is not verified' do + before do + allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false) + end + + it 'rejects an issue recognized as a spam' do + expect { update_issue }.not_to change { issue.reload.title } + end + + it 'rejects an issue recognized as a spam when recaptcha disabled' do + stub_application_setting(recaptcha_enabled: false) + + expect { update_issue }.not_to change { issue.reload.title } + end + + it 'creates a spam log' do + update_issue(issue_params: { title: 'Spam title' }) + + spam_logs = SpamLog.all + + expect(spam_logs.count).to eq(1) + expect(spam_logs.first.title).to eq('Spam title') + expect(spam_logs.first.recaptcha_verified).to be_falsey + end + + it 'renders json errors' do + update_issue + + expect(json_response) + .to eql("errors" => ["Your issue has been recognized as spam. Please, change the content or solve the reCAPTCHA to proceed."]) + end + + it 'returns 422 status' do + update_issue + + expect(response).to have_http_status(422) + end + end + + context 'when captcha is verified' do + let(:spammy_title) { 'Whatever' } + let!(:spam_logs) { create_list(:spam_log, 2, user: user, title: spammy_title) } + + def update_verified_issue + update_issue( + issue_params: { title: spammy_title }, + additional_params: { spam_log_id: spam_logs.last.id, recaptcha_verification: true }) + end + + before do + allow_any_instance_of(described_class).to receive(:verify_recaptcha) + .and_return(true) + end + + it 'returns 200 status' do + expect(response).to have_http_status(200) + end + + it 'accepts an issue after recaptcha is verified' do + expect { update_verified_issue }.to change { issue.reload.title }.to(spammy_title) + end + + it 'marks spam log as recaptcha_verified' do + expect { update_verified_issue }.to change { SpamLog.last.recaptcha_verified }.from(false).to(true) + end + + it 'does not mark spam log as recaptcha_verified when it does not belong to current_user' do + spam_log = create(:spam_log) + + expect { update_issue(issue_params: { spam_log_id: spam_log.id, recaptcha_verification: true }) } + .not_to change { SpamLog.last.recaptcha_verified } + end + end + end + end + end + describe 'GET #show' do it_behaves_like 'restricted action', success: 200 @@ -573,29 +557,6 @@ describe Projects::IssuesController do end end end - - describe 'GET #edit' do - it_behaves_like 'restricted action', success: 200 - - def go(id:) - get :edit, - namespace_id: project.namespace.to_param, - project_id: project, - id: id - end - end - - describe 'PUT #update' do - it_behaves_like 'restricted action', success: 302 - - def go(id:) - put :update, - namespace_id: project.namespace.to_param, - project_id: project, - id: id, - issue: { title: 'New title' } - end - end end describe 'POST #create' do diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index fdd7e6f173f..d01339a0b88 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -216,7 +216,7 @@ describe Projects::JobsController do expect(json_response['text']).to eq status.text expect(json_response['label']).to eq status.label expect(json_response['icon']).to eq status.icon - expect(json_response['favicon']).to eq "/assets/ci_favicons/#{status.favicon}.ico" + expect(json_response['favicon']).to match_asset_path "/assets/ci_favicons/#{status.favicon}.ico" end end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 6775012bab5..629c131aee6 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -658,7 +658,7 @@ describe Projects::MergeRequestsController do expect(json_response['text']).to eq status.text expect(json_response['label']).to eq status.label expect(json_response['icon']).to eq status.icon - expect(json_response['favicon']).to eq "/assets/ci_favicons/#{status.favicon}.ico" + expect(json_response['favicon']).to match_asset_path "/assets/ci_favicons/#{status.favicon}.ico" end end diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index 6ffe41b8608..c0337f96fc6 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -120,6 +120,40 @@ describe Projects::NotesController do expect(note_json[:diff_discussion_html]).to be_nil end end + + context 'with cross-reference system note', :request_store do + let(:new_issue) { create(:issue) } + let(:cross_reference) { "mentioned in #{new_issue.to_reference(issue.project)}" } + + before do + note + create(:discussion_note_on_issue, :system, noteable: issue, project: issue.project, note: cross_reference) + end + + it 'filters notes that the user should not see' do + get :index, request_params + + expect(parsed_response[:notes].count).to eq(1) + expect(note_json[:id]).to eq(note.id) + end + + it 'does not result in N+1 queries' do + # Instantiate the controller variables to ensure QueryRecorder has an accurate base count + get :index, request_params + + RequestStore.clear! + + control_count = ActiveRecord::QueryRecorder.new do + get :index, request_params + end.count + + RequestStore.clear! + + create_list(:discussion_note_on_issue, 2, :system, noteable: issue, project: issue.project, note: cross_reference) + + expect { get :index, request_params }.not_to exceed_query_limit(control_count) + end + end end describe 'POST create' do diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index f9d77c7ad03..167e80ed9cd 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -142,7 +142,7 @@ describe Projects::PipelinesController do expect(json_response['text']).to eq status.text expect(json_response['label']).to eq status.label expect(json_response['icon']).to eq status.icon - expect(json_response['favicon']).to eq "/assets/ci_favicons/#{status.favicon}.ico" + expect(json_response['favicon']).to match_asset_path("/assets/ci_favicons/#{status.favicon}.ico") end end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 4459e227fb3..2a91a6613e6 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -289,6 +289,24 @@ describe ProjectsController do end end + it 'updates Fast Forward Merge attributes' do + controller.instance_variable_set(:@project, project) + + params = { + merge_method: :ff + } + + put :update, + namespace_id: project.namespace, + id: project.id, + project: params + + expect(response).to have_http_status(302) + params.each do |param, value| + expect(project.public_send(param)).to eq(value) + end + end + def update_project(**parameters) put :update, namespace_id: project.namespace.path, diff --git a/spec/features/copy_as_gfm_spec.rb b/spec/features/copy_as_gfm_spec.rb index dfeba722ac6..ebcd0ba0dcd 100644 --- a/spec/features/copy_as_gfm_spec.rb +++ b/spec/features/copy_as_gfm_spec.rb @@ -446,7 +446,7 @@ describe 'Copy as GFM', js: true do def verify(label, *gfms) aggregate_failures(label) do gfms.each do |gfm| - html = gfm_to_html(gfm) + html = gfm_to_html(gfm).gsub(/\A
|
\z/, '') output_gfm = html_to_gfm(html) expect(output_gfm.strip).to eq(gfm.strip) end @@ -463,42 +463,98 @@ describe 'Copy as GFM', js: true do let(:project) { create(:project, :repository) } context 'from a diff' do - before do - visit project_commit_path(project, sample_commit.id) - end + shared_examples 'copying code from a diff' do + context 'selecting one word of text' do + it 'copies as inline code' do + verify( + '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"] .line .no', - context 'selecting one word of text' do - it 'copies as inline code' do - verify( - '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"] .line .no', + '`RuntimeError`', - '`RuntimeError`' - ) + target: '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]' + ) + end end - end - context 'selecting one line of text' do - it 'copies as inline code' do - verify( - '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"] .line', + context 'selecting one line of text' do + it 'copies as inline code' do + verify( + '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]', - '`raise RuntimeError, "System commands must be given as an array of strings"`' - ) + '`raise RuntimeError, "System commands must be given as an array of strings"`', + + target: '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]' + ) + end + end + + context 'selecting multiple lines of text' do + it 'copies as a code block' do + verify( + '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10"]', + + <<-GFM.strip_heredoc, + ```ruby + raise RuntimeError, "System commands must be given as an array of strings" + end + ``` + GFM + + target: '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]' + ) + end end end - context 'selecting multiple lines of text' do - it 'copies as a code block' do - verify( - '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10"]', + context 'inline diff' do + before do + visit project_commit_path(project, sample_commit.id, view: 'inline') + end - <<-GFM.strip_heredoc, - ```ruby - raise RuntimeError, "System commands must be given as an array of strings" - end - ``` - GFM - ) + it_behaves_like 'copying code from a diff' + end + + context 'parallel diff' do + before do + visit project_commit_path(project, sample_commit.id, view: 'parallel') + end + + it_behaves_like 'copying code from a diff' + + context 'selecting code on the left' do + it 'copies as a code block' do + verify( + '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10"]', + + <<-GFM.strip_heredoc, + ```ruby + unless cmd.is_a?(Array) + raise "System commands must be given as an array of strings" + end + ``` + GFM + + target: '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8"].left-side' + ) + end + end + + context 'selecting code on the right' do + it 'copies as a code block' do + verify( + '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10"]', + + <<-GFM.strip_heredoc, + ```ruby + unless cmd.is_a?(Array) + raise RuntimeError, "System commands must be given as an array of strings" + end + ``` + GFM + + target: '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8"].right-side' + ) + end end end end @@ -587,9 +643,9 @@ describe 'Copy as GFM', js: true do end end - def verify(selector, gfm) + def verify(selector, gfm, target: nil) html = html_for_selector(selector) - output_gfm = html_to_gfm(html, 'transformCodeSelection') + output_gfm = html_to_gfm(html, 'transformCodeSelection', target: target) expect(output_gfm.strip).to eq(gfm.strip) end end @@ -605,15 +661,21 @@ describe 'Copy as GFM', js: true do page.evaluate_script(js) end - def html_to_gfm(html, transformer = 'transformGFMSelection') + def html_to_gfm(html, transformer = 'transformGFMSelection', target: nil) js = <<-JS.strip_heredoc (function(html) { var transformer = window.gl.CopyAsGFM[#{transformer.inspect}]; var node = document.createElement('div'); - node.innerHTML = html; + $(html).each(function() { node.appendChild(this) }); + + var targetSelector = #{target.to_json}; + var target; + if (targetSelector) { + target = document.querySelector(targetSelector); + } - node = transformer(node); + node = transformer(node, target); if (!node) return null; return window.gl.CopyAsGFM.nodeToGFM(node); diff --git a/spec/features/issues/form_spec.rb b/spec/features/issues/form_spec.rb index 2db6f9a2982..8ce470fc288 100644 --- a/spec/features/issues/form_spec.rb +++ b/spec/features/issues/form_spec.rb @@ -218,54 +218,15 @@ describe 'New/edit issue', :js do context 'edit issue' do before do - visit edit_project_issue_path(project, issue) - end - - it 'allows user to update issue' do - expect(find('input[name="issue[assignee_ids][]"]', visible: false).value).to match(user.id.to_s) - expect(find('input[name="issue[milestone_id]"]', visible: false).value).to match(milestone.id.to_s) - expect(find('a', text: 'Assign to me', visible: false)).not_to be_visible - - page.within '.js-user-search' do - expect(page).to have_content user.name - end - - page.within '.js-milestone-select' do - expect(page).to have_content milestone.title - end - - click_button 'Labels' - page.within '.dropdown-menu-labels' do - click_link label.title - click_link label2.title - end - page.within '.js-label-select' do - expect(page).to have_content label.title - end - expect(page.all('input[name="issue[label_ids][]"]', visible: false)[1].value).to match(label.id.to_s) - expect(page.all('input[name="issue[label_ids][]"]', visible: false)[2].value).to match(label2.id.to_s) - - click_button 'Save changes' - - page.within '.issuable-sidebar' do - page.within '.assignee' do - expect(page).to have_content user.name - end - - page.within '.milestone' do - expect(page).to have_content milestone.title - end - - page.within '.labels' do - expect(page).to have_content label.title - expect(page).to have_content label2.title - end + visit project_issue_path(project, issue) + page.within('.content .issuable-actions') do + click_on 'Edit' end end it 'description has autocomplete' do - find('#issue_description').native.send_keys('') - fill_in 'issue_description', with: '@' + find_field('issue-description').native.send_keys('') + fill_in 'issue-description', with: '@' expect(page).to have_selector('.atwho-view') end diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb index b4222edbcd0..aa8cf3b013c 100644 --- a/spec/features/issues_spec.rb +++ b/spec/features/issues_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe 'Issues' do +describe 'Issues', :js do include DropzoneHelper include IssueHelpers include SortingHelper @@ -24,109 +24,15 @@ describe 'Issues' do end before do - visit edit_project_issue_path(project, issue) - find('.js-zen-enter').click - end - - it 'opens new issue popup' do - expect(page).to have_content("Issue ##{issue.iid}") - end - end - - describe 'Editing issue assignee' do - let!(:issue) do - create(:issue, - author: user, - assignees: [user], - project: project) - end - - it 'allows user to select unassigned', js: true do - visit edit_project_issue_path(project, issue) - - expect(page).to have_content "Assignee #{user.name}" - - first('.js-user-search').click - click_link 'Unassigned' - - click_button 'Save changes' - - page.within('.assignee') do - expect(page).to have_content 'No assignee - assign yourself' - end - - expect(issue.reload.assignees).to be_empty - end - end - - describe 'due date', js: true do - context 'on new form' do - before do - visit new_project_issue_path(project) - end - - it 'saves with due date' do - date = Date.today.at_beginning_of_month - - fill_in 'issue_title', with: 'bug 345' - fill_in 'issue_description', with: 'bug description' - find('#issuable-due-date').click - - page.within '.pika-single' do - click_button date.day - end - - expect(find('#issuable-due-date').value).to eq date.to_s - - click_button 'Submit issue' - - page.within '.issuable-sidebar' do - expect(page).to have_content date.to_s(:medium) - end + visit project_issue_path(project, issue) + page.within('.content .issuable-actions') do + find('.issuable-edit').click end + find('.issue-details .content-block .js-zen-enter').click end - context 'on edit form' do - let(:issue) { create(:issue, author: user, project: project, due_date: Date.today.at_beginning_of_month.to_s) } - - before do - visit edit_project_issue_path(project, issue) - end - - it 'saves with due date' do - date = Date.today.at_beginning_of_month - - expect(find('#issuable-due-date').value).to eq date.to_s - - date = date.tomorrow - - fill_in 'issue_title', with: 'bug 345' - fill_in 'issue_description', with: 'bug description' - find('#issuable-due-date').click - - page.within '.pika-single' do - click_button date.day - end - - expect(find('#issuable-due-date').value).to eq date.to_s - - click_button 'Save changes' - - page.within '.issuable-sidebar' do - expect(page).to have_content date.to_s(:medium) - end - end - - it 'warns about version conflict' do - issue.update(title: "New title") - - fill_in 'issue_title', with: 'bug 345' - fill_in 'issue_description', with: 'bug description' - - click_button 'Save changes' - - expect(page).to have_content 'Someone edited the issue the same time you did' - end + it 'opens new issue popup' do + expect(page).to have_content(issue.description) end end diff --git a/spec/features/merge_requests/diff_notes_avatars_spec.rb b/spec/features/merge_requests/diff_notes_avatars_spec.rb index 9bcb78d5206..4766cdf716f 100644 --- a/spec/features/merge_requests/diff_notes_avatars_spec.rb +++ b/spec/features/merge_requests/diff_notes_avatars_spec.rb @@ -84,7 +84,7 @@ feature 'Diff note avatars', js: true do end it 'shows note avatar' do - page.within find("[id='#{position.line_code(project.repository)}']") do + page.within find_line(position.line_code(project.repository)) do find('.diff-notes-collapse').click expect(page).to have_selector('img.js-diff-comment-avatar', count: 1) @@ -92,7 +92,7 @@ feature 'Diff note avatars', js: true do end it 'shows comment on note avatar' do - page.within find("[id='#{position.line_code(project.repository)}']") do + page.within find_line(position.line_code(project.repository)) do find('.diff-notes-collapse').click expect(first('img.js-diff-comment-avatar')["data-original-title"]).to eq("#{note.author.name}: #{note.note.truncate(17)}") @@ -100,13 +100,13 @@ feature 'Diff note avatars', js: true do end it 'toggles comments when clicking avatar' do - page.within find("[id='#{position.line_code(project.repository)}']") do + page.within find_line(position.line_code(project.repository)) do find('.diff-notes-collapse').click end expect(page).to have_selector('.notes_holder', visible: false) - page.within find("[id='#{position.line_code(project.repository)}']") do + page.within find_line(position.line_code(project.repository)) do first('img.js-diff-comment-avatar').click end @@ -122,7 +122,7 @@ feature 'Diff note avatars', js: true do wait_for_requests - page.within find("[id='#{position.line_code(project.repository)}']") do + page.within find_line(position.line_code(project.repository)) do expect(page).not_to have_selector('img.js-diff-comment-avatar') end end @@ -138,7 +138,7 @@ feature 'Diff note avatars', js: true do wait_for_requests end - page.within find("[id='#{position.line_code(project.repository)}']") do + page.within find_line(position.line_code(project.repository)) do find('.diff-notes-collapse').trigger('click') expect(page).to have_selector('img.js-diff-comment-avatar', count: 2) @@ -158,7 +158,7 @@ feature 'Diff note avatars', js: true do end end - page.within find("[id='#{position.line_code(project.repository)}']") do + page.within find_line(position.line_code(project.repository)) do find('.diff-notes-collapse').trigger('click') expect(page).to have_selector('img.js-diff-comment-avatar', count: 3) @@ -176,7 +176,7 @@ feature 'Diff note avatars', js: true do end it 'shows extra comment count' do - page.within find("[id='#{position.line_code(project.repository)}']") do + page.within find_line(position.line_code(project.repository)) do find('.diff-notes-collapse').click expect(find('.diff-comments-more-count')).to have_content '+1' @@ -185,4 +185,10 @@ feature 'Diff note avatars', js: true do end end end + + def find_line(line_code) + line = find("[id='#{line_code}']") + line = line.find(:xpath, 'preceding-sibling::*[1][self::td]') if line.tag_name == 'td' + line + end end diff --git a/spec/features/merge_requests/widget_spec.rb b/spec/features/merge_requests/widget_spec.rb index 443b596b3c6..791cfa308c3 100644 --- a/spec/features/merge_requests/widget_spec.rb +++ b/spec/features/merge_requests/widget_spec.rb @@ -202,6 +202,28 @@ describe 'Merge request', :js do end end + context 'view merge request where fast-forward merge is not possible' do + before do + project.update(merge_requests_ff_only_enabled: true) + + merge_request.update( + merge_user: merge_request.author, + merge_status: :cannot_be_merged + ) + + visit project_merge_request_path(project, merge_request) + end + + it 'shows information about the merge error' do + # Wait for the `ci_status` and `merge_check` requests + wait_for_requests + + page.within('.mr-widget-body') do + expect(page).to have_content('Fast-forward merge is not possible') + end + end + end + context 'merge error' do before do allow_any_instance_of(Repository).to receive(:merge).and_return(false) diff --git a/spec/features/projects/issuable_templates_spec.rb b/spec/features/projects/issuable_templates_spec.rb index d2789d0aa52..1f9b52dd998 100644 --- a/spec/features/projects/issuable_templates_spec.rb +++ b/spec/features/projects/issuable_templates_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' feature 'issuable templates', js: true do let(:user) { create(:user) } let(:project) { create(:project, :public, :repository) } + let(:issue_form_location) { '#content-body .issuable-details .detail-page-description' } before do project.team << [user, :master] @@ -28,14 +29,17 @@ feature 'issuable templates', js: true do longtemplate_content, message: 'added issue template', branch_name: 'master') - visit edit_project_issue_path project, issue - fill_in :'issue[title]', with: 'test issue title' + visit project_issue_path project, issue + page.within('.content .issuable-actions') do + click_on 'Edit' + end + fill_in :'issue-title', with: 'test issue title' end scenario 'user selects "bug" template' do select_template 'bug' wait_for_requests - assert_template + assert_template(page_part: issue_form_location) save_changes end @@ -43,30 +47,19 @@ feature 'issuable templates', js: true do select_template 'bug' wait_for_requests select_option 'No template' - assert_template('') + assert_template(expected_content: '', page_part: issue_form_location) save_changes('') end scenario 'user selects "bug" template, edits description and then selects "reset template"' do select_template 'bug' wait_for_requests - find_field('issue_description').send_keys(description_addition) - assert_template(template_content + description_addition) + find_field('issue-description').send_keys(description_addition) + assert_template(expected_content: template_content + description_addition, page_part: issue_form_location) select_option 'Reset template' - assert_template + assert_template(page_part: issue_form_location) save_changes end - - it 'updates height of markdown textarea' do - start_height = page.evaluate_script('$(".markdown-area").outerHeight()') - - select_template 'test' - wait_for_requests - - end_height = page.evaluate_script('$(".markdown-area").outerHeight()') - - expect(end_height).not_to eq(start_height) - end end context 'user creates an issue using templates, with a prior description' do @@ -81,15 +74,18 @@ feature 'issuable templates', js: true do template_content, message: 'added issue template', branch_name: 'master') - visit edit_project_issue_path project, issue - fill_in :'issue[title]', with: 'test issue title' - fill_in :'issue[description]', with: prior_description + visit project_issue_path project, issue + page.within('.content .issuable-actions') do + click_on 'Edit' + end + fill_in :'issue-title', with: 'test issue title' + fill_in :'issue-description', with: prior_description end scenario 'user selects "bug" template' do select_template 'bug' wait_for_requests - assert_template("#{template_content}") + assert_template(page_part: issue_form_location) save_changes end end @@ -154,8 +150,10 @@ feature 'issuable templates', js: true do end end - def assert_template(expected_content = template_content) - expect(find('textarea')['value']).to eq(expected_content) + def assert_template(expected_content: template_content, page_part: '#content-body') + page.within(page_part) do + expect(find('textarea')['value']).to eq(expected_content) + end end def save_changes(expected_content = template_content) diff --git a/spec/features/projects/project_settings_spec.rb b/spec/features/projects/project_settings_spec.rb index 5d77cd1ccd5..06568817757 100644 --- a/spec/features/projects/project_settings_spec.rb +++ b/spec/features/projects/project_settings_spec.rb @@ -32,6 +32,32 @@ describe 'Edit Project Settings' do end end + describe 'Merge request settings section' do + it 'shows "Merge commit" strategy' do + visit edit_project_path(project) + + page.within '.merge-requests-feature' do + expect(page).to have_content 'Merge commit' + end + end + + it 'shows "Merge commit with semi-linear history " strategy' do + visit edit_project_path(project) + + page.within '.merge-requests-feature' do + expect(page).to have_content 'Merge commit with semi-linear history' + end + end + + it 'shows "Fast-forward merge" strategy' do + visit edit_project_path(project) + + page.within '.merge-requests-feature' do + expect(page).to have_content 'Fast-forward merge' + end + end + end + describe 'Rename repository section' do context 'with invalid characters' do it 'shows errors for invalid project path/name' do diff --git a/spec/features/projects/wiki/user_views_wiki_page_spec.rb b/spec/features/projects/wiki/user_views_wiki_page_spec.rb index 49ba2969ef0..470391dc66b 100644 --- a/spec/features/projects/wiki/user_views_wiki_page_spec.rb +++ b/spec/features/projects/wiki/user_views_wiki_page_spec.rb @@ -83,7 +83,7 @@ describe 'User views a wiki page' do it 'shows a file stored in a page' do file = Gollum::File.new(project.wiki) - allow_any_instance_of(Gollum::Wiki).to receive(:file).with('image.jpg', 'master', true).and_return(file) + allow_any_instance_of(Gollum::Wiki).to receive(:file).with('image.jpg', 'master').and_return(file) allow_any_instance_of(Gollum::File).to receive(:mime_type).and_return('image/jpeg') expect(page).to have_xpath('//img[@data-src="image.jpg"]') diff --git a/spec/features/security/project/internal_access_spec.rb b/spec/features/security/project/internal_access_spec.rb index a7928857b7d..d70cf1527e7 100644 --- a/spec/features/security/project/internal_access_spec.rb +++ b/spec/features/security/project/internal_access_spec.rb @@ -181,21 +181,6 @@ describe "Internal Project Access" do it { is_expected.to be_denied_for(:visitor) } end - describe "GET /:project_path/issues/:id/edit" do - let(:issue) { create(:issue, project: project) } - subject { edit_project_issue_path(project, issue) } - - it { is_expected.to be_allowed_for(:admin) } - it { is_expected.to be_allowed_for(:owner).of(project) } - it { is_expected.to be_allowed_for(:master).of(project) } - it { is_expected.to be_allowed_for(:developer).of(project) } - it { is_expected.to be_allowed_for(:reporter).of(project) } - it { is_expected.to be_denied_for(:guest).of(project) } - it { is_expected.to be_denied_for(:user) } - it { is_expected.to be_denied_for(:external) } - it { is_expected.to be_denied_for(:visitor) } - end - describe "GET /:project_path/snippets" do subject { project_snippets_path(project) } diff --git a/spec/features/security/project/private_access_spec.rb b/spec/features/security/project/private_access_spec.rb index a4396b20afd..ea130606545 100644 --- a/spec/features/security/project/private_access_spec.rb +++ b/spec/features/security/project/private_access_spec.rb @@ -181,21 +181,6 @@ describe "Private Project Access" do it { is_expected.to be_denied_for(:visitor) } end - describe "GET /:project_path/issues/:id/edit" do - let(:issue) { create(:issue, project: project) } - subject { edit_project_issue_path(project, issue) } - - it { is_expected.to be_allowed_for(:admin) } - it { is_expected.to be_allowed_for(:owner).of(project) } - it { is_expected.to be_allowed_for(:master).of(project) } - it { is_expected.to be_allowed_for(:developer).of(project) } - it { is_expected.to be_allowed_for(:reporter).of(project) } - it { is_expected.to be_denied_for(:guest).of(project) } - it { is_expected.to be_denied_for(:user) } - it { is_expected.to be_denied_for(:external) } - it { is_expected.to be_denied_for(:visitor) } - end - describe "GET /:project_path/snippets" do subject { project_snippets_path(project) } diff --git a/spec/features/security/project/public_access_spec.rb b/spec/features/security/project/public_access_spec.rb index fccdeb0e5b7..d15f5af66c9 100644 --- a/spec/features/security/project/public_access_spec.rb +++ b/spec/features/security/project/public_access_spec.rb @@ -394,21 +394,6 @@ describe "Public Project Access" do it { is_expected.to be_allowed_for(:visitor) } end - describe "GET /:project_path/issues/:id/edit" do - let(:issue) { create(:issue, project: project) } - subject { edit_project_issue_path(project, issue) } - - it { is_expected.to be_allowed_for(:admin) } - it { is_expected.to be_allowed_for(:owner).of(project) } - it { is_expected.to be_allowed_for(:master).of(project) } - it { is_expected.to be_allowed_for(:developer).of(project) } - it { is_expected.to be_allowed_for(:reporter).of(project) } - it { is_expected.to be_denied_for(:guest).of(project) } - it { is_expected.to be_denied_for(:user) } - it { is_expected.to be_denied_for(:external) } - it { is_expected.to be_denied_for(:visitor) } - end - describe "GET /:project_path/snippets" do subject { project_snippets_path(project) } diff --git a/spec/fixtures/api/schemas/entities/merge_request.json b/spec/fixtures/api/schemas/entities/merge_request.json index 1030f323a1f..0796d9b8af9 100644 --- a/spec/fixtures/api/schemas/entities/merge_request.json +++ b/spec/fixtures/api/schemas/entities/merge_request.json @@ -96,7 +96,9 @@ "remove_wip_path": { "type": "string" }, "commits_count": { "type": "integer" }, "remove_source_branch": { "type": ["boolean", "null"] }, - "merge_ongoing": { "type": "boolean" } + "merge_ongoing": { "type": "boolean" }, + "ff_only_enabled": { "type": ["boolean", false] }, + "should_be_rebased": { "type": "boolean" } }, "additionalProperties": false } diff --git a/spec/fixtures/config/kubeconfig.yml b/spec/fixtures/config/kubeconfig.yml index c4e8e573c32..5152dae0104 100644 --- a/spec/fixtures/config/kubeconfig.yml +++ b/spec/fixtures/config/kubeconfig.yml @@ -4,7 +4,7 @@ clusters: - name: gitlab-deploy cluster: server: https://kube.domain.com - certificate-authority-data: "UEVN\n" + certificate-authority-data: "UEVN" contexts: - name: gitlab-deploy context: diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb index 36031ac1a28..76e5964ccf7 100644 --- a/spec/helpers/groups_helper_spec.rb +++ b/spec/helpers/groups_helper_spec.rb @@ -17,7 +17,7 @@ describe GroupsHelper do it 'gives default avatar_icon when no avatar is present' do group = create(:group) group.save! - expect(group_icon(group.path)).to match('group_avatar.png') + expect(group_icon(group.path)).to match_asset_path('group_avatar.png') end end diff --git a/spec/helpers/page_layout_helper_spec.rb b/spec/helpers/page_layout_helper_spec.rb index 9aca3987657..baf927a9acc 100644 --- a/spec/helpers/page_layout_helper_spec.rb +++ b/spec/helpers/page_layout_helper_spec.rb @@ -54,7 +54,7 @@ describe PageLayoutHelper do describe 'page_image' do it 'defaults to the GitLab logo' do - expect(helper.page_image).to end_with 'assets/gitlab_logo.png' + expect(helper.page_image).to match_asset_path 'assets/gitlab_logo.png' end %w(project user group).each do |type| @@ -70,13 +70,13 @@ describe PageLayoutHelper do object = double(avatar_url: nil) assign(type, object) - expect(helper.page_image).to end_with 'assets/gitlab_logo.png' + expect(helper.page_image).to match_asset_path 'assets/gitlab_logo.png' end end context "with no assignments" do it 'falls back to the default' do - expect(helper.page_image).to end_with 'assets/gitlab_logo.png' + expect(helper.page_image).to match_asset_path 'assets/gitlab_logo.png' end end end diff --git a/spec/javascripts/locale/sprintf_spec.js b/spec/javascripts/locale/sprintf_spec.js new file mode 100644 index 00000000000..52e903b819f --- /dev/null +++ b/spec/javascripts/locale/sprintf_spec.js @@ -0,0 +1,74 @@ +import sprintf from '~/locale/sprintf'; + +describe('locale', () => { + describe('sprintf', () => { + it('does not modify string without parameters', () => { + const input = 'No parameters'; + + const output = sprintf(input); + + expect(output).toBe(input); + }); + + it('ignores extraneous parameters', () => { + const input = 'No parameters'; + + const output = sprintf(input, { ignore: 'this' }); + + expect(output).toBe(input); + }); + + it('ignores extraneous placeholders', () => { + const input = 'No %{parameters}'; + + const output = sprintf(input); + + expect(output).toBe(input); + }); + + it('replaces parameters', () => { + const input = '%{name} has %{count} parameters'; + const parameters = { + name: 'this', + count: 2, + }; + + const output = sprintf(input, parameters); + + expect(output).toBe('this has 2 parameters'); + }); + + it('replaces multiple occurrences', () => { + const input = 'to %{verb} or not to %{verb}'; + const parameters = { + verb: 'be', + }; + + const output = sprintf(input, parameters); + + expect(output).toBe('to be or not to be'); + }); + + it('escapes parameters', () => { + const input = 'contains %{userContent}'; + const parameters = { + userContent: '<script>alert("malicious!")</script>', + }; + + const output = sprintf(input, parameters); + + expect(output).toBe('contains <script>alert("malicious!")</script>'); + }); + + it('does not escape parameters for escapeParameters = false', () => { + const input = 'contains %{safeContent}'; + const parameters = { + safeContent: '<strong>bold attempt</strong>', + }; + + const output = sprintf(input, parameters, false); + + expect(output).toBe('contains <strong>bold attempt</strong>'); + }); + }); +}); diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_closed_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_closed_spec.js index 47303d1e80f..d23b558f4ea 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_closed_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_closed_spec.js @@ -4,11 +4,15 @@ import closedComponent from '~/vue_merge_request_widget/components/states/mr_wid const mr = { targetBranch: 'good-branch', targetBranchPath: '/good-branch', - closedBy: { - name: 'Fatih Acet', - username: 'fatihacet', + closedEvent: { + author: { + name: 'Fatih Acet', + username: 'fatihacet', + }, + updatedAt: 'closedEventUpdatedAt', + formattedUpdatedAt: '', }, - updatedAt: '2017-03-23T20:08:08.845Z', + updatedAt: 'mrUpdatedAt', closedAt: '1 day ago', }; @@ -18,7 +22,7 @@ const createComponent = () => { return new Component({ el: document.createElement('div'), propsData: { mr }, - }).$el; + }); }; describe('MRWidgetClosed', () => { @@ -38,14 +42,30 @@ describe('MRWidgetClosed', () => { }); describe('template', () => { - it('should have correct elements', () => { - const el = createComponent(); + let vm; + let el; + beforeEach(() => { + vm = createComponent(); + el = vm.$el; + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('should have correct elements', () => { expect(el.querySelector('h4').textContent).toContain('Closed by'); - expect(el.querySelector('h4').textContent).toContain(mr.closedBy.name); + expect(el.querySelector('h4').textContent).toContain(mr.closedEvent.author.name); expect(el.textContent).toContain('The changes were not merged into'); expect(el.querySelector('.label-branch').getAttribute('href')).toEqual(mr.targetBranchPath); expect(el.querySelector('.label-branch').textContent).toContain(mr.targetBranch); }); + + it('should use closedEvent updatedAt as tooltip title', () => { + expect( + el.querySelector('time').getAttribute('title'), + ).toBe('closedEventUpdatedAt'); + }); }); }); diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_conflicts_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_conflicts_spec.js index 3b7b7d93662..5d4c7ec09dc 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_conflicts_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_conflicts_spec.js @@ -1,20 +1,9 @@ import Vue from 'vue'; import conflictsComponent from '~/vue_merge_request_widget/components/states/mr_widget_conflicts'; +import mountComponent from '../../../helpers/vue_mount_component_helper'; +const ConflictsComponent = Vue.extend(conflictsComponent); const path = '/conflicts'; -const createComponent = () => { - const Component = Vue.extend(conflictsComponent); - - return new Component({ - el: document.createElement('div'), - propsData: { - mr: { - canMerge: true, - conflictResolutionPath: path, - }, - }, - }); -}; describe('MRWidgetConflicts', () => { describe('props', () => { @@ -27,44 +16,90 @@ describe('MRWidgetConflicts', () => { }); describe('template', () => { - it('should have correct elements', () => { - const el = createComponent().$el; - const resolveButton = el.querySelector('.js-resolve-conflicts-button'); - const mergeButton = el.querySelector('.mr-widget-body .btn'); - const mergeLocallyButton = el.querySelector('.js-merge-locally-button'); - - expect(el.textContent).toContain('There are merge conflicts'); - expect(el.textContent).not.toContain('ask someone with write access'); - expect(el.querySelector('.btn-success').disabled).toBeTruthy(); - expect(resolveButton.textContent).toContain('Resolve conflicts'); - expect(resolveButton.getAttribute('href')).toEqual(path); - expect(mergeButton.textContent).toContain('Merge'); - expect(mergeLocallyButton.textContent).toContain('Merge locally'); + describe('when allowed to merge', () => { + let vm; + + beforeEach(() => { + vm = mountComponent(ConflictsComponent, { + mr: { + canMerge: true, + conflictResolutionPath: path, + }, + }); + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('should tell you about conflicts without bothering other people', () => { + expect(vm.$el.textContent).toContain('There are merge conflicts'); + expect(vm.$el.textContent).not.toContain('ask someone with write access'); + }); + + it('should allow you to resolve the conflicts', () => { + const resolveButton = vm.$el.querySelector('.js-resolve-conflicts-button'); + + expect(resolveButton.textContent).toContain('Resolve conflicts'); + expect(resolveButton.getAttribute('href')).toEqual(path); + }); + + it('should have merge buttons', () => { + const mergeButton = vm.$el.querySelector('.js-disabled-merge-button'); + const mergeLocallyButton = vm.$el.querySelector('.js-merge-locally-button'); + + expect(mergeButton.textContent).toContain('Merge'); + expect(mergeButton.disabled).toBeTruthy(); + expect(mergeButton.classList.contains('btn-success')).toEqual(true); + expect(mergeLocallyButton.textContent).toContain('Merge locally'); + }); }); describe('when user does not have permission to merge', () => { let vm; beforeEach(() => { - vm = createComponent(); - vm.mr.canMerge = false; + vm = mountComponent(ConflictsComponent, { + mr: { + canMerge: false, + }, + }); }); - it('should show proper message', (done) => { - Vue.nextTick(() => { - expect(vm.$el.textContent).toContain('ask someone with write access'); - done(); - }); + afterEach(() => { + vm.$destroy(); + }); + + it('should show proper message', () => { + expect(vm.$el.textContent).toContain('ask someone with write access'); + }); + + it('should not have action buttons', () => { + expect(vm.$el.querySelector('.js-disabled-merge-button')).toBeDefined(); + expect(vm.$el.querySelector('.js-resolve-conflicts-button')).toBeNull(); + expect(vm.$el.querySelector('.js-merge-locally-button')).toBeNull(); }); + }); - it('should not have action buttons', (done) => { - Vue.nextTick(() => { - expect(vm.$el.querySelectorAll('.btn').length).toBe(1); - expect(vm.$el.querySelector('.js-resolve-conflicts-button')).toEqual(null); - expect(vm.$el.querySelector('.js-merge-locally-button')).toEqual(null); - done(); + describe('when fast-forward or semi-linear merge enabled', () => { + let vm; + + beforeEach(() => { + vm = mountComponent(ConflictsComponent, { + mr: { + shouldBeRebased: true, + }, }); }); + + afterEach(() => { + vm.$destroy(); + }); + + it('should tell you to rebase locally', () => { + expect(vm.$el.textContent).toContain('Fast-forward merge is not possible.'); + expect(vm.$el.textContent).toContain('To merge this request, first rebase locally'); + }); }); }); }); diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js index afaa750199a..2714e8294fa 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js @@ -14,9 +14,12 @@ const createComponent = () => { canRevertInCurrentMR: true, canRemoveSourceBranch: true, sourceBranchRemoved: true, - mergedBy: {}, - mergedAt: '', - updatedAt: '', + mergedEvent: { + author: {}, + updatedAt: 'mergedUpdatedAt', + formattedUpdatedAt: '', + }, + updatedAt: 'mrUpdatedAt', targetBranch, }; @@ -170,5 +173,11 @@ describe('MRWidgetMerged', () => { done(); }); }); + + it('should use mergedEvent updatedAt as tooltip title', () => { + expect( + el.querySelector('time').getAttribute('title'), + ).toBe('mergedUpdatedAt'); + }); }); }); diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js index 03a52f1f91c..2422e844e97 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js @@ -182,36 +182,6 @@ describe('MRWidgetReadyToMerge', () => { expect(vm.isMergeButtonDisabled).toBeTruthy(); }); }); - - describe('Remove source branch checkbox', () => { - describe('when user can merge but cannot delete branch', () => { - it('isRemoveSourceBranchButtonDisabled should be true', () => { - expect(vm.isRemoveSourceBranchButtonDisabled).toBe(true); - }); - - it('should be disabled in the rendered output', () => { - const checkboxElement = vm.$el.querySelector('#remove-source-branch-input'); - expect(checkboxElement.getAttribute('disabled')).toBe('disabled'); - }); - }); - - describe('when user can merge and can delete branch', () => { - beforeEach(() => { - this.customVm = createComponent({ - mr: { canRemoveSourceBranch: true }, - }); - }); - - it('isRemoveSourceBranchButtonDisabled should be false', () => { - expect(this.customVm.isRemoveSourceBranchButtonDisabled).toBe(false); - }); - - it('should be enabled in rendered output', () => { - const checkboxElement = this.customVm.$el.querySelector('#remove-source-branch-input'); - expect(checkboxElement.getAttribute('disabled')).toBeNull(); - }); - }); - }); }); describe('methods', () => { @@ -467,4 +437,54 @@ describe('MRWidgetReadyToMerge', () => { }); }); }); + + describe('Remove source branch checkbox', () => { + describe('when user can merge but cannot delete branch', () => { + it('isRemoveSourceBranchButtonDisabled should be true', () => { + expect(vm.isRemoveSourceBranchButtonDisabled).toBe(true); + }); + + it('should be disabled in the rendered output', () => { + const checkboxElement = vm.$el.querySelector('#remove-source-branch-input'); + expect(checkboxElement.getAttribute('disabled')).toBe('disabled'); + }); + }); + + describe('when user can merge and can delete branch', () => { + beforeEach(() => { + this.customVm = createComponent({ + mr: { canRemoveSourceBranch: true }, + }); + }); + + it('isRemoveSourceBranchButtonDisabled should be false', () => { + expect(this.customVm.isRemoveSourceBranchButtonDisabled).toBe(false); + }); + + it('should be enabled in rendered output', () => { + const checkboxElement = this.customVm.$el.querySelector('#remove-source-branch-input'); + expect(checkboxElement.getAttribute('disabled')).toBeNull(); + }); + }); + }); + + describe('Commit message area', () => { + it('when using merge commits, should show "Modify commit message" button', () => { + const customVm = createComponent({ + mr: { ffOnlyEnabled: false }, + }); + + expect(customVm.$el.querySelector('.js-fast-forward-message')).toBeNull(); + expect(customVm.$el.querySelector('.js-modify-commit-message-button')).toBeDefined(); + }); + + it('when fast-forward merge is enabled, only show fast-forward message', () => { + const customVm = createComponent({ + mr: { ffOnlyEnabled: true }, + }); + + expect(customVm.$el.querySelector('.js-fast-forward-message')).toBeDefined(); + expect(customVm.$el.querySelector('.js-modify-commit-message-button')).toBeNull(); + }); + }); }); diff --git a/spec/lib/gitlab/closing_issue_extractor_spec.rb b/spec/lib/gitlab/closing_issue_extractor_spec.rb index 9e528392756..ef7d766a13d 100644 --- a/spec/lib/gitlab/closing_issue_extractor_spec.rb +++ b/spec/lib/gitlab/closing_issue_extractor_spec.rb @@ -254,6 +254,46 @@ describe Gitlab::ClosingIssueExtractor do expect(subject.closed_by_message(message)).to eq([issue]) end + it do + message = "Implement: #{reference}" + expect(subject.closed_by_message(message)).to eq([issue]) + end + + it do + message = "Implements: #{reference}" + expect(subject.closed_by_message(message)).to eq([issue]) + end + + it do + message = "Implemented: #{reference}" + expect(subject.closed_by_message(message)).to eq([issue]) + end + + it do + message = "Implementing: #{reference}" + expect(subject.closed_by_message(message)).to eq([issue]) + end + + it do + message = "implement: #{reference}" + expect(subject.closed_by_message(message)).to eq([issue]) + end + + it do + message = "implements: #{reference}" + expect(subject.closed_by_message(message)).to eq([issue]) + end + + it do + message = "implemented: #{reference}" + expect(subject.closed_by_message(message)).to eq([issue]) + end + + it do + message = "implementing: #{reference}" + expect(subject.closed_by_message(message)).to eq([issue]) + end + context 'with an external issue tracker reference' do it 'extracts the referenced issue' do jira_project = create(:jira_project, name: 'JIRA_EXT1') diff --git a/spec/lib/gitlab/git/user_spec.rb b/spec/lib/gitlab/git/user_spec.rb index ab64b041187..31d5f59a562 100644 --- a/spec/lib/gitlab/git/user_spec.rb +++ b/spec/lib/gitlab/git/user_spec.rb @@ -8,6 +8,20 @@ describe Gitlab::Git::User do subject { described_class.new(username, name, email, gl_id) } + describe '.from_gitaly' do + let(:gitaly_user) { Gitaly::User.new(name: name, email: email, gl_id: gl_id) } + subject { described_class.from_gitaly(gitaly_user) } + + it { expect(subject).to eq(described_class.new('', name, email, gl_id)) } + end + + describe '.from_gitlab' do + let(:user) { build(:user) } + subject { described_class.from_gitlab(user) } + + it { expect(subject).to eq(described_class.new(user.username, user.name, user.email, 'user-')) } + end + describe '#==' do def eq_other(username, name, email, gl_id) eq(described_class.new(username, name, email, gl_id)) diff --git a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb index 1ef3e2e3a5d..b2275119a04 100644 --- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb @@ -53,7 +53,7 @@ describe Gitlab::GitalyClient::CommitService do end it 'encodes paths correctly' do - expect { client.diff_from_parent(commit, paths: ['encoding/test.txt', 'encoding/テスト.txt']) }.not_to raise_error + expect { client.diff_from_parent(commit, paths: ['encoding/test.txt', 'encoding/テスト.txt', nil]) }.not_to raise_error end end diff --git a/spec/lib/gitlab/gitaly_client_spec.rb b/spec/lib/gitlab/gitaly_client_spec.rb index 9a84d6e6a67..a1f4e65b8d4 100644 --- a/spec/lib/gitlab/gitaly_client_spec.rb +++ b/spec/lib/gitlab/gitaly_client_spec.rb @@ -38,6 +38,20 @@ describe Gitlab::GitalyClient, skip_gitaly_mock: true do end end + describe 'encode' do + [ + [nil, ""], + ["", ""], + [" ", " "], + %w(a1 a1), + ["编码", "\xE7\xBC\x96\xE7\xA0\x81".b] + ].each do |input, result| + it "encodes #{input.inspect} to #{result.inspect}" do + expect(described_class.encode(input)).to eq result + end + end + end + describe 'allow_n_plus_1_calls' do context 'when RequestStore is enabled', :request_store do it 'returns the result of the allow_n_plus_1_calls block' do diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 899d17d97c2..7268226112c 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -412,6 +412,8 @@ Project: - last_repository_updated_at - ci_config_path - delete_error +- merge_requests_ff_only_enabled +- merge_requests_rebase_enabled Author: - name ProjectFeature: diff --git a/spec/lib/gitlab/path_regex_spec.rb b/spec/lib/gitlab/path_regex_spec.rb index 2f989397f7e..1f1c48ee9b5 100644 --- a/spec/lib/gitlab/path_regex_spec.rb +++ b/spec/lib/gitlab/path_regex_spec.rb @@ -84,9 +84,9 @@ describe Gitlab::PathRegex do let(:top_level_words) do words = routes_not_starting_in_wildcard.map do |route| route.split('/')[1] - end.compact.uniq + end.compact - words + ee_top_level_words + files_in_public + Array(API::API.prefix.to_s) + (words + ee_top_level_words + files_in_public + Array(API::API.prefix.to_s)).uniq end let(:ee_top_level_words) do @@ -95,10 +95,11 @@ describe Gitlab::PathRegex do let(:files_in_public) do git = Gitlab.config.git.bin_path - `cd #{Rails.root} && #{git} ls-files public` + tracked = `cd #{Rails.root} && #{git} ls-files public` .split("\n") .map { |entry| entry.gsub('public/', '') } .uniq + tracked + %w(assets uploads) end # All routes that start with a namespaced path, that have 1 or more diff --git a/spec/lib/gitlab/popen_spec.rb b/spec/lib/gitlab/popen_spec.rb index 4567f220c11..b145ca36f26 100644 --- a/spec/lib/gitlab/popen_spec.rb +++ b/spec/lib/gitlab/popen_spec.rb @@ -14,7 +14,7 @@ describe 'Gitlab::Popen' do end it { expect(@status).to be_zero } - it { expect(@output).to include('cache') } + it { expect(@output).to include('tests') } end context 'non-zero status' do diff --git a/spec/lib/gitlab/url_sanitizer_spec.rb b/spec/lib/gitlab/url_sanitizer_spec.rb index 59c28431e1e..fc8991fd31f 100644 --- a/spec/lib/gitlab/url_sanitizer_spec.rb +++ b/spec/lib/gitlab/url_sanitizer_spec.rb @@ -39,7 +39,8 @@ describe Gitlab::UrlSanitizer do false | nil false | '' false | '123://invalid:url' - true | 'valid@project:url.git' + false | 'valid@project:url.git' + false | 'valid:pass@project:url.git' true | 'ssh://example.com' true | 'ssh://:@example.com' true | 'ssh://foo@example.com' @@ -81,24 +82,6 @@ describe Gitlab::UrlSanitizer do describe '#credentials' do context 'credentials in hash' do - where(:input, :output) do - { user: 'foo', password: 'bar' } | { user: 'foo', password: 'bar' } - { user: 'foo', password: '' } | { user: 'foo', password: nil } - { user: 'foo', password: nil } | { user: 'foo', password: nil } - { user: '', password: 'bar' } | { user: nil, password: 'bar' } - { user: '', password: '' } | { user: nil, password: nil } - { user: '', password: nil } | { user: nil, password: nil } - { user: nil, password: 'bar' } | { user: nil, password: 'bar' } - { user: nil, password: '' } | { user: nil, password: nil } - { user: nil, password: nil } | { user: nil, password: nil } - end - - with_them do - subject { described_class.new('user@example.com:path.git', credentials: input).credentials } - - it { is_expected.to eq(output) } - end - it 'overrides URL-provided credentials' do sanitizer = described_class.new('http://a:b@example.com', credentials: { user: 'c', password: 'd' }) @@ -116,10 +99,6 @@ describe Gitlab::UrlSanitizer do 'http://@example.com' | { user: nil, password: nil } 'http://example.com' | { user: nil, password: nil } - # Credentials from SCP-style URLs are not supported at present - 'foo@example.com:path' | { user: nil, password: nil } - 'foo:bar@example.com:path' | { user: nil, password: nil } - # Other invalid URLs nil | { user: nil, password: nil } '' | { user: nil, password: nil } diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb index 72496e9a212..4dffe2bd82f 100644 --- a/spec/lib/gitlab/workhorse_spec.rb +++ b/spec/lib/gitlab/workhorse_spec.rb @@ -13,13 +13,51 @@ describe Gitlab::Workhorse do end describe ".send_git_archive" do + let(:ref) { 'master' } + let(:format) { 'zip' } + let(:storage_path) { Gitlab.config.gitlab.repository_downloads_path } + let(:base_params) { repository.archive_metadata(ref, storage_path, format) } + let(:gitaly_params) do + base_params.merge( + 'GitalyServer' => { + 'address' => Gitlab::GitalyClient.address(project.repository_storage), + 'token' => Gitlab::GitalyClient.token(project.repository_storage) + }, + 'GitalyRepository' => repository.gitaly_repository.to_h.deep_stringify_keys + ) + end + + subject do + described_class.send_git_archive(repository, ref: ref, format: format) + end + + context 'when Gitaly workhorse_archive feature is enabled' do + it 'sets the header correctly' do + key, command, params = decode_workhorse_header(subject) + + expect(key).to eq('Gitlab-Workhorse-Send-Data') + expect(command).to eq('git-archive') + expect(params).to include(gitaly_params) + end + end + + context 'when Gitaly workhorse_archive feature is disabled', skip_gitaly_mock: true do + it 'sets the header correctly' do + key, command, params = decode_workhorse_header(subject) + + expect(key).to eq('Gitlab-Workhorse-Send-Data') + expect(command).to eq('git-archive') + expect(params).to eq(base_params) + end + end + context "when the repository doesn't have an archive file path" do before do allow(project.repository).to receive(:archive_metadata).and_return(Hash.new) end it "raises an error" do - expect { described_class.send_git_archive(project.repository, ref: "master", format: "zip") }.to raise_error(RuntimeError) + expect { subject }.to raise_error(RuntimeError) end end end diff --git a/spec/lib/system_check/app/git_user_default_ssh_config_check_spec.rb b/spec/lib/system_check/app/git_user_default_ssh_config_check_spec.rb index 7125bfcab59..a0fb86345f3 100644 --- a/spec/lib/system_check/app/git_user_default_ssh_config_check_spec.rb +++ b/spec/lib/system_check/app/git_user_default_ssh_config_check_spec.rb @@ -16,7 +16,12 @@ describe SystemCheck::App::GitUserDefaultSSHConfigCheck do end it 'only whitelists safe files' do - expect(described_class::WHITELIST).to contain_exactly('authorized_keys', 'authorized_keys2', 'known_hosts') + expect(described_class::WHITELIST).to contain_exactly( + 'authorized_keys', + 'authorized_keys2', + 'authorized_keys.lock', + 'known_hosts' + ) end describe '#skip?' do diff --git a/spec/models/project_services/kubernetes_service_spec.rb b/spec/models/project_services/kubernetes_service_spec.rb index 537cdadd528..2298dcab55f 100644 --- a/spec/models/project_services/kubernetes_service_spec.rb +++ b/spec/models/project_services/kubernetes_service_spec.rb @@ -208,7 +208,7 @@ describe KubernetesService, :use_clean_rails_memory_store_caching do config.dig('users', 0, 'user')['token'] = 'token' config.dig('contexts', 0, 'context')['namespace'] = namespace config.dig('clusters', 0, 'cluster')['certificate-authority-data'] = - Base64.encode64('CA PEM DATA') + Base64.strict_encode64('CA PEM DATA') YAML.dump(config) end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index c94481d53c3..176bb568cbe 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -408,6 +408,18 @@ describe Project do end end + describe '#merge_method' do + it 'returns "ff" merge_method when ff is enabled' do + project = build(:project, merge_requests_ff_only_enabled: true) + expect(project.merge_method).to be :ff + end + + it 'returns "merge" merge_method when ff is disabled' do + project = build(:project, merge_requests_ff_only_enabled: false) + expect(project.merge_method).to be :merge + end + end + describe '#repository_storage_path' do let(:project) { create(:project, repository_storage: 'custom') } diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index 953df7746eb..78fb2df884a 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -6,13 +6,10 @@ describe ProjectWiki do let(:user) { project.owner } let(:gitlab_shell) { Gitlab::Shell.new } let(:project_wiki) { described_class.new(project, user) } + let(:raw_repository) { Gitlab::Git::Repository.new(project.repository_storage, subject.disk_path + '.git', 'foo') } subject { project_wiki } - before do - project_wiki.wiki - end - describe "#path_with_namespace" do it "returns the project path with namespace with the .wiki extension" do expect(subject.path_with_namespace).to eq(project.full_path + '.wiki') @@ -61,8 +58,8 @@ describe ProjectWiki do end describe "#wiki" do - it "contains a Gollum::Wiki instance" do - expect(subject.wiki).to be_a Gollum::Wiki + it "contains a Gitlab::Git::Wiki instance" do + expect(subject.wiki).to be_a Gitlab::Git::Wiki end it "creates a new wiki repo if one does not yet exist" do @@ -70,20 +67,18 @@ describe ProjectWiki do end it "raises CouldNotCreateWikiError if it can't create the wiki repository" do - allow(project_wiki).to receive(:init_repo).and_return(false) - expect { project_wiki.send(:create_repo!) }.to raise_exception(ProjectWiki::CouldNotCreateWikiError) + # Create a fresh project which will not have a wiki + project_wiki = described_class.new(create(:project), user) + gitlab_shell = double(:gitlab_shell) + allow(gitlab_shell).to receive(:add_repository) + allow(project_wiki).to receive(:gitlab_shell).and_return(gitlab_shell) + + expect { project_wiki.send(:wiki) }.to raise_exception(ProjectWiki::CouldNotCreateWikiError) end end describe "#empty?" do context "when the wiki repository is empty" do - before do - allow_any_instance_of(Gitlab::Shell).to receive(:add_repository) do - create_temp_repo("#{Rails.root}/tmp/test-git-base-path/non-existant.wiki.git") - end - allow(project).to receive(:full_path).and_return("non-existant") - end - describe '#empty?' do subject { super().empty? } it { is_expected.to be_truthy } @@ -154,13 +149,13 @@ describe ProjectWiki do before do file = Gollum::File.new(subject.wiki) allow_any_instance_of(Gollum::Wiki) - .to receive(:file).with('image.jpg', 'master', true) + .to receive(:file).with('image.jpg', 'master') .and_return(file) allow_any_instance_of(Gollum::File) .to receive(:mime_type) .and_return('image/jpeg') allow_any_instance_of(Gollum::Wiki) - .to receive(:file).with('non-existant', 'master', true) + .to receive(:file).with('non-existant', 'master') .and_return(nil) end @@ -178,9 +173,9 @@ describe ProjectWiki do expect(subject.find_file('non-existant')).to eq(nil) end - it 'returns a Gollum::File instance' do + it 'returns a Gitlab::Git::WikiFile instance' do file = subject.find_file('image.jpg') - expect(file).to be_a Gollum::File + expect(file).to be_a Gitlab::Git::WikiFile end end @@ -222,9 +217,9 @@ describe ProjectWiki do describe "#update_page" do before do create_page("update-page", "some content") - @gollum_page = subject.wiki.paged("update-page") + @gitlab_git_wiki_page = subject.wiki.page(title: "update-page") subject.update_page( - @gollum_page, + @gitlab_git_wiki_page, content: "some other content", format: :markdown, message: "updated page" @@ -246,7 +241,7 @@ describe ProjectWiki do it 'updates project activity' do subject.update_page( - @gollum_page, + @gitlab_git_wiki_page, content: 'Yet more content', format: :markdown, message: 'Updated page again' @@ -262,7 +257,7 @@ describe ProjectWiki do describe "#delete_page" do before do create_page("index", "some content") - @page = subject.wiki.paged("index") + @page = subject.wiki.page(title: "index") end it "deletes the page" do @@ -282,27 +277,28 @@ describe ProjectWiki do describe '#create_repo!' do it 'creates a repository' do - expect(subject).to receive(:init_repo) - .with(subject.full_path) - .and_return(true) - + expect(raw_repository.exists?).to eq(false) expect(subject.repository).to receive(:after_create) - expect(subject.create_repo!).to be_an_instance_of(Gollum::Wiki) + subject.send(:create_repo!, raw_repository) + + expect(raw_repository.exists?).to eq(true) end end describe '#ensure_repository' do it 'creates the repository if it not exist' do - allow(subject).to receive(:repository_exists?).and_return(false) - - expect(subject).to receive(:create_repo!) + expect(raw_repository.exists?).to eq(false) + expect(subject).to receive(:create_repo!).and_call_original subject.ensure_repository + + expect(raw_repository.exists?).to eq(true) end it 'does not create the repository if it exists' do - allow(subject).to receive(:repository_exists?).and_return(true) + subject.wiki + expect(raw_repository.exists?).to eq(true) expect(subject).not_to receive(:create_repo!) @@ -329,7 +325,7 @@ describe ProjectWiki do end def commit_details - { name: user.name, email: user.email, message: "test commit" } + Gitlab::Git::Wiki::CommitDetails.new(user.name, user.email, "test commit") end def create_page(name, content) @@ -337,6 +333,6 @@ describe ProjectWiki do end def destroy_page(page) - subject.wiki.delete_page(page, commit_details) + subject.delete_page(page, commit_details) end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 157a10edd73..8a4dcdc311e 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1345,6 +1345,34 @@ describe Repository do end end + describe '#ff_merge' do + before do + repository.add_branch(user, 'ff-target', 'feature~5') + end + + it 'merges the code and return the commit id' do + merge_request = create(:merge_request, source_branch: 'feature', target_branch: 'ff-target', source_project: project) + merge_commit_id = repository.ff_merge(user, + merge_request.diff_head_sha, + merge_request.target_branch, + merge_request: merge_request) + merge_commit = repository.commit(merge_commit_id) + + expect(merge_commit).to be_present + expect(repository.blob_at(merge_commit.id, 'files/ruby/feature.rb')).to be_present + end + + it 'sets the `in_progress_merge_commit_sha` flag for the given merge request' do + merge_request = create(:merge_request, source_branch: 'feature', target_branch: 'ff-target', source_project: project) + merge_commit_id = repository.ff_merge(user, + merge_request.diff_head_sha, + merge_request.target_branch, + merge_request: merge_request) + + expect(merge_request.in_progress_merge_commit_sha).to eq(merge_commit_id) + end + end + describe '#revert' do let(:new_image_commit) { repository.commit('33f3729a45c02fc67d00adb1b8bca394b0e761d9') } let(:update_image_commit) { repository.commit('2f63565e7aac07bcdadb654e253078b727143ec4') } diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb index 9ef8d117123..1f14d06997e 100644 --- a/spec/models/wiki_page_spec.rb +++ b/spec/models/wiki_page_spec.rb @@ -80,7 +80,7 @@ describe WikiPage do context "when initialized with an existing gollum page" do before do create_page("test page", "test content") - @page = wiki.wiki.paged("test page") + @page = wiki.wiki.page(title: "test page") @wiki_page = described_class.new(wiki, @page, true) end @@ -105,7 +105,7 @@ describe WikiPage do end it "sets the version attribute" do - expect(@wiki_page.version).to be_a Gollum::Git::Commit + expect(@wiki_page.version).to be_a Gitlab::Git::WikiPageVersion end end end @@ -321,14 +321,14 @@ describe WikiPage do end it 'returns true when requesting an old version' do - old_version = @page.versions.last.to_s + old_version = @page.versions.last.id old_page = wiki.find_page('Update', old_version) expect(old_page.historical?).to eq true end it 'returns false when requesting latest version' do - latest_version = @page.versions.first.to_s + latest_version = @page.versions.first.id latest_page = wiki.find_page('Update', latest_version) expect(latest_page.historical?).to eq false @@ -393,7 +393,7 @@ describe WikiPage do end def commit_details - { name: user.name, email: user.email, message: "test commit" } + Gitlab::Git::Wiki::CommitDetails.new(user.name, user.email, "test commit") end def create_page(name, content) @@ -401,8 +401,8 @@ describe WikiPage do end def destroy_page(title) - page = wiki.wiki.paged(title) - wiki.wiki.delete_page(page, commit_details) + page = wiki.wiki.page(title: title) + wiki.delete_page(page, commit_details) end def get_slugs(page_or_dir) diff --git a/spec/serializers/build_serializer_spec.rb b/spec/serializers/build_serializer_spec.rb index 01e2cfed6f8..9673b11c2a2 100644 --- a/spec/serializers/build_serializer_spec.rb +++ b/spec/serializers/build_serializer_spec.rb @@ -38,7 +38,7 @@ describe BuildSerializer do expect(subject[:text]).to eq(status.text) expect(subject[:label]).to eq(status.label) expect(subject[:icon]).to eq(status.icon) - expect(subject[:favicon]).to eq("/assets/ci_favicons/#{status.favicon}.ico") + expect(subject[:favicon]).to match_asset_path("/assets/ci_favicons/#{status.favicon}.ico") end end end diff --git a/spec/serializers/merge_request_entity_spec.rb b/spec/serializers/merge_request_entity_spec.rb index a2fd5b7daae..4288955ddbc 100644 --- a/spec/serializers/merge_request_entity_spec.rb +++ b/spec/serializers/merge_request_entity_spec.rb @@ -47,7 +47,8 @@ describe MergeRequestEntity do :cancel_merge_when_pipeline_succeeds_path, :create_issue_to_resolve_discussions_path, :source_branch_path, :target_branch_commits_path, - :target_branch_tree_path, :commits_count, :merge_ongoing) + :target_branch_tree_path, :commits_count, :merge_ongoing, + :ff_only_enabled) end it 'has email_patches_path' do diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb index 3baf9b1edab..8fc1ceedc34 100644 --- a/spec/serializers/pipeline_serializer_spec.rb +++ b/spec/serializers/pipeline_serializer_spec.rb @@ -168,7 +168,7 @@ describe PipelineSerializer do expect(subject[:text]).to eq(status.text) expect(subject[:label]).to eq(status.label) expect(subject[:icon]).to eq(status.icon) - expect(subject[:favicon]).to eq("/assets/ci_favicons/#{status.favicon}.ico") + expect(subject[:favicon]).to match_asset_path("/assets/ci_favicons/#{status.favicon}.ico") end end end diff --git a/spec/serializers/status_entity_spec.rb b/spec/serializers/status_entity_spec.rb index 3964b998084..16431ed4188 100644 --- a/spec/serializers/status_entity_spec.rb +++ b/spec/serializers/status_entity_spec.rb @@ -18,12 +18,12 @@ describe StatusEntity do it 'contains status details' do expect(subject).to include :text, :icon, :favicon, :label, :group expect(subject).to include :has_details, :details_path - expect(subject[:favicon]).to eq('/assets/ci_favicons/favicon_status_success.ico') + expect(subject[:favicon]).to match_asset_path('/assets/ci_favicons/favicon_status_success.ico') end it 'contains a dev namespaced favicon if dev env' do allow(Rails.env).to receive(:development?) { true } - expect(entity.as_json[:favicon]).to eq('/assets/ci_favicons/dev/favicon_status_success.ico') + expect(entity.as_json[:favicon]).to match_asset_path('/assets/ci_favicons/dev/favicon_status_success.ico') end end end diff --git a/spec/services/merge_requests/ff_merge_service_spec.rb b/spec/services/merge_requests/ff_merge_service_spec.rb new file mode 100644 index 00000000000..aaabf3ed2b0 --- /dev/null +++ b/spec/services/merge_requests/ff_merge_service_spec.rb @@ -0,0 +1,84 @@ +require 'spec_helper' + +describe MergeRequests::FfMergeService do + let(:user) { create(:user) } + let(:user2) { create(:user) } + let(:merge_request) do + create(:merge_request, + source_branch: 'flatten-dir', + target_branch: 'improve/awesome', + assignee: user2) + end + let(:project) { merge_request.project } + + before do + project.team << [user, :master] + project.team << [user2, :developer] + end + + describe '#execute' do + context 'valid params' do + let(:service) { described_class.new(project, user, {}) } + + before do + allow(service).to receive(:execute_hooks) + + perform_enqueued_jobs do + service.execute(merge_request) + end + end + + it "does not create merge commit" do + source_branch_sha = merge_request.source_project.repository.commit(merge_request.source_branch).sha + target_branch_sha = merge_request.target_project.repository.commit(merge_request.target_branch).sha + expect(source_branch_sha).to eq(target_branch_sha) + end + + it { expect(merge_request).to be_valid } + it { expect(merge_request).to be_merged } + + it 'sends email to user2 about merge of new merge_request' do + email = ActionMailer::Base.deliveries.last + expect(email.to.first).to eq(user2.email) + expect(email.subject).to include(merge_request.title) + end + + it 'creates system note about merge_request merge' do + note = merge_request.notes.last + expect(note.note).to include 'merged' + end + end + + context "error handling" do + let(:service) { described_class.new(project, user, commit_message: 'Awesome message') } + + before do + allow(Rails.logger).to receive(:error) + end + + it 'logs and saves error if there is an exception' do + error_message = 'error message' + + allow(service).to receive(:repository).and_raise("error message") + allow(service).to receive(:execute_hooks) + + service.execute(merge_request) + + expect(merge_request.merge_error).to include(error_message) + expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message)) + end + + it 'logs and saves error if there is an PreReceiveError exception' do + error_message = 'error message' + + allow(service).to receive(:repository).and_raise(Gitlab::Git::HooksService::PreReceiveError, error_message) + allow(service).to receive(:execute_hooks) + + service.execute(merge_request) + + expect(merge_request.merge_error).to include(error_message) + expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message)) + end + end + end +end diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index 35f0c85b0ec..dc89fdebce7 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -76,9 +76,8 @@ describe Projects::CreateService, '#execute' do context 'wiki_enabled true creates wiki repository directory' do it do project = create_project(user, opts) - path = ProjectWiki.new(project, user).send(:path_to_repo) - expect(File.exist?(path)).to be_truthy + expect(wiki_repo(project).exists?).to be_truthy end end @@ -86,11 +85,15 @@ describe Projects::CreateService, '#execute' do it do opts[:wiki_enabled] = false project = create_project(user, opts) - path = ProjectWiki.new(project, user).send(:path_to_repo) - expect(File.exist?(path)).to be_falsey + expect(wiki_repo(project).exists?).to be_falsey end end + + def wiki_repo(project) + relative_path = ProjectWiki.new(project).disk_path + '.git' + Gitlab::Git::Repository.new(project.repository_storage, relative_path, 'foobar') + end end context 'builds_enabled global setting' do @@ -149,6 +152,9 @@ describe Projects::CreateService, '#execute' do end context 'when another repository already exists on disk' do + let(:repository_storage) { 'default' } + let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] } + let(:opts) do { name: 'Existing', @@ -156,31 +162,59 @@ describe Projects::CreateService, '#execute' do } end - let(:repository_storage) { 'default' } - let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] } + context 'with legacy storage' do + before do + gitlab_shell.add_repository(repository_storage, "#{user.namespace.full_path}/existing") + end - before do - gitlab_shell.add_repository(repository_storage, "#{user.namespace.full_path}/existing") - end + after do + gitlab_shell.remove_repository(repository_storage_path, "#{user.namespace.full_path}/existing") + end - after do - gitlab_shell.remove_repository(repository_storage_path, "#{user.namespace.full_path}/existing") - end + it 'does not allow to create a project when path matches existing repository on disk' do + project = create_project(user, opts) - it 'does not allow to create project with same path' do - project = create_project(user, opts) + expect(project).not_to be_persisted + expect(project).to respond_to(:errors) + expect(project.errors.messages).to have_key(:base) + expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk') + end + + it 'does not allow to import project when path matches existing repository on disk' do + project = create_project(user, opts.merge({ import_url: 'https://gitlab.com/gitlab-org/gitlab-test.git' })) - expect(project).to respond_to(:errors) - expect(project.errors.messages).to have_key(:base) - expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk') + expect(project).not_to be_persisted + expect(project).to respond_to(:errors) + expect(project.errors.messages).to have_key(:base) + expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk') + end end - it 'does not allow to import a project with the same path' do - project = create_project(user, opts.merge({ import_url: 'https://gitlab.com/gitlab-org/gitlab-test.git' })) + context 'with hashed storage' do + let(:hash) { '6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b' } + let(:hashed_path) { '@hashed/6b/86/6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b' } + + before do + stub_application_setting(hashed_storage_enabled: true) + allow(Digest::SHA2).to receive(:hexdigest) { hash } + end + + before do + gitlab_shell.add_repository(repository_storage, hashed_path) + end + + after do + gitlab_shell.remove_repository(repository_storage_path, hashed_path) + end + + it 'does not allow to create a project when path matches existing repository on disk' do + project = create_project(user, opts) - expect(project).to respond_to(:errors) - expect(project.errors.messages).to have_key(:base) - expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk') + expect(project).not_to be_persisted + expect(project).to respond_to(:errors) + expect(project.errors.messages).to have_key(:base) + expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk') + end end end end diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 4873e967535..d400304622e 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -152,22 +152,40 @@ describe Projects::UpdateService, '#execute' do let(:repository_storage) { 'default' } let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] } - before do - gitlab_shell.add_repository(repository_storage, "#{user.namespace.full_path}/existing") - end + context 'with legacy storage' do + before do + gitlab_shell.add_repository(repository_storage, "#{user.namespace.full_path}/existing") + end - after do - gitlab_shell.remove_repository(repository_storage_path, "#{user.namespace.full_path}/existing") + after do + gitlab_shell.remove_repository(repository_storage_path, "#{user.namespace.full_path}/existing") + end + + it 'does not allow renaming when new path matches existing repository on disk' do + result = update_project(project, admin, path: 'existing') + + expect(result).to include(status: :error) + expect(result[:message]).to match('There is already a repository with that name on disk') + expect(project).not_to be_valid + expect(project.errors.messages).to have_key(:base) + expect(project.errors.messages[:base]).to include('There is already a repository with that name on disk') + end end - it 'does not allow renaming when new path matches existing repository on disk' do - result = update_project(project, admin, path: 'existing') + context 'with hashed storage' do + let(:project) { create(:project, :repository, creator: user, namespace: user.namespace) } - expect(result).to include(status: :error) - expect(result[:message]).to match('There is already a repository with that name on disk') - expect(project).not_to be_valid - expect(project.errors.messages).to have_key(:base) - expect(project.errors.messages[:base]).to include('There is already a repository with that name on disk') + before do + stub_application_setting(hashed_storage_enabled: true) + end + + it 'does not check if new path matches existing repository on disk' do + expect(project).not_to receive(:repository_with_same_path_already_exists?) + + result = update_project(project, admin, path: 'existing') + + expect(result).to include(status: :success) + end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index dbf05b7f004..576e4ae1d38 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -169,6 +169,24 @@ RSpec.configure do |config| end end +# add simpler way to match asset paths containing digest strings +RSpec::Matchers.define :match_asset_path do |expected| + match do |actual| + path = Regexp.escape(expected) + extname = Regexp.escape(File.extname(expected)) + digest_regex = Regexp.new(path.sub(extname, "(?:-\\h+)?#{extname}") << '$') + digest_regex =~ actual + end + + failure_message do |actual| + "expected that #{actual} would include an asset path for #{expected}" + end + + failure_message_when_negated do |actual| + "expected that #{actual} would not include an asset path for #{expected}" + end +end + FactoryGirl::SyntaxRunner.class_eval do include RSpec::Mocks::ExampleMethods end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index b4e8b5ea67b..79395f4c564 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -17,6 +17,7 @@ module TestEnv 'feature_conflict' => 'bb5206f', 'fix' => '48f0be4', 'improve/awesome' => '5937ac0', + 'merged-target' => '21751bf', 'markdown' => '0ed8c6c', 'lfs' => 'be93687', 'master' => 'b83d6e3', diff --git a/spec/support/update_invalid_issuable.rb b/spec/support/update_invalid_issuable.rb index 1490287681b..50a1d4a56e2 100644 --- a/spec/support/update_invalid_issuable.rb +++ b/spec/support/update_invalid_issuable.rb @@ -25,11 +25,13 @@ shared_examples 'update invalid issuable' do |klass| .and_raise(ActiveRecord::StaleObjectError.new(issuable, :save)) end - it 'renders edit when format is html' do - put :update, params + if klass == MergeRequest + it 'renders edit when format is html' do + put :update, params - expect(response).to render_template(:edit) - expect(assigns[:conflict]).to be_truthy + expect(response).to render_template(:edit) + expect(assigns[:conflict]).to be_truthy + end end it 'renders json error message when format is json' do @@ -42,16 +44,17 @@ shared_examples 'update invalid issuable' do |klass| end end - context 'when updating an invalid issuable' do - before do - key = klass == Issue ? :issue : :merge_request - params[key][:title] = "" - end + if klass == MergeRequest + context 'when updating an invalid issuable' do + before do + params[:merge_request][:title] = "" + end - it 'renders edit when merge request is invalid' do - put :update, params + it 'renders edit when merge request is invalid' do + put :update, params - expect(response).to render_template(:edit) + expect(response).to render_template(:edit) + end end end end diff --git a/spec/workers/repository_check/single_repository_worker_spec.rb b/spec/workers/repository_check/single_repository_worker_spec.rb index d2609d21546..1d9bbf2ca62 100644 --- a/spec/workers/repository_check/single_repository_worker_spec.rb +++ b/spec/workers/repository_check/single_repository_worker_spec.rb @@ -69,7 +69,12 @@ describe RepositoryCheck::SingleRepositoryWorker do end def break_wiki(project) - FileUtils.rm_rf(wiki_path(project) + '/objects') + objects_dir = wiki_path(project) + '/objects' + + # Replace the /objects directory with a file so that the repo is + # invalid, _and_ 'git init' cannot fix it. + FileUtils.rm_rf(objects_dir) + FileUtils.touch(objects_dir) if File.directory?(wiki_path(project)) end def wiki_path(project) |