diff options
58 files changed, 520 insertions, 292 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 07969475503..d4b375696c2 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -256,7 +256,7 @@ flaky-examples-check: USE_BUNDLE_INSTALL: "false" NEW_FLAKY_SPECS_REPORT: rspec_flaky/report-new.json stage: post-test - allow_failure: yes + allow_failure: true retry: 0 only: - branches @@ -416,7 +416,6 @@ ee_compat_check: - /^[\d-]+-stable(-ee)?/ - branches@gitlab-org/gitlab-ee - branches@gitlab/gitlab-ee - allow_failure: no retry: 0 artifacts: name: "${CI_JOB_NAME}_${CI_COMIT_REF_NAME}_${CI_COMMIT_SHA}" @@ -263,6 +263,8 @@ gem 'gettext_i18n_rails', '~> 1.8.0' gem 'gettext_i18n_rails_js', '~> 1.2.0' gem 'gettext', '~> 3.2.2', require: false, group: :development +gem 'batch-loader' + # Perf bar gem 'peek', '~> 1.0.1' gem 'peek-gc', '~> 0.0.2' diff --git a/Gemfile.lock b/Gemfile.lock index 0669957e80f..4787be92365 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -73,6 +73,7 @@ GEM thread_safe (~> 0.3, >= 0.3.1) babosa (1.0.2) base32 (0.3.2) + batch-loader (1.1.1) bcrypt (3.1.11) bcrypt_pbkdf (1.0.0) benchmark-ips (2.3.0) @@ -355,10 +356,10 @@ GEM rake grape_logging (1.7.0) grape - grpc (1.6.6) + grpc (1.7.2) google-protobuf (~> 3.1) googleapis-common-protos-types (~> 1.0.0) - googleauth (~> 0.5.1) + googleauth (>= 0.5.1, < 0.7) haml (4.0.7) tilt haml_lint (0.26.0) @@ -982,6 +983,7 @@ DEPENDENCIES awesome_print (~> 1.2.0) babosa (~> 1.0.2) base32 (~> 0.3.0) + batch-loader bcrypt_pbkdf (~> 1.0) benchmark-ips (~> 2.3.0) better_errors (~> 2.1.0) diff --git a/app/assets/javascripts/blob_edit/blob_bundle.js b/app/assets/javascripts/blob_edit/blob_bundle.js index b5500ac116f..6b06344f5ba 100644 --- a/app/assets/javascripts/blob_edit/blob_bundle.js +++ b/app/assets/javascripts/blob_edit/blob_bundle.js @@ -1,7 +1,6 @@ /* eslint-disable func-names, space-before-function-paren, prefer-arrow-callback, no-var, quotes, vars-on-top, no-unused-vars, no-new, max-len */ /* global EditBlob */ -/* global NewCommitForm */ - +import NewCommitForm from '../new_commit_form'; import EditBlob from './edit_blob'; import BlobFileDropzone from '../blob/blob_file_dropzone'; diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index 344b31cf8b7..b4307761c6b 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -2,11 +2,11 @@ import { s__ } from './locale'; import projectSelect from './project_select'; import IssuableIndex from './issuable_index'; -/* global Milestone */ +import Milestone from './milestone'; import IssuableForm from './issuable_form'; import LabelsSelect from './labels_select'; /* global MilestoneSelect */ -/* global NewBranchForm */ +import NewBranchForm from './new_branch_form'; /* global NotificationsForm */ /* global NotificationsDropdown */ import groupAvatar from './group_avatar'; @@ -18,8 +18,7 @@ import groupsSelect from './groups_select'; /* global Search */ /* global Admin */ import NamespaceSelect from './namespace_select'; -/* global NewCommitForm */ -/* global NewBranchForm */ +import NewCommitForm from './new_commit_form'; import Project from './project'; import projectAvatar from './project_avatar'; /* global MergeRequest */ diff --git a/app/assets/javascripts/init_legacy_filters.js b/app/assets/javascripts/init_legacy_filters.js index 1b265721581..2cbb70220d0 100644 --- a/app/assets/javascripts/init_legacy_filters.js +++ b/app/assets/javascripts/init_legacy_filters.js @@ -1,8 +1,7 @@ /* eslint-disable no-new */ import LabelsSelect from './labels_select'; /* global MilestoneSelect */ -/* global SubscriptionSelect */ - +import subscriptionSelect from './subscription_select'; import UsersSelect from './users_select'; import issueStatusSelect from './issue_status_select'; @@ -11,5 +10,5 @@ export default () => { new LabelsSelect(); new MilestoneSelect(); issueStatusSelect(); - new SubscriptionSelect(); + subscriptionSelect(); }; diff --git a/app/assets/javascripts/issuable_bulk_update_sidebar.js b/app/assets/javascripts/issuable_bulk_update_sidebar.js index af6358953cf..ba2b6737988 100644 --- a/app/assets/javascripts/issuable_bulk_update_sidebar.js +++ b/app/assets/javascripts/issuable_bulk_update_sidebar.js @@ -1,11 +1,10 @@ /* eslint-disable class-methods-use-this, no-new */ /* global MilestoneSelect */ -/* global SubscriptionSelect */ import IssuableBulkUpdateActions from './issuable_bulk_update_actions'; import './milestone_select'; import issueStatusSelect from './issue_status_select'; -import './subscription_select'; +import subscriptionSelect from './subscription_select'; import LabelsSelect from './labels_select'; const HIDDEN_CLASS = 'hidden'; @@ -48,7 +47,7 @@ export default class IssuableBulkUpdateSidebar { new LabelsSelect(); new MilestoneSelect(); issueStatusSelect(); - new SubscriptionSelect(); + subscriptionSelect(); } setupBulkUpdateActions() { diff --git a/app/assets/javascripts/issue_show/components/app.vue b/app/assets/javascripts/issue_show/components/app.vue index e8ac8d3b5bb..4e39d483b31 100644 --- a/app/assets/javascripts/issue_show/components/app.vue +++ b/app/assets/javascripts/issue_show/components/app.vue @@ -102,6 +102,11 @@ export default { required: false, default: 'issue', }, + canAttachFile: { + type: Boolean, + required: false, + default: true, + }, }, data() { const store = new Store({ @@ -234,6 +239,7 @@ export default { :project-path="projectPath" :project-namespace="projectNamespace" :show-delete-button="showDeleteButton" + :can-attach-file="canAttachFile" /> <div v-else> <title-component diff --git a/app/assets/javascripts/issue_show/components/fields/description.vue b/app/assets/javascripts/issue_show/components/fields/description.vue index 0aa1b2c2e31..4d2ef409bad 100644 --- a/app/assets/javascripts/issue_show/components/fields/description.vue +++ b/app/assets/javascripts/issue_show/components/fields/description.vue @@ -17,6 +17,11 @@ type: String, required: true, }, + canAttachFile: { + type: Boolean, + required: false, + default: true, + }, }, components: { markdownField, @@ -36,7 +41,8 @@ </label> <markdown-field :markdown-preview-path="markdownPreviewPath" - :markdown-docs-path="markdownDocsPath"> + :markdown-docs-path="markdownDocsPath" + :can-attach-file="canAttachFile"> <textarea id="issue-description" class="note-textarea js-gfm-input js-autosize markdown-area" diff --git a/app/assets/javascripts/issue_show/components/form.vue b/app/assets/javascripts/issue_show/components/form.vue index 8bb5c86d567..d61776d480d 100644 --- a/app/assets/javascripts/issue_show/components/form.vue +++ b/app/assets/javascripts/issue_show/components/form.vue @@ -41,6 +41,11 @@ required: false, default: true, }, + canAttachFile: { + type: Boolean, + required: false, + default: true, + }, }, components: { lockedWarning, @@ -83,7 +88,8 @@ <description-field :form-state="formState" :markdown-preview-path="markdownPreviewPath" - :markdown-docs-path="markdownDocsPath" /> + :markdown-docs-path="markdownDocsPath" + :can-attach-file="canAttachFile" /> <edit-actions :form-state="formState" :can-destroy="canDestroy" diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index 601ab90bb30..b7ef1ecd923 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -58,11 +58,7 @@ import './line_highlighter'; import initLogoAnimation from './logo'; import './merge_request'; import './merge_request_tabs'; -import './milestone'; import './milestone_select'; -import './namespace_select'; -import './new_branch_form'; -import './new_commit_form'; import './notes'; import './notifications_dropdown'; import './notifications_form'; @@ -78,8 +74,6 @@ import './render_gfm'; import './right_sidebar'; import './search'; import './search_autocomplete'; -import './smart_interval'; -import './subscription_select'; import initBreadcrumbs from './breadcrumb'; import './dispatcher'; diff --git a/app/assets/javascripts/milestone.js b/app/assets/javascripts/milestone.js index 8f3f1986763..f76a998bf8c 100644 --- a/app/assets/javascripts/milestone.js +++ b/app/assets/javascripts/milestone.js @@ -1,54 +1,49 @@ -/* eslint-disable func-names, space-before-function-paren, wrap-iife, no-use-before-define, camelcase, quotes, object-shorthand, no-shadow, no-unused-vars, comma-dangle, no-var, prefer-template, no-underscore-dangle, consistent-return, one-var, one-var-declaration-per-line, default-case, prefer-arrow-callback, max-len */ /* global Sortable */ import Flash from './flash'; -(function() { - this.Milestone = (function() { - function Milestone() { - this.bindTabsSwitching(); +export default class Milestone { + constructor() { + this.bindTabsSwitching(); - // Load merge request tab if it is active - // merge request tab is active based on different conditions in the backend - this.loadTab($('.js-milestone-tabs .active a')); + // Load merge request tab if it is active + // merge request tab is active based on different conditions in the backend + this.loadTab($('.js-milestone-tabs .active a')); - this.loadInitialTab(); - } + this.loadInitialTab(); + } + + bindTabsSwitching() { + return $('a[data-toggle="tab"]').on('show.bs.tab', (e) => { + const $target = $(e.target); - Milestone.prototype.bindTabsSwitching = function() { - return $('a[data-toggle="tab"]').on('show.bs.tab', (e) => { - const $target = $(e.target); + location.hash = $target.attr('href'); + this.loadTab($target); + }); + } + // eslint-disable-next-line class-methods-use-this + loadInitialTab() { + const $target = $(`.js-milestone-tabs a[href="${location.hash}"]`); - location.hash = $target.attr('href'); - this.loadTab($target); + if ($target.length) { + $target.tab('show'); + } + } + // eslint-disable-next-line class-methods-use-this + loadTab($target) { + const endpoint = $target.data('endpoint'); + const tabElId = $target.attr('href'); + + if (endpoint && !$target.hasClass('is-loaded')) { + $.ajax({ + url: endpoint, + dataType: 'JSON', + }) + .fail(() => new Flash('Error loading milestone tab')) + .done((data) => { + $(tabElId).html(data.html); + $target.addClass('is-loaded'); }); - }; - - Milestone.prototype.loadInitialTab = function() { - const $target = $(`.js-milestone-tabs a[href="${location.hash}"]`); - - if ($target.length) { - $target.tab('show'); - } - }; - - Milestone.prototype.loadTab = function($target) { - const endpoint = $target.data('endpoint'); - const tabElId = $target.attr('href'); - - if (endpoint && !$target.hasClass('is-loaded')) { - $.ajax({ - url: endpoint, - dataType: 'JSON', - }) - .fail(() => new Flash('Error loading milestone tab')) - .done((data) => { - $(tabElId).html(data.html); - $target.addClass('is-loaded'); - }); - } - }; - - return Milestone; - })(); -}).call(window); + } + } +} diff --git a/app/assets/javascripts/new_branch_form.js b/app/assets/javascripts/new_branch_form.js index 39fb302b644..77733b67c4d 100644 --- a/app/assets/javascripts/new_branch_form.js +++ b/app/assets/javascripts/new_branch_form.js @@ -1,97 +1,93 @@ /* eslint-disable func-names, space-before-function-paren, no-var, one-var, prefer-rest-params, max-len, vars-on-top, wrap-iife, consistent-return, comma-dangle, one-var-declaration-per-line, quotes, no-return-assign, prefer-arrow-callback, prefer-template, no-shadow, no-else-return, max-len, object-shorthand */ -import RefSelectDropdown from '~/ref_select_dropdown'; +import RefSelectDropdown from './ref_select_dropdown'; -(function() { - this.NewBranchForm = (function() { - function NewBranchForm(form, availableRefs) { - this.validate = this.validate.bind(this); - this.branchNameError = form.find('.js-branch-name-error'); - this.name = form.find('.js-branch-name'); - this.ref = form.find('#ref'); - new RefSelectDropdown($('.js-branch-select'), availableRefs); // eslint-disable-line no-new - this.setupRestrictions(); - this.addBinding(); - this.init(); +export default class NewBranchForm { + constructor(form, availableRefs) { + this.validate = this.validate.bind(this); + this.branchNameError = form.find('.js-branch-name-error'); + this.name = form.find('.js-branch-name'); + this.ref = form.find('#ref'); + new RefSelectDropdown($('.js-branch-select'), availableRefs); // eslint-disable-line no-new + this.setupRestrictions(); + this.addBinding(); + this.init(); + } + + addBinding() { + return this.name.on('blur', this.validate); + } + + init() { + if (this.name.length && this.name.val().length > 0) { + return this.name.trigger('blur'); } + } - NewBranchForm.prototype.addBinding = function() { - return this.name.on('blur', this.validate); + setupRestrictions() { + var endsWith, invalid, single, startsWith; + startsWith = { + pattern: /^(\/|\.)/g, + prefix: "can't start with", + conjunction: "or" }; - - NewBranchForm.prototype.init = function() { - if (this.name.length && this.name.val().length > 0) { - return this.name.trigger('blur'); - } + endsWith = { + pattern: /(\/|\.|\.lock)$/g, + prefix: "can't end in", + conjunction: "or" }; - - NewBranchForm.prototype.setupRestrictions = function() { - var endsWith, invalid, single, startsWith; - startsWith = { - pattern: /^(\/|\.)/g, - prefix: "can't start with", - conjunction: "or" - }; - endsWith = { - pattern: /(\/|\.|\.lock)$/g, - prefix: "can't end in", - conjunction: "or" - }; - invalid = { - pattern: /(\s|~|\^|:|\?|\*|\[|\\|\.\.|@\{|\/{2,}){1}/g, - prefix: "can't contain", - conjunction: ", " - }; - single = { - pattern: /^@+$/g, - prefix: "can't be", - conjunction: "or" - }; - return this.restrictions = [startsWith, invalid, endsWith, single]; + invalid = { + pattern: /(\s|~|\^|:|\?|\*|\[|\\|\.\.|@\{|\/{2,}){1}/g, + prefix: "can't contain", + conjunction: ", " + }; + single = { + pattern: /^@+$/g, + prefix: "can't be", + conjunction: "or" }; + return this.restrictions = [startsWith, invalid, endsWith, single]; + } - NewBranchForm.prototype.validate = function() { - var errorMessage, errors, formatter, unique, validator; - const indexOf = [].indexOf; + validate() { + var errorMessage, errors, formatter, unique, validator; + const indexOf = [].indexOf; - this.branchNameError.empty(); - unique = function(values, value) { - if (indexOf.call(values, value) === -1) { - values.push(value); - } - return values; - }; - formatter = function(values, restriction) { - var formatted; - formatted = values.map(function(value) { - switch (false) { - case !/\s/.test(value): - return 'spaces'; - case !/\/{2,}/g.test(value): - return 'consecutive slashes'; - default: - return "'" + value + "'"; - } - }); - return restriction.prefix + " " + (formatted.join(restriction.conjunction)); - }; - validator = (function(_this) { - return function(errors, restriction) { - var matched; - matched = _this.name.val().match(restriction.pattern); - if (matched) { - return errors.concat(formatter(matched.reduce(unique, []), restriction)); - } else { - return errors; - } - }; - })(this); - errors = this.restrictions.reduce(validator, []); - if (errors.length > 0) { - errorMessage = $("<span/>").text(errors.join(', ')); - return this.branchNameError.append(errorMessage); + this.branchNameError.empty(); + unique = function(values, value) { + if (indexOf.call(values, value) === -1) { + values.push(value); } + return values; }; - - return NewBranchForm; - })(); -}).call(window); + formatter = function(values, restriction) { + var formatted; + formatted = values.map(function(value) { + switch (false) { + case !/\s/.test(value): + return 'spaces'; + case !/\/{2,}/g.test(value): + return 'consecutive slashes'; + default: + return "'" + value + "'"; + } + }); + return restriction.prefix + " " + (formatted.join(restriction.conjunction)); + }; + validator = (function(_this) { + return function(errors, restriction) { + var matched; + matched = _this.name.val().match(restriction.pattern); + if (matched) { + return errors.concat(formatter(matched.reduce(unique, []), restriction)); + } else { + return errors; + } + }; + })(this); + errors = this.restrictions.reduce(validator, []); + if (errors.length > 0) { + errorMessage = $("<span/>").text(errors.join(', ')); + return this.branchNameError.append(errorMessage); + } + } +} diff --git a/app/assets/javascripts/new_commit_form.js b/app/assets/javascripts/new_commit_form.js index 04073ef7270..6e152497d20 100644 --- a/app/assets/javascripts/new_commit_form.js +++ b/app/assets/javascripts/new_commit_form.js @@ -1,32 +1,28 @@ /* eslint-disable func-names, space-before-function-paren, no-var, prefer-rest-params, wrap-iife, no-return-assign, max-len */ -(function() { - this.NewCommitForm = (function() { - function NewCommitForm(form) { - this.form = form; - this.renderDestination = this.renderDestination.bind(this); - this.branchName = form.find('.js-branch-name'); - this.originalBranch = form.find('.js-original-branch'); - this.createMergeRequest = form.find('.js-create-merge-request'); - this.createMergeRequestContainer = form.find('.js-create-merge-request-container'); - this.branchName.keyup(this.renderDestination); - this.renderDestination(); - } +export default class NewCommitForm { + constructor(form) { + this.form = form; + this.renderDestination = this.renderDestination.bind(this); + this.branchName = form.find('.js-branch-name'); + this.originalBranch = form.find('.js-original-branch'); + this.createMergeRequest = form.find('.js-create-merge-request'); + this.createMergeRequestContainer = form.find('.js-create-merge-request-container'); + this.branchName.keyup(this.renderDestination); + this.renderDestination(); + } - NewCommitForm.prototype.renderDestination = function() { - var different; - different = this.branchName.val() !== this.originalBranch.val(); - if (different) { - this.createMergeRequestContainer.show(); - if (!this.wasDifferent) { - this.createMergeRequest.prop('checked', true); - } - } else { - this.createMergeRequestContainer.hide(); - this.createMergeRequest.prop('checked', false); + renderDestination() { + var different; + different = this.branchName.val() !== this.originalBranch.val(); + if (different) { + this.createMergeRequestContainer.show(); + if (!this.wasDifferent) { + this.createMergeRequest.prop('checked', true); } - return this.wasDifferent = different; - }; - - return NewCommitForm; - })(); -}).call(window); + } else { + this.createMergeRequestContainer.hide(); + this.createMergeRequest.prop('checked', false); + } + return this.wasDifferent = different; + } +} diff --git a/app/assets/javascripts/subscription_select.js b/app/assets/javascripts/subscription_select.js index 37e39ce5477..1ab4c2229ca 100644 --- a/app/assets/javascripts/subscription_select.js +++ b/app/assets/javascripts/subscription_select.js @@ -1,33 +1,24 @@ -/* eslint-disable func-names, space-before-function-paren, wrap-iife, no-var, quotes, object-shorthand, no-unused-vars, no-shadow, one-var, one-var-declaration-per-line, comma-dangle, max-len */ +export default function subscriptionSelect() { + $('.js-subscription-event').each((i, element) => { + const fieldName = $(element).data('field-name'); -class SubscriptionSelect { - constructor() { - $('.js-subscription-event').each(function(i, el) { - var fieldName; - fieldName = $(el).data("field-name"); - return $(el).glDropdown({ - selectable: true, - fieldName: fieldName, - toggleLabel: (function(_this) { - return function(selected, el, instance) { - var $item, label; - label = 'Subscription'; - $item = instance.dropdown.find('.is-active'); - if ($item.length) { - label = $item.text(); - } - return label; - }; - })(this), - clicked: function(options) { - return options.e.preventDefault(); - }, - id: function(obj, el) { - return $(el).data("id"); + return $(element).glDropdown({ + selectable: true, + fieldName, + toggleLabel(selected, el, instance) { + let label = 'Subscription'; + const $item = instance.dropdown.find('.is-active'); + if ($item.length) { + label = $item.text(); } - }); + return label; + }, + clicked(options) { + return options.e.preventDefault(); + }, + id(obj, el) { + return $(el).data('id'); + }, }); - } + }); } - -window.SubscriptionSelect = SubscriptionSelect; diff --git a/app/assets/javascripts/vue_shared/components/markdown/field.vue b/app/assets/javascripts/vue_shared/components/markdown/field.vue index a873e00d0f3..ee50ce27c3d 100644 --- a/app/assets/javascripts/vue_shared/components/markdown/field.vue +++ b/app/assets/javascripts/vue_shared/components/markdown/field.vue @@ -25,6 +25,11 @@ type: String, required: false, }, + canAttachFile: { + type: Boolean, + required: false, + default: true, + }, }, data() { return { @@ -129,6 +134,7 @@ <markdown-toolbar :markdown-docs-path="markdownDocsPath" :quick-actions-docs-path="quickActionsDocsPath" + :can-attach-file="canAttachFile" /> </div> </div> diff --git a/app/assets/javascripts/vue_shared/components/markdown/toolbar.vue b/app/assets/javascripts/vue_shared/components/markdown/toolbar.vue index 65fe7bbd94e..ea2509d2839 100644 --- a/app/assets/javascripts/vue_shared/components/markdown/toolbar.vue +++ b/app/assets/javascripts/vue_shared/components/markdown/toolbar.vue @@ -9,6 +9,11 @@ type: String, required: false, }, + canAttachFile: { + type: Boolean, + required: false, + default: true, + }, }, }; </script> @@ -41,7 +46,10 @@ are supported </template> </div> - <span class="uploading-container"> + <span + v-if="canAttachFile" + class="uploading-container" + > <span class="uploading-progress-container hide"> <i class="fa fa-file-image-o toolbar-button-icon" diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index 494d412b532..6ff96a3f295 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -22,12 +22,7 @@ class Projects::CommitController < Projects::ApplicationController apply_diff_view_cookie! respond_to do |format| - format.html do - # n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/37599 - Gitlab::GitalyClient.allow_n_plus_1_calls do - render - end - end + format.html { render } format.diff { render text: @commit.to_diff } format.patch { render text: @commit.to_patch } end @@ -112,7 +107,7 @@ class Projects::CommitController < Projects::ApplicationController end def commit - @noteable = @commit ||= @project.commit(params[:id]) + @noteable = @commit ||= @project.commit_by(oid: params[:id]) end def define_commit_vars diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb index 7d16e77ef66..d60a24d3f1d 100644 --- a/app/controllers/projects/merge_requests/diffs_controller.rb +++ b/app/controllers/projects/merge_requests/diffs_controller.rb @@ -10,10 +10,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic def show @environment = @merge_request.environments_for(current_user).last - # n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/37431 - Gitlab::GitalyClient.allow_n_plus_1_calls do - render json: { html: view_to_html_string("projects/merge_requests/diffs/_diffs") } - end + render json: { html: view_to_html_string("projects/merge_requests/diffs/_diffs") } end def diff_for_path diff --git a/app/models/blob.rb b/app/models/blob.rb index ad0bc2e2ead..29e762724e3 100644 --- a/app/models/blob.rb +++ b/app/models/blob.rb @@ -76,12 +76,24 @@ class Blob < SimpleDelegator new(blob, project) end + def self.lazy(project, commit_id, path) + BatchLoader.for(commit_id: commit_id, path: path).batch do |items, loader| + project.repository.blobs_at(items.map(&:values)).each do |blob| + loader.call({ commit_id: blob.commit_id, path: blob.path }, blob) if blob + end + end + end + def initialize(blob, project = nil) @project = project super(blob) end + def inspect + "#<#{self.class.name} oid:#{id[0..8]} commit:#{commit_id[0..8]} path:#{path}>" + end + # Returns the data of the blob. # # If the blob is a text based blob the content is converted to UTF-8 and any @@ -95,7 +107,10 @@ class Blob < SimpleDelegator end def load_all_data! - super(project.repository) if project + # Endpoint needed: gitlab-org/gitaly#756 + Gitlab::GitalyClient.allow_n_plus_1_calls do + super(project.repository) if project + end end def no_highlighting? diff --git a/app/models/commit.rb b/app/models/commit.rb index a31ebe9cc87..8401d99a08f 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -84,7 +84,7 @@ class Commit end def id - @raw.id + raw.id end def ==(other) @@ -361,7 +361,7 @@ class Commit @deltas ||= raw.deltas end - def diffs(diff_options = nil) + def diffs(diff_options = {}) Gitlab::Diff::FileCollection::Commit.new(self, diff_options: diff_options) end diff --git a/app/models/note.rb b/app/models/note.rb index f9676361072..50c9caf8529 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -110,6 +110,7 @@ class Note < ActiveRecord::Base includes(:author, :noteable, :updated_by, project: [:project_members, { group: [:group_members] }]) end + scope :with_metadata, -> { includes(:system_note_metadata) } after_initialize :ensure_discussion_id before_validation :nullify_blank_type, :nullify_blank_line_code @@ -169,7 +170,13 @@ class Note < ActiveRecord::Base end def cross_reference? - system? && matches_cross_reference_regex? + return unless system? + + if force_cross_reference_regex_check? + matches_cross_reference_regex? + else + SystemNoteService.cross_reference?(note) + end end def diff_note? @@ -382,4 +389,10 @@ class Note < ActiveRecord::Base def set_discussion_id self.discussion_id ||= discussion_class.discussion_id(self) end + + def force_cross_reference_regex_check? + return unless system? + + SystemNoteMetadata::TYPES_WITH_CROSS_REFERENCES.include?(system_note_metadata&.action) + end end diff --git a/app/models/repository.rb b/app/models/repository.rb index 8a6a8377de9..26d1bc12232 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -478,6 +478,11 @@ class Repository nil end + # items is an Array like: [[oid, path], [oid1, path1]] + def blobs_at(items) + raw_repository.batch_blobs(items).map { |blob| Blob.decorate(blob, project) } + end + def root_ref if raw_repository raw_repository.root_ref diff --git a/app/models/system_note_metadata.rb b/app/models/system_note_metadata.rb index 1f9f8d7286b..29035480371 100644 --- a/app/models/system_note_metadata.rb +++ b/app/models/system_note_metadata.rb @@ -1,4 +1,14 @@ class SystemNoteMetadata < ActiveRecord::Base + # These notes's action text might contain a reference that is external. + # We should always force a deep validation upon references that are found + # in this note type. + # Other notes can always be safely shown as all its references are + # in the same project (i.e. with the same permissions) + TYPES_WITH_CROSS_REFERENCES = %w[ + commit cross_reference + close duplicate + ].freeze + ICON_TYPES = %w[ commit description merge confidential visible label assignee cross_reference title time_tracking branch milestone discussion task moved diff --git a/app/models/user.rb b/app/models/user.rb index 0329d094d09..f98165754ca 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -445,6 +445,10 @@ class User < ActiveRecord::Base skip_confirmation! if bool end + def skip_reconfirmation=(bool) + skip_reconfirmation! if bool + end + def generate_reset_token @reset_token, enc = Devise.token_generator.generate(self.class, :reset_password_token) diff --git a/app/services/milestones/promote_service.rb b/app/services/milestones/promote_service.rb index bd9cfd4e0ea..2187f26d1ed 100644 --- a/app/services/milestones/promote_service.rb +++ b/app/services/milestones/promote_service.rb @@ -6,14 +6,14 @@ module Milestones check_project_milestone!(milestone) Milestone.transaction do - # Destroy all milestones with same title across projects - destroy_old_milestones(milestone) - group_milestone = clone_project_milestone(milestone) move_children_to_group_milestone(group_milestone) - # Just to be safe + # Destroy all milestones with same title across projects + destroy_old_milestones(milestone) + + # Rollback if milestone is not valid unless group_milestone.valid? raise_error(group_milestone.errors.full_messages.to_sentence) end @@ -35,7 +35,7 @@ module Milestones end def move_children_to_group_milestone(group_milestone) - milestone_ids_for_merge(group_milestone).in_groups_of(100) do |milestone_ids| + milestone_ids_for_merge(group_milestone).in_groups_of(100, false) do |milestone_ids| update_children(group_milestone, milestone_ids) end end @@ -49,7 +49,12 @@ module Milestones create_service = CreateService.new(group, current_user, params) - create_service.execute + milestone = create_service.execute + + # milestone won't be valid here because of duplicated title + milestone.save(validate: false) + + milestone end def update_children(group_milestone, milestone_ids) @@ -65,12 +70,12 @@ module Milestones @group ||= parent.group || raise_error('Project does not belong to a group.') end - def destroy_old_milestones(group_milestone) - Milestone.where(id: milestone_ids_for_merge(group_milestone)).destroy_all + def destroy_old_milestones(milestone) + Milestone.where(id: milestone_ids_for_merge(milestone)).destroy_all end def group_project_ids - @group_project_ids ||= group.projects.map(&:id) + @group_project_ids ||= group.projects.pluck(:id) end def raise_error(message) diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index e946218824c..fe71a405565 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -583,6 +583,10 @@ module SystemNoteService create_note(NoteSummary.new(issuable, issuable.project, author, body, action: action)) end + def cross_reference?(note_text) + note_text =~ /\A#{cross_reference_note_prefix}/i + end + private def notes_for_mentioner(mentioner, noteable, notes) diff --git a/app/views/projects/tags/index.html.haml b/app/views/projects/tags/index.html.haml index fd3b8c01b83..da364b58e36 100644 --- a/app/views/projects/tags/index.html.haml +++ b/app/views/projects/tags/index.html.haml @@ -1,6 +1,6 @@ - @no_container = true - @sort ||= sort_value_recently_updated -- page_title _('TagsPage|Tags') +- page_title s_('TagsPage|Tags') - add_to_breadcrumbs("Repository", project_tree_path(@project)) .flex-list{ class: container_class } diff --git a/app/views/shared/_ref_switcher.html.haml b/app/views/shared/_ref_switcher.html.haml index 6356e9f92cb..f4a4bfaec54 100644 --- a/app/views/shared/_ref_switcher.html.haml +++ b/app/views/shared/_ref_switcher.html.haml @@ -1,3 +1,5 @@ +- show_create = local_assigns.fetch(:show_create, false) + - show_new_branch_form = show_new_repo? && show_create && can?(current_user, :push_code, @project) - dropdown_toggle_text = @ref || @project.default_branch = form_tag switch_project_refs_path(@project), method: :get, class: "project-refs-form" do diff --git a/changelogs/unreleased/39461-notes-api-for-issues-no-longer-returns-label-additions-removals.yml b/changelogs/unreleased/39461-notes-api-for-issues-no-longer-returns-label-additions-removals.yml new file mode 100644 index 00000000000..36c2f789eeb --- /dev/null +++ b/changelogs/unreleased/39461-notes-api-for-issues-no-longer-returns-label-additions-removals.yml @@ -0,0 +1,5 @@ +--- +title: Label addition/removal are not going to be redacted wrongfully in the API. +merge_request: 15080 +author: +type: fixed diff --git a/changelogs/unreleased/39821-fix-commits-list-with-multi-file-editor.yml b/changelogs/unreleased/39821-fix-commits-list-with-multi-file-editor.yml new file mode 100644 index 00000000000..8b27c43d15b --- /dev/null +++ b/changelogs/unreleased/39821-fix-commits-list-with-multi-file-editor.yml @@ -0,0 +1,5 @@ +--- +title: Fix commits page throwing 500 when the multi-file editor was enabled +merge_request: 15502 +author: +type: fixed diff --git a/changelogs/unreleased/issue_40337.yml b/changelogs/unreleased/issue_40337.yml new file mode 100644 index 00000000000..0cd6c5f46a9 --- /dev/null +++ b/changelogs/unreleased/issue_40337.yml @@ -0,0 +1,5 @@ +--- +title: Fix promoting milestone updating all issuables without milestone +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/skip_confirmation_user_API.yml b/changelogs/unreleased/skip_confirmation_user_API.yml new file mode 100644 index 00000000000..144ccd69e68 --- /dev/null +++ b/changelogs/unreleased/skip_confirmation_user_API.yml @@ -0,0 +1,7 @@ +--- +title: Add email confirmation parameters for user creation and update via API +merge_request: +author: Daniel Juarez +type: added + + diff --git a/changelogs/unreleased/zj-commit-show-n-1.yml b/changelogs/unreleased/zj-commit-show-n-1.yml new file mode 100644 index 00000000000..e536434f74a --- /dev/null +++ b/changelogs/unreleased/zj-commit-show-n-1.yml @@ -0,0 +1,5 @@ +--- +title: Fetch blobs in bulk when generating diffs +merge_request: +author: +type: performance diff --git a/config/initializers/batch_loader.rb b/config/initializers/batch_loader.rb new file mode 100644 index 00000000000..2e2256b0eb9 --- /dev/null +++ b/config/initializers/batch_loader.rb @@ -0,0 +1 @@ +Rails.application.config.middleware.use(BatchLoader::Middleware) diff --git a/doc/api/users.md b/doc/api/users.md index aa711090af1..478d747a50d 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -297,6 +297,7 @@ Parameters: - `location` (optional) - User's location - `admin` (optional) - User is admin - true or false (default) - `can_create_group` (optional) - User can create groups - true or false +- `skip_reconfirmation` (optional) - Skip reconfirmation - true or false (default) - `external` (optional) - Flags the user as external - true or false(default) - `avatar` (optional) - Image file for user's avatar diff --git a/doc/integration/external-issue-tracker.md b/doc/integration/external-issue-tracker.md index 372e1909330..075feaeead9 100644 --- a/doc/integration/external-issue-tracker.md +++ b/doc/integration/external-issue-tracker.md @@ -22,6 +22,7 @@ Visit the links below for details: - [Redmine](../user/project/integrations/redmine.md) - [Jira](../user/project/integrations/jira.md) - [Bugzilla](../user/project/integrations/bugzilla.md) +- [Custom Issue Tracker](../user/project/integrations/custom_issue_tracker.md) ### Service Template diff --git a/doc/user/project/integrations/custom_issue_tracker.md b/doc/user/project/integrations/custom_issue_tracker.md new file mode 100644 index 00000000000..757522c2ae3 --- /dev/null +++ b/doc/user/project/integrations/custom_issue_tracker.md @@ -0,0 +1,20 @@ +# Custom Issue Tracker Service + +To enable the Custom Issue Tracker integration in a project, navigate to the +[Integrations page](project_services.md#accessing-the-project-services), click +the **Customer Issue Tracker** service, and fill in the required details on the page as described +in the table below. + +| Field | Description | +| ----- | ----------- | +| `title` | A title for the issue tracker (to differentiate between instances, for example) | +| `description` | A name for the issue tracker (to differentiate between instances, for example) | +| `project_url` | Currently unused. Will be changed in a future release. | +| `issues_url` | The URL to the issue in the issue tracker project that is linked to this GitLab project. Note that the `issues_url` requires `:id` in the URL. This ID is used by GitLab as a placeholder to replace the issue number. For example, `https://customissuetracker.com/project-name/:id`. | +| `new_issue_url` | Currently unused. Will be changed in a future release. | + + +## Referencing issues + +Issues are referenced with `#<ID>`, where `<ID>` is a number (example `#143`). +So with the example above, `#143` would refer to `https://customissuetracker.com/project-name/143`.
\ No newline at end of file diff --git a/lib/api/notes.rb b/lib/api/notes.rb index 0b9ab4eeb05..ceaaeca4046 100644 --- a/lib/api/notes.rb +++ b/lib/api/notes.rb @@ -33,7 +33,7 @@ module API # paginate() only works with a relation. This could lead to a # mismatch between the pagination headers info and the actual notes # array returned, but this is really a edge-case. - paginate(noteable.notes) + paginate(noteable.notes.with_metadata) .reject { |n| n.cross_reference_not_visible_for?(current_user) } present notes, with: Entities::Note else @@ -50,7 +50,7 @@ module API end get ":id/#{noteables_str}/:noteable_id/notes/:note_id" do noteable = find_project_noteable(noteables_str, params[:noteable_id]) - note = noteable.notes.find(params[:note_id]) + note = noteable.notes.with_metadata.find(params[:note_id]) can_read_note = can?(current_user, noteable_read_ability_name(noteable), noteable) && !note.cross_reference_not_visible_for?(current_user) if can_read_note diff --git a/lib/api/users.rb b/lib/api/users.rb index d80b364bd09..0cd89b1bcf8 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -31,7 +31,6 @@ module API optional :location, type: String, desc: 'The location of the user' optional :admin, type: Boolean, desc: 'Flag indicating the user is an administrator' optional :can_create_group, type: Boolean, desc: 'Flag indicating the user can create groups' - optional :skip_confirmation, type: Boolean, default: false, desc: 'Flag indicating the account is confirmed' optional :external, type: Boolean, desc: 'Flag indicating the user is an external user' optional :avatar, type: File, desc: 'Avatar image for user' all_or_none_of :extern_uid, :provider @@ -101,6 +100,7 @@ module API requires :email, type: String, desc: 'The email of the user' optional :password, type: String, desc: 'The password of the new user' optional :reset_password, type: Boolean, desc: 'Flag indicating the user will be sent a password reset token' + optional :skip_confirmation, type: Boolean, desc: 'Flag indicating the account is confirmed' at_least_one_of :password, :reset_password requires :name, type: String, desc: 'The name of the user' requires :username, type: String, desc: 'The username of the user' @@ -134,6 +134,7 @@ module API requires :id, type: Integer, desc: 'The ID of the user' optional :email, type: String, desc: 'The email of the user' optional :password, type: String, desc: 'The password of the new user' + optional :skip_reconfirmation, type: Boolean, desc: 'Flag indicating the account skips the confirmation by email' optional :name, type: String, desc: 'The name of the user' optional :username, type: String, desc: 'The username of the user' use :optional_attributes diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb index ea5891a028a..d0cfe2386ca 100644 --- a/lib/gitlab/diff/file.rb +++ b/lib/gitlab/diff/file.rb @@ -25,6 +25,10 @@ module Gitlab @repository = repository @diff_refs = diff_refs @fallback_diff_refs = fallback_diff_refs + + # Ensure items are collected in the the batch + new_blob + old_blob end def position(position_marker, position_type: :text) @@ -95,21 +99,15 @@ module Gitlab end def new_blob - return @new_blob if defined?(@new_blob) - - sha = new_content_sha - return @new_blob = nil unless sha + return unless new_content_sha - @new_blob = repository.blob_at(sha, file_path) + Blob.lazy(repository.project, new_content_sha, file_path) end def old_blob - return @old_blob if defined?(@old_blob) - - sha = old_content_sha - return @old_blob = nil unless sha + return unless old_content_sha - @old_blob = repository.blob_at(sha, old_path) + Blob.lazy(repository.project, old_content_sha, old_path) end def content_sha diff --git a/lib/gitlab/diff/file_collection/base.rb b/lib/gitlab/diff/file_collection/base.rb index 88ae65cb468..a6007ebf531 100644 --- a/lib/gitlab/diff/file_collection/base.rb +++ b/lib/gitlab/diff/file_collection/base.rb @@ -22,10 +22,7 @@ module Gitlab end def diff_files - # n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/37445 - Gitlab::GitalyClient.allow_n_plus_1_calls do - @diff_files ||= @diffs.decorate! { |diff| decorate_diff!(diff) } - end + @diff_files ||= @diffs.decorate! { |diff| decorate_diff!(diff) } end def diff_file_with_old_path(old_path) diff --git a/lib/gitlab/git/blob.rb b/lib/gitlab/git/blob.rb index bd5039fb87e..ddd52136bc4 100644 --- a/lib/gitlab/git/blob.rb +++ b/lib/gitlab/git/blob.rb @@ -179,6 +179,8 @@ module Gitlab ) end end + rescue Rugged::ReferenceError + nil end def rugged_raw(repository, sha, limit:) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 0c522deb6fa..3cb9b254e6e 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -1161,6 +1161,11 @@ module Gitlab Gitlab::Git::Blob.find(self, sha, path) unless Gitlab::Git.blank_ref?(sha) end + # Items should be of format [[commit_id, path], [commit_id1, path1]] + def batch_blobs(items, blob_size_limit: nil) + Gitlab::Git::Blob.batch(self, items, blob_size_limit: blob_size_limit) + end + def commit_index(user, branch_name, index, options) committer = user_to_committer(user) diff --git a/lib/gitlab/shell_adapter.rb b/lib/gitlab/shell_adapter.rb index fbe2a7a0d72..053dd4ab9e0 100644 --- a/lib/gitlab/shell_adapter.rb +++ b/lib/gitlab/shell_adapter.rb @@ -5,7 +5,7 @@ module Gitlab module ShellAdapter def gitlab_shell - Gitlab::Shell.new + @gitlab_shell ||= Gitlab::Shell.new end end end diff --git a/lib/tasks/gitlab/gitaly.rake b/lib/tasks/gitlab/gitaly.rake index 8377fe3269d..87835dbe719 100644 --- a/lib/tasks/gitlab/gitaly.rake +++ b/lib/tasks/gitlab/gitaly.rake @@ -17,9 +17,7 @@ namespace :gitlab do _, status = Gitlab::Popen.popen(%w[which gmake]) command = status.zero? ? ['gmake'] : ['make'] - if Rails.env.test? - command += %W[BUNDLE_PATH=#{Bundler.bundle_path}] - end + command << 'BUNDLE_FLAGS=--no-deployment' if Rails.env.test? Dir.chdir(args.dir) do create_gitaly_configuration diff --git a/spec/controllers/projects/commit_controller_spec.rb b/spec/controllers/projects/commit_controller_spec.rb index 5dc27e2bbba..fd90c0d8bad 100644 --- a/spec/controllers/projects/commit_controller_spec.rb +++ b/spec/controllers/projects/commit_controller_spec.rb @@ -1,15 +1,15 @@ require 'spec_helper' describe Projects::CommitController do - let(:project) { create(:project, :repository) } - let(:user) { create(:user) } + set(:project) { create(:project, :repository) } + set(:user) { create(:user) } let(:commit) { project.commit("master") } let(:master_pickable_sha) { '7d3b0f7cff5f37573aea97cebfd5692ea1689924' } let(:master_pickable_commit) { project.commit(master_pickable_sha) } before do sign_in(user) - project.team << [user, :master] + project.add_master(user) end describe 'GET show' do diff --git a/spec/features/commits_spec.rb b/spec/features/commits_spec.rb index b163ca8dc75..98586ddbd81 100644 --- a/spec/features/commits_spec.rb +++ b/spec/features/commits_spec.rb @@ -200,5 +200,12 @@ describe 'Commits' do expect(page).to have_content("committed #{commit.committed_date.strftime("%b %d, %Y")}") end end + + it 'shows the ref switcher with the multi-file editor enabled', :js do + set_cookie('new_repo', 'true') + visit project_commits_path(project, branch_name) + + expect(find('.js-project-refs-dropdown')).to have_content branch_name + end end end diff --git a/spec/javascripts/new_branch_spec.js b/spec/javascripts/new_branch_spec.js index c57f44dae17..50a5e4ff056 100644 --- a/spec/javascripts/new_branch_spec.js +++ b/spec/javascripts/new_branch_spec.js @@ -1,7 +1,6 @@ /* eslint-disable space-before-function-paren, one-var, no-var, one-var-declaration-per-line, no-return-assign, quotes, max-len */ -/* global NewBranchForm */ -import '~/new_branch_form'; +import NewBranchForm from '~/new_branch_form'; (function() { describe('Branch', function() { diff --git a/spec/javascripts/vue_shared/components/markdown/toolbar_spec.js b/spec/javascripts/vue_shared/components/markdown/toolbar_spec.js new file mode 100644 index 00000000000..818ef0af3c2 --- /dev/null +++ b/spec/javascripts/vue_shared/components/markdown/toolbar_spec.js @@ -0,0 +1,37 @@ +import Vue from 'vue'; +import toolbar from '~/vue_shared/components/markdown/toolbar.vue'; +import mountComponent from '../../../helpers/vue_mount_component_helper'; + +describe('toolbar', () => { + let vm; + const Toolbar = Vue.extend(toolbar); + const props = { + markdownDocsPath: '', + }; + + afterEach(() => { + vm.$destroy(); + }); + + describe('user can attach file', () => { + beforeEach(() => { + vm = mountComponent(Toolbar, props); + }); + + it('should render uploading-container', () => { + expect(vm.$el.querySelector('.uploading-container')).not.toBeNull(); + }); + }); + + describe('user cannot attach file', () => { + beforeEach(() => { + vm = mountComponent(Toolbar, Object.assign({}, props, { + canAttachFile: false, + })); + }); + + it('should not render uploading-container', () => { + expect(vm.$el.querySelector('.uploading-container')).toBeNull(); + }); + }); +}); diff --git a/spec/lib/gitlab/diff/file_spec.rb b/spec/lib/gitlab/diff/file_spec.rb index c91895cedc3..ff9acfd08b9 100644 --- a/spec/lib/gitlab/diff/file_spec.rb +++ b/spec/lib/gitlab/diff/file_spec.rb @@ -116,12 +116,8 @@ describe Gitlab::Diff::File do end context 'when renamed' do - let(:commit) { project.commit('6907208d755b60ebeacb2e9dfea74c92c3449a1f') } - let(:diff_file) { commit.diffs.diff_file_with_new_path('files/js/commit.coffee') } - - before do - allow(diff_file.new_blob).to receive(:id).and_return(diff_file.old_blob.id) - end + let(:commit) { project.commit('94bb47ca1297b7b3731ff2a36923640991e9236f') } + let(:diff_file) { commit.diffs.diff_file_with_new_path('CHANGELOG.md') } it 'returns false' do expect(diff_file.content_changed?).to be_falsey diff --git a/spec/models/blob_spec.rb b/spec/models/blob_spec.rb index 47342f98283..81e35e6c931 100644 --- a/spec/models/blob_spec.rb +++ b/spec/models/blob_spec.rb @@ -16,6 +16,23 @@ describe Blob do end end + describe '.lazy' do + let(:project) { create(:project, :repository) } + let(:commit) { project.commit_by(oid: 'e63f41fe459e62e1228fcef60d7189127aeba95a') } + + it 'fetches all blobs when the first is accessed' do + changelog = described_class.lazy(project, commit.id, 'CHANGELOG') + contributing = described_class.lazy(project, commit.id, 'CONTRIBUTING.md') + + expect(Gitlab::Git::Blob).to receive(:batch).once.and_call_original + expect(Gitlab::Git::Blob).not_to receive(:find) + + # Access property so the values are loaded + changelog.id + contributing.id + end + end + describe '#data' do context 'using a binary blob' do it 'returns the data as-is' do diff --git a/spec/models/diff_viewer/base_spec.rb b/spec/models/diff_viewer/base_spec.rb index b26de3f3b97..c90b32c5d77 100644 --- a/spec/models/diff_viewer/base_spec.rb +++ b/spec/models/diff_viewer/base_spec.rb @@ -32,10 +32,8 @@ describe DiffViewer::Base do end context 'when the binaryness does not match' do - before do - allow(diff_file.old_blob).to receive(:binary?).and_return(false) - allow(diff_file.new_blob).to receive(:binary?).and_return(false) - end + let(:commit) { project.commit_by(oid: 'ae73cb07c9eeaf35924a10f713b364d32b2dd34f') } + let(:diff_file) { commit.diffs.diff_file_with_new_path('Gemfile.zip') } it 'returns false' do expect(viewer_class.can_render?(diff_file)).to be_falsey @@ -60,8 +58,7 @@ describe DiffViewer::Base do context 'when the binaryness does not match' do before do - allow(diff_file.old_blob).to receive(:binary?).and_return(true) - allow(diff_file.new_blob).to receive(:binary?).and_return(true) + allow_any_instance_of(Blob).to receive(:binary?).and_return(true) end it 'returns false' do @@ -77,12 +74,12 @@ describe DiffViewer::Base do end context 'when the file was renamed and only the old blob is supported' do - let(:commit) { project.commit('2f63565e7aac07bcdadb654e253078b727143ec4') } + let(:commit) { project.commit_by(oid: '2f63565e7aac07bcdadb654e253078b727143ec4') } let(:diff_file) { commit.diffs.diff_file_with_new_path('files/images/6049019_460s.jpg') } before do allow(diff_file).to receive(:renamed_file?).and_return(true) - allow(diff_file.new_blob).to receive(:extension).and_return('jpeg') + viewer_class.extensions = %w(notjpg) end it 'returns false' do @@ -94,8 +91,7 @@ describe DiffViewer::Base do describe '#collapsed?' do context 'when the combined blob size is larger than the collapse limit' do before do - allow(diff_file.old_blob).to receive(:raw_size).and_return(512.kilobytes) - allow(diff_file.new_blob).to receive(:raw_size).and_return(513.kilobytes) + allow(diff_file).to receive(:raw_size).and_return(1025.kilobytes) end it 'returns true' do @@ -113,8 +109,7 @@ describe DiffViewer::Base do describe '#too_large?' do context 'when the combined blob size is larger than the size limit' do before do - allow(diff_file.old_blob).to receive(:raw_size).and_return(2.megabytes) - allow(diff_file.new_blob).to receive(:raw_size).and_return(4.megabytes) + allow(diff_file).to receive(:raw_size).and_return(6.megabytes) end it 'returns true' do @@ -132,8 +127,7 @@ describe DiffViewer::Base do describe '#render_error' do context 'when the combined blob size is larger than the size limit' do before do - allow(diff_file.old_blob).to receive(:raw_size).and_return(2.megabytes) - allow(diff_file.new_blob).to receive(:raw_size).and_return(4.megabytes) + allow(diff_file).to receive(:raw_size).and_return(6.megabytes) end it 'returns :too_large' do diff --git a/spec/models/diff_viewer/server_side_spec.rb b/spec/models/diff_viewer/server_side_spec.rb index 92e613f92de..98a8f6d4cc9 100644 --- a/spec/models/diff_viewer/server_side_spec.rb +++ b/spec/models/diff_viewer/server_side_spec.rb @@ -1,9 +1,9 @@ require 'spec_helper' describe DiffViewer::ServerSide do - let(:project) { create(:project, :repository) } - let(:commit) { project.commit('570e7b2abdd848b95f2f578043fc23bd6f6fd24d') } - let(:diff_file) { commit.diffs.diff_file_with_new_path('files/ruby/popen.rb') } + set(:project) { create(:project, :repository) } + let(:commit) { project.commit_by(oid: '570e7b2abdd848b95f2f578043fc23bd6f6fd24d') } + let!(:diff_file) { commit.diffs.diff_file_with_new_path('files/ruby/popen.rb') } let(:viewer_class) do Class.new(DiffViewer::Base) do @@ -15,8 +15,7 @@ describe DiffViewer::ServerSide do describe '#prepare!' do it 'loads all diff file data' do - expect(diff_file.old_blob).to receive(:load_all_data!) - expect(diff_file.new_blob).to receive(:load_all_data!) + expect(Blob).to receive(:lazy).at_least(:twice) subject.prepare! end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 1ecb50586c7..6e7e8c4c570 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -231,6 +231,37 @@ describe Note do end end + describe '#cross_reference?' do + it 'falsey for user-generated notes' do + note = create(:note, system: false) + + expect(note.cross_reference?).to be_falsy + end + + context 'when the note might contain cross references' do + SystemNoteMetadata::TYPES_WITH_CROSS_REFERENCES.each do |type| + let(:note) { create(:note, :system) } + let!(:metadata) { create(:system_note_metadata, note: note, action: type) } + + it 'delegates to the cross-reference regex' do + expect(note).to receive(:matches_cross_reference_regex?).and_return(false) + + note.cross_reference? + end + end + end + + context 'when the note cannot contain cross references' do + let(:commit_note) { build(:note, note: 'mentioned in 1312312313 something else.', system: true) } + let(:label_note) { build(:note, note: 'added ~2323232323', system: true) } + + it 'scan for a `mentioned in` prefix' do + expect(commit_note.cross_reference?).to be_truthy + expect(label_note.cross_reference?).to be_falsy + end + end + end + describe 'clear_blank_line_code!' do it 'clears a blank line code before validation' do note = build(:note, line_code: ' ') diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 2aeae6f9ec7..2428e63e149 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -510,6 +510,14 @@ describe API::Users do expect(user.reload.notification_email).to eq('new@email.com') end + it 'skips reconfirmation when requested' do + put api("/users/#{user.id}", admin), { skip_reconfirmation: true } + + user.reload + + expect(user.confirmed_at).to be_present + end + it 'updates user with his own username' do put api("/users/#{user.id}", admin), username: user.username diff --git a/spec/services/milestones/promote_service_spec.rb b/spec/services/milestones/promote_service_spec.rb index 9f2df6d6d19..a0a2843b676 100644 --- a/spec/services/milestones/promote_service_spec.rb +++ b/spec/services/milestones/promote_service_spec.rb @@ -25,6 +25,18 @@ describe Milestones::PromoteService do expect { service.execute(milestone) }.to raise_error(described_class::PromoteMilestoneError) end + + it 'does not promote milestone and update issuables if promoted milestone is not valid' do + issue = create(:issue, milestone: milestone, project: project) + merge_request = create(:merge_request, milestone: milestone, source_project: project) + allow_any_instance_of(Milestone).to receive(:valid?).and_return(false) + + expect { service.execute(milestone) }.to raise_error(described_class::PromoteMilestoneError) + + expect(milestone.reload).to be_persisted + expect(issue.reload.milestone).to eq(milestone) + expect(merge_request.reload.milestone).to eq(milestone) + end end context 'without duplicated milestone titles across projects' do @@ -34,6 +46,16 @@ describe Milestones::PromoteService do expect(promoted_milestone).to be_group_milestone end + it 'does not update issuables without milestone with the new promoted milestone' do + issue_without_milestone = create(:issue, project: project, milestone: nil) + merge_request_without_milestone = create(:merge_request, milestone: nil, source_project: project) + + service.execute(milestone) + + expect(issue_without_milestone.reload.milestone).to be_nil + expect(merge_request_without_milestone.reload.milestone).to be_nil + end + it 'sets issuables with new promoted milestone' do issue = create(:issue, milestone: milestone, project: project) merge_request = create(:merge_request, milestone: milestone, source_project: project) @@ -59,6 +81,20 @@ describe Milestones::PromoteService do expect(Milestone.exists?(milestone_2.id)).to be_falsy end + it 'does not update issuables without milestone with the new promoted milestone' do + issue_without_milestone_1 = create(:issue, project: project, milestone: nil) + issue_without_milestone_2 = create(:issue, project: project_2, milestone: nil) + merge_request_without_milestone_1 = create(:merge_request, milestone: nil, source_project: project) + merge_request_without_milestone_2 = create(:merge_request, milestone: nil, source_project: project_2) + + service.execute(milestone) + + expect(issue_without_milestone_1.reload.milestone).to be_nil + expect(issue_without_milestone_2.reload.milestone).to be_nil + expect(merge_request_without_milestone_1.reload.milestone).to be_nil + expect(merge_request_without_milestone_2.reload.milestone).to be_nil + end + it 'sets all issuables with new promoted milestone' do issue = create(:issue, milestone: milestone, project: project) issue_2 = create(:issue, milestone: milestone_2, project: project_2) diff --git a/spec/tasks/gitlab/gitaly_rake_spec.rb b/spec/tasks/gitlab/gitaly_rake_spec.rb index 5dd8fe8eaa5..255f0a37ec8 100644 --- a/spec/tasks/gitlab/gitaly_rake_spec.rb +++ b/spec/tasks/gitlab/gitaly_rake_spec.rb @@ -57,7 +57,7 @@ describe 'gitlab:gitaly namespace rake task' do it 'calls gmake in the gitaly directory' do expect(Gitlab::Popen).to receive(:popen).with(%w[which gmake]).and_return(['/usr/bin/gmake', 0]) - expect(main_object).to receive(:run_command!).with(command_preamble + %w[gmake BUNDLE_PATH=/fake/bundle_path]).and_return(true) + expect(main_object).to receive(:run_command!).with(command_preamble + %w[gmake BUNDLE_FLAGS=--no-deployment]).and_return(true) run_rake_task('gitlab:gitaly:install', clone_path) end @@ -70,7 +70,7 @@ describe 'gitlab:gitaly namespace rake task' do end it 'calls make in the gitaly directory' do - expect(main_object).to receive(:run_command!).with(command_preamble + %w[make BUNDLE_PATH=/fake/bundle_path]).and_return(true) + expect(main_object).to receive(:run_command!).with(command_preamble + %w[make BUNDLE_FLAGS=--no-deployment]).and_return(true) run_rake_task('gitlab:gitaly:install', clone_path) end |