diff options
65 files changed, 914 insertions, 372 deletions
diff --git a/.gitlab/issue_templates/Feature proposal.md b/.gitlab/issue_templates/Feature proposal.md index 0b22c7bc26b..1bb8d33ff63 100644 --- a/.gitlab/issue_templates/Feature proposal.md +++ b/.gitlab/issue_templates/Feature proposal.md @@ -39,7 +39,7 @@ Existing personas are: (copy relevant personas out of this comment, and delete a ### What does success look like, and how can we measure that? -<!--- Define both the success metrics and acceptance criteria. Note thet success metrics indicate the desired business outcomes, while acceptance criteria indicate when the solution is working correctly. If there is no way to measure success, link to an issue that will implement a way to measure this --> +<!--- Define both the success metrics and acceptance criteria. Note that success metrics indicate the desired business outcomes, while acceptance criteria indicate when the solution is working correctly. If there is no way to measure success, link to an issue that will implement a way to measure this --> ### Links / references diff --git a/CHANGELOG.md b/CHANGELOG.md index 4985c607d57..e220d61b316 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,20 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 11.7.5 (2019-02-06) + +### Fixed (8 changes) + +- Fix import handling errors in Bitbucket Server importer. !24499 +- Adjusts suggestions unable to be applied. !24603 +- Fix 500 errors with legacy appearance logos. !24615 +- Fix form functionality for edit tag page. !24645 +- Update Workhorse to v8.0.2. !24870 +- Downcase aliased OAuth2 callback providers. !24877 +- Fix Detect Host Keys not working. !24884 +- Changed external wiki query method to prevent attribute caching. !24907 + + ## 11.7.2 (2019-01-29) ### Fixed (1 change) diff --git a/app/assets/javascripts/behaviors/markdown/copy_as_gfm.js b/app/assets/javascripts/behaviors/markdown/copy_as_gfm.js index 947d019c725..52d9f2f0322 100644 --- a/app/assets/javascripts/behaviors/markdown/copy_as_gfm.js +++ b/app/assets/javascripts/behaviors/markdown/copy_as_gfm.js @@ -1,8 +1,5 @@ import $ from 'jquery'; -import { DOMParser } from 'prosemirror-model'; import { getSelectedFragment } from '~/lib/utils/common_utils'; -import schema from './schema'; -import markdownSerializer from './serializer'; export class CopyAsGFM { constructor() { @@ -39,9 +36,13 @@ export class CopyAsGFM { div.appendChild(el.cloneNode(true)); const html = div.innerHTML; - clipboardData.setData('text/plain', el.textContent); - clipboardData.setData('text/x-gfm', this.nodeToGFM(el)); - clipboardData.setData('text/html', html); + CopyAsGFM.nodeToGFM(el) + .then(res => { + clipboardData.setData('text/plain', el.textContent); + clipboardData.setData('text/x-gfm', res); + clipboardData.setData('text/html', html); + }) + .catch(() => {}); } static pasteGFM(e) { @@ -137,11 +138,21 @@ export class CopyAsGFM { } static nodeToGFM(node) { - const wrapEl = document.createElement('div'); - wrapEl.appendChild(node.cloneNode(true)); - const doc = DOMParser.fromSchema(schema).parse(wrapEl); - - return markdownSerializer.serialize(doc); + return Promise.all([ + import(/* webpackChunkName: 'gfm_copy_extra' */ 'prosemirror-model'), + import(/* webpackChunkName: 'gfm_copy_extra' */ './schema'), + import(/* webpackChunkName: 'gfm_copy_extra' */ './serializer'), + ]) + .then(([prosemirrorModel, schema, markdownSerializer]) => { + const { DOMParser } = prosemirrorModel; + const wrapEl = document.createElement('div'); + wrapEl.appendChild(node.cloneNode(true)); + const doc = DOMParser.fromSchema(schema.default).parse(wrapEl); + + const res = markdownSerializer.default.serialize(doc); + return res; + }) + .catch(() => {}); } } diff --git a/app/assets/javascripts/behaviors/shortcuts/shortcuts_issuable.js b/app/assets/javascripts/behaviors/shortcuts/shortcuts_issuable.js index 0eb067d4963..680f2031409 100644 --- a/app/assets/javascripts/behaviors/shortcuts/shortcuts_issuable.js +++ b/app/assets/javascripts/behaviors/shortcuts/shortcuts_issuable.js @@ -64,26 +64,30 @@ export default class ShortcutsIssuable extends Shortcuts { const el = CopyAsGFM.transformGFMSelection(documentFragment.cloneNode(true)); const blockquoteEl = document.createElement('blockquote'); blockquoteEl.appendChild(el); - const text = CopyAsGFM.nodeToGFM(blockquoteEl); - - if (text.trim() === '') { - return false; - } - - // If replyField already has some content, add a newline before our quote - const separator = ($replyField.val().trim() !== '' && '\n\n') || ''; - $replyField - .val((a, current) => `${current}${separator}${text}\n\n`) - .trigger('input') - .trigger('change'); - - // Trigger autosize - const event = document.createEvent('Event'); - event.initEvent('autosize:update', true, false); - $replyField.get(0).dispatchEvent(event); + CopyAsGFM.nodeToGFM(blockquoteEl) + .then(text => { + if (text.trim() === '') { + return false; + } + + // If replyField already has some content, add a newline before our quote + const separator = ($replyField.val().trim() !== '' && '\n\n') || ''; + $replyField + .val((a, current) => `${current}${separator}${text}\n\n`) + .trigger('input') + .trigger('change'); + + // Trigger autosize + const event = document.createEvent('Event'); + event.initEvent('autosize:update', true, false); + $replyField.get(0).dispatchEvent(event); + + // Focus the input field + $replyField.focus(); - // Focus the input field - $replyField.focus(); + return false; + }) + .catch(() => {}); return false; } 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 ab194e84ab4..36cac230d9d 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 @@ -32,10 +32,10 @@ export default class MergeRequestStore { this.sourceBranchProtected = data.source_branch_protected; this.conflictsDocsPath = data.conflicts_docs_path; this.mergeStatus = data.merge_status; - this.commitMessage = data.merge_commit_message; + this.commitMessage = data.default_merge_commit_message; this.shortMergeCommitSha = data.short_merge_commit_sha; this.mergeCommitSha = data.merge_commit_sha; - this.commitMessageWithDescription = data.merge_commit_message_with_description; + this.commitMessageWithDescription = data.default_merge_commit_message_with_description; this.commitsCount = data.commits_count; this.divergedCommitsCount = data.diverged_commits_count; this.pipeline = data.pipeline || {}; diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 7c4dc95529a..5cf7fa3422d 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -240,7 +240,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo end def merge_params_attributes - [:should_remove_source_branch, :commit_message, :squash] + [:should_remove_source_branch, :commit_message, :squash_commit_message, :squash] end def merge_when_pipeline_succeeds_active? diff --git a/app/graphql/types/merge_request_type.rb b/app/graphql/types/merge_request_type.rb index fb740b6fb1c..47b915b451e 100644 --- a/app/graphql/types/merge_request_type.rb +++ b/app/graphql/types/merge_request_type.rb @@ -39,7 +39,8 @@ module Types field :rebase_commit_sha, GraphQL::STRING_TYPE, null: true field :rebase_in_progress, GraphQL::BOOLEAN_TYPE, method: :rebase_in_progress?, null: false field :diff_head_sha, GraphQL::STRING_TYPE, null: true - field :merge_commit_message, GraphQL::STRING_TYPE, null: true + field :merge_commit_message, GraphQL::STRING_TYPE, method: :default_merge_commit_message, null: true, deprecation_reason: "Renamed to defaultMergeCommitMessage" + field :default_merge_commit_message, GraphQL::STRING_TYPE, null: true field :merge_ongoing, GraphQL::BOOLEAN_TYPE, method: :merge_ongoing?, null: false field :source_branch_exists, GraphQL::BOOLEAN_TYPE, method: :source_branch_exists?, null: false field :mergeable_discussions_state, GraphQL::BOOLEAN_TYPE, null: true diff --git a/app/helpers/sorting_helper.rb b/app/helpers/sorting_helper.rb index 02762897c89..07ec129dea3 100644 --- a/app/helpers/sorting_helper.rb +++ b/app/helpers/sorting_helper.rb @@ -128,7 +128,9 @@ module SortingHelper sort_value_recently_created => sort_title_recently_created, sort_value_oldest_created => sort_title_oldest_created, sort_value_recently_updated => sort_title_recently_updated, - sort_value_oldest_updated => sort_title_oldest_updated + sort_value_oldest_updated => sort_title_oldest_updated, + sort_value_recently_last_activity => sort_title_recently_last_activity, + sort_value_oldest_last_activity => sort_title_oldest_last_activity } end @@ -317,6 +319,14 @@ module SortingHelper s_('SortOptions|Most stars') end + def sort_title_oldest_last_activity + s_('SortOptions|Oldest last activity') + end + + def sort_title_recently_last_activity + s_('SortOptions|Recent last activity') + end + # Values. def sort_value_access_level_asc 'access_level_asc' @@ -445,4 +455,12 @@ module SortingHelper def sort_value_most_stars 'stars_desc' end + + def sort_value_oldest_last_activity + 'last_activity_on_asc' + end + + def sort_value_recently_last_activity + 'last_activity_on_desc' + end end diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb index 73c1402eae5..73ca17c6605 100644 --- a/app/helpers/users_helper.rb +++ b/app/helpers/users_helper.rb @@ -74,6 +74,15 @@ module UsersHelper Gitlab.config.gitlab.impersonation_enabled end + def user_badges_in_admin_section(user) + [].tap do |badges| + badges << { text: s_('AdminUsers|Blocked'), variant: 'danger' } if user.blocked? + badges << { text: s_('AdminUsers|Admin'), variant: 'success' } if user.admin? + badges << { text: s_('AdminUsers|External'), variant: 'secondary' } if user.external? + badges << { text: s_("AdminUsers|It's you!"), variant: nil } if current_user == user + end + end + private def get_profile_tabs diff --git a/app/models/clusters/concerns/application_version.rb b/app/models/clusters/concerns/application_version.rb index ccad74dc35a..e355de23df6 100644 --- a/app/models/clusters/concerns/application_version.rb +++ b/app/models/clusters/concerns/application_version.rb @@ -7,8 +7,8 @@ module Clusters included do state_machine :status do - after_transition any => [:installing] do |application| - application.update(version: application.class.const_get(:VERSION)) + before_transition any => [:installed, :updated] do |application| + application.version = application.class.const_get(:VERSION) end end end diff --git a/app/models/commit.rb b/app/models/commit.rb index 982e13e2845..f412d252e5c 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -379,7 +379,7 @@ class Commit end def merge_commit? - parents.size > 1 + parent_ids.size > 1 end def merged_merge_request(current_user) diff --git a/app/models/commit_collection.rb b/app/models/commit_collection.rb index 885f61beb05..42ec5b5e664 100644 --- a/app/models/commit_collection.rb +++ b/app/models/commit_collection.rb @@ -3,6 +3,7 @@ # A collection of Commit instances for a specific project and Git reference. class CommitCollection include Enumerable + include Gitlab::Utils::StrongMemoize attr_reader :project, :ref, :commits @@ -20,11 +21,17 @@ class CommitCollection end def committers - emails = commits.reject(&:merge_commit?).map(&:committer_email).uniq + emails = without_merge_commits.map(&:committer_email).uniq User.by_any_email(emails) end + def without_merge_commits + strong_memoize(:without_merge_commits) do + commits.reject(&:merge_commit?) + end + end + # Sets the pipeline status for every commit. # # Setting this status ahead of time removes the need for running a query for diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 84cb8e1c50b..2035bffd829 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -939,7 +939,7 @@ class MergeRequest < ActiveRecord::Base self.target_project.repository.branch_exists?(self.target_branch) end - def merge_commit_message(include_description: false) + def default_merge_commit_message(include_description: false) closes_issues_references = visible_closing_issues_for.map do |issue| issue.to_reference(target_project) end @@ -959,6 +959,13 @@ class MergeRequest < ActiveRecord::Base message.join("\n\n") end + # Returns the oldest multi-line commit message, or the MR title if none found + def default_squash_commit_message + strong_memoize(:default_squash_commit_message) do + commits.without_merge_commits.reverse.find(&:description?)&.safe_message || title + end + end + def reset_merge_when_pipeline_succeeds return unless merge_when_pipeline_succeeds? @@ -967,6 +974,7 @@ class MergeRequest < ActiveRecord::Base if merge_params merge_params.delete('should_remove_source_branch') merge_params.delete('commit_message') + merge_params.delete('squash_commit_message') end self.save diff --git a/app/models/repository.rb b/app/models/repository.rb index 9ae13fbaa80..bfd2608bed4 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -1030,12 +1030,12 @@ class Repository remote_branch: merge_request.target_branch) end - def squash(user, merge_request) + def squash(user, merge_request, message) raw.squash(user, merge_request.id, branch: merge_request.target_branch, start_sha: merge_request.diff_start_sha, end_sha: merge_request.diff_head_sha, author: merge_request.author, - message: merge_request.title) + message: message) end def update_submodule(user, submodule, commit_sha, message:, branch:) diff --git a/app/models/user.rb b/app/models/user.rb index 691abe3175f..9c091ac366c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -267,6 +267,8 @@ class User < ApplicationRecord scope :without_projects, -> { joins('LEFT JOIN project_authorizations ON users.id = project_authorizations.user_id').where(project_authorizations: { user_id: nil }) } scope :order_recent_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('current_sign_in_at', 'DESC')) } scope :order_oldest_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('current_sign_in_at', 'ASC')) } + scope :order_recent_last_activity, -> { reorder(Gitlab::Database.nulls_last_order('last_activity_on', 'DESC')) } + scope :order_oldest_last_activity, -> { reorder(Gitlab::Database.nulls_first_order('last_activity_on', 'ASC')) } scope :confirmed, -> { where.not(confirmed_at: nil) } scope :by_username, -> (usernames) { iwhere(username: Array(usernames).map(&:to_s)) } scope :for_todos, -> (todos) { where(id: todos.select(:user_id)) } @@ -337,6 +339,8 @@ class User < ApplicationRecord case order_method.to_s when 'recent_sign_in' then order_recent_sign_in when 'oldest_sign_in' then order_oldest_sign_in + when 'last_activity_on_desc' then order_recent_last_activity + when 'last_activity_on_asc' then order_oldest_last_activity else order_by(order_method) end diff --git a/app/serializers/merge_request_widget_commit_entity.rb b/app/serializers/merge_request_widget_commit_entity.rb new file mode 100644 index 00000000000..50a5c44a6ad --- /dev/null +++ b/app/serializers/merge_request_widget_commit_entity.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class MergeRequestWidgetCommitEntity < Grape::Entity + expose :safe_message, as: :message + expose :short_id + expose :title +end diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb index f42abf06e1e..2142ceb6122 100644 --- a/app/serializers/merge_request_widget_entity.rb +++ b/app/serializers/merge_request_widget_entity.rb @@ -56,10 +56,23 @@ class MergeRequestWidgetEntity < IssuableEntity merge_request.diff_head_sha.presence end - expose :merge_commit_message expose :actual_head_pipeline, with: PipelineDetailsEntity, as: :pipeline, if: -> (mr, _) { presenter(mr).can_read_pipeline? } + expose :merge_pipeline, with: PipelineDetailsEntity, if: ->(mr, _) { mr.merged? && can?(request.current_user, :read_pipeline, mr.target_project)} + expose :default_squash_commit_message + expose :default_merge_commit_message + + expose :default_merge_commit_message_with_description do |merge_request| + merge_request.default_merge_commit_message(include_description: true) + end + + expose :commits_without_merge_commits, using: MergeRequestWidgetCommitEntity do |merge_request| + merge_request.commits.without_merge_commits + end + + expose :commits_count + # Booleans expose :merge_ongoing?, as: :merge_ongoing expose :work_in_progress?, as: :work_in_progress @@ -77,7 +90,6 @@ class MergeRequestWidgetEntity < IssuableEntity end expose :branch_missing?, as: :branch_missing - expose :commits_count expose :cannot_be_merged?, as: :has_conflicts expose :can_be_merged?, as: :can_be_merged expose :mergeable?, as: :mergeable @@ -205,10 +217,6 @@ class MergeRequestWidgetEntity < IssuableEntity ci_environments_status_project_merge_request_path(merge_request.project, merge_request) end - expose :merge_commit_message_with_description do |merge_request| - merge_request.merge_commit_message(include_description: true) - end - expose :diverged_commits_count do |merge_request| if merge_request.open? && merge_request.diverged_from_target_branch? merge_request.diverged_commits_count diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index 70a67baa01c..449997bcf07 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -8,6 +8,8 @@ module MergeRequests # Executed when you do merge via GitLab UI # class MergeService < MergeRequests::BaseService + include Gitlab::Utils::StrongMemoize + MergeError = Class.new(StandardError) attr_reader :merge_request, :source @@ -37,15 +39,10 @@ module MergeRequests end def source - return merge_request.diff_head_sha unless merge_request.squash - - squash_result = ::MergeRequests::SquashService.new(project, current_user, params).execute(merge_request) - - case squash_result[:status] - when :success - squash_result[:squash_sha] - when :error - raise ::MergeRequests::MergeService::MergeError, squash_result[:message] + if merge_request.squash + squash_sha! + else + merge_request.diff_head_sha end end @@ -82,8 +79,22 @@ module MergeRequests merge_request.update!(merge_commit_sha: commit_id) end + def squash_sha! + strong_memoize(:squash_sha) do + params[:merge_request] = merge_request + squash_result = ::MergeRequests::SquashService.new(project, current_user, params).execute + + case squash_result[:status] + when :success + squash_result[:squash_sha] + when :error + raise ::MergeRequests::MergeService::MergeError, squash_result[:message] + end + end + end + def try_merge - message = params[:commit_message] || merge_request.merge_commit_message + message = params[:commit_message] || merge_request.default_merge_commit_message repository.merge(current_user, source, merge_request, message) rescue Gitlab::Git::PreReceiveError => e diff --git a/app/services/merge_requests/squash_service.rb b/app/services/merge_requests/squash_service.rb index a439a380255..9d1a5d5e6d4 100644 --- a/app/services/merge_requests/squash_service.rb +++ b/app/services/merge_requests/squash_service.rb @@ -2,15 +2,10 @@ module MergeRequests class SquashService < MergeRequests::WorkingCopyBaseService - def execute(merge_request) - @merge_request = merge_request - @repository = target_project.repository - - squash || error('Failed to squash. Should be done manually.') - end - - def squash - if merge_request.commits_count < 2 + def execute + # If performing a squash would result in no change, then + # immediately return a success message without performing a squash + if merge_request.commits_count < 2 && message.nil? return success(squash_sha: merge_request.diff_head_sha) end @@ -18,7 +13,13 @@ module MergeRequests return error('Squash task canceled: another squash is already in progress.') end - squash_sha = repository.squash(current_user, merge_request) + squash! || error('Failed to squash. Should be done manually.') + end + + private + + def squash! + squash_sha = repository.squash(current_user, merge_request, message || merge_request.default_squash_commit_message) success(squash_sha: squash_sha) rescue => e @@ -26,5 +27,17 @@ module MergeRequests log_error(e.message) false end + + def repository + target_project.repository + end + + def merge_request + params[:merge_request] + end + + def message + params[:squash_commit_message].presence + end end end diff --git a/app/views/admin/users/_user.html.haml b/app/views/admin/users/_user.html.haml index a4e2c3252af..be7bfa958b2 100644 --- a/app/views/admin/users/_user.html.haml +++ b/app/views/admin/users/_user.html.haml @@ -1,36 +1,37 @@ -%li.flex-row - .user-avatar - = image_tag avatar_icon_for_user(user), class: "avatar", alt: '' - .row-main-content - .user-name.row-title.str-truncated-100 - = link_to user.name, [:admin, user], class: "js-user-link", data: { user_id: user.id } - - if user.blocked? - %span.badge.badge-danger blocked - - if user.admin? - %span.badge.badge-success Admin - - if user.external? - %span.badge.badge-secondary External - - if user == current_user - %span It's you! - .row-second-line.str-truncated-100 - = mail_to user.email, user.email - .controls - = link_to 'Edit', edit_admin_user_path(user), id: "edit_#{dom_id(user)}", class: 'btn' - - unless user == current_user - .dropdown.inline - %a.dropdown-new.btn.btn-default#project-settings-button{ href: '#', data: { toggle: 'dropdown' } } +.gl-responsive-table-row{ role: 'row' } + .table-section.section-40 + .table-mobile-header{ role: 'rowheader' } + = _('Name') + .table-mobile-content + = render 'user_detail', user: user + .table-section.section-25 + .table-mobile-header{ role: 'rowheader' } + = _('Created on') + .table-mobile-content + = l(user.created_at.to_date, format: :admin) + .table-section.section-15 + .table-mobile-header{ role: 'rowheader' } + = _('Last activity') + .table-mobile-content + = user.last_activity_on.nil? ? _('Never') : l(user.last_activity_on, format: :admin) + .table-section.section-20.table-button-footer + .table-action-buttons + = link_to _('Edit'), edit_admin_user_path(user), id: "edit_#{dom_id(user)}", class: 'btn btn-default' + - unless user == current_user + %button.dropdown-new.btn.btn-default{ type: 'button', data: { toggle: 'dropdown' } } = icon('cog') = icon('caret-down') %ul.dropdown-menu.dropdown-menu-right %li.dropdown-header - Settings + = _('Settings') %li - if user.ldap_blocked? - %span.small Cannot unblock LDAP blocked users + %span.small + = s_('AdminUsers|Cannot unblock LDAP blocked users') - elsif user.blocked? - = link_to 'Unblock', unblock_admin_user_path(user), method: :put + = link_to _('Unblock'), unblock_admin_user_path(user), method: :put - else - = link_to 'Block', block_admin_user_path(user), data: { confirm: 'USER WILL BE BLOCKED! Are you sure?' }, method: :put + = link_to _('Block'), block_admin_user_path(user), data: { confirm: "#{s_('AdminUsers|User will be blocked').upcase}! #{_('Are you sure')}?" }, method: :put - if user.access_locked? %li = link_to _('Unlock'), unlock_admin_user_path(user), method: :put, data: { confirm: _('Are you sure?') } @@ -42,7 +43,7 @@ target: '#delete-user-modal', delete_user_url: admin_user_path(user), block_user_url: block_admin_user_path(user), - username: user.name, + username: sanitize_name(user.name), delete_contributions: false }, type: 'button' } = s_('AdminUsers|Delete user') @@ -51,6 +52,6 @@ target: '#delete-user-modal', delete_user_url: admin_user_path(user, hard_delete: true), block_user_url: block_admin_user_path(user), - username: user.name, + username: sanitize_name(user.name), delete_contributions: true }, type: 'button' } = s_('AdminUsers|Delete user and contributions') diff --git a/app/views/admin/users/_user_detail.html.haml b/app/views/admin/users/_user_detail.html.haml new file mode 100644 index 00000000000..3319b4bad3a --- /dev/null +++ b/app/views/admin/users/_user_detail.html.haml @@ -0,0 +1,17 @@ +.flex-list + .flex-row + = image_tag avatar_icon_for_user(user), class: 'avatar s32 d-none d-md-flex', alt: _('Avatar for %{name}') % { name: sanitize_name(user.name) } + .row-main-content + .row-title.str-truncated-100 + = image_tag avatar_icon_for_user(user), class: 'avatar s16 d-xs-flex d-md-none mr-1 prepend-top-2', alt: _('Avatar for %{name}') % { name: sanitize_name(user.name) } + = link_to user.name, admin_user_path(user), class: 'text-plain js-user-link', data: { user_id: user.id } + + = render_if_exists 'admin/users/user_detail_note', user: user + + - user_badges_in_admin_section(user).each do |badge| + - css_badge = "badge badge-#{badge[:variant]}" if badge[:variant].present? + %span{ class: css_badge } + = badge[:text] + + .row-second-line.str-truncated-100 + = mail_to user.email, user.email, class: 'text-secondary' diff --git a/app/views/admin/users/index.html.haml b/app/views/admin/users/index.html.haml index 600120c4f05..c3d5ce0fe70 100644 --- a/app/views/admin/users/index.html.haml +++ b/app/views/admin/users/index.html.haml @@ -2,72 +2,78 @@ - page_title "Users" %div{ class: container_class } - .prepend-top-default + .top-area.scrolling-tabs-container.inner-page-scroll-tabs + .fade-left + = icon('angle-left') + .fade-right + = icon('angle-right') + %ul.nav-links.nav.nav-tabs.scrolling-tabs + = nav_link(html_options: { class: active_when(params[:filter].nil?) }) do + = link_to admin_users_path do + = s_('AdminUsers|Active') + %small.badge.badge-pill= limited_counter_with_delimiter(User.active) + = nav_link(html_options: { class: active_when(params[:filter] == 'admins') }) do + = link_to admin_users_path(filter: "admins") do + = s_('AdminUsers|Admins') + %small.badge.badge-pill= limited_counter_with_delimiter(User.admins) + = nav_link(html_options: { class: "#{active_when(params[:filter] == 'two_factor_enabled')} filter-two-factor-enabled" }) do + = link_to admin_users_path(filter: 'two_factor_enabled') do + = s_('AdminUsers|2FA Enabled') + %small.badge.badge-pill= limited_counter_with_delimiter(User.with_two_factor) + = nav_link(html_options: { class: "#{active_when(params[:filter] == 'two_factor_disabled')} filter-two-factor-disabled" }) do + = link_to admin_users_path(filter: 'two_factor_disabled') do + = s_('AdminUsers|2FA Disabled') + %small.badge.badge-pill= limited_counter_with_delimiter(User.without_two_factor) + = nav_link(html_options: { class: active_when(params[:filter] == 'external') }) do + = link_to admin_users_path(filter: 'external') do + = s_('AdminUsers|External') + %small.badge.badge-pill= limited_counter_with_delimiter(User.external) + = nav_link(html_options: { class: active_when(params[:filter] == 'blocked') }) do + = link_to admin_users_path(filter: "blocked") do + = s_('AdminUsers|Blocked') + %small.badge.badge-pill= limited_counter_with_delimiter(User.blocked) + = nav_link(html_options: { class: active_when(params[:filter] == 'wop') }) do + = link_to admin_users_path(filter: "wop") do + = s_('AdminUsers|Without projects') + %small.badge.badge-pill= limited_counter_with_delimiter(User.without_projects) + .nav-controls + = render_if_exists 'admin/users/admin_email_users' + = link_to s_('AdminUsers|New user'), new_admin_user_path, class: 'btn btn-success btn-search float-right' + + .filtered-search-block.row-content-block.border-top-0 = form_tag admin_users_path, method: :get do - if params[:filter].present? = hidden_field_tag "filter", h(params[:filter]) .search-holder .search-field-holder - = search_field_tag :search_query, params[:search_query], placeholder: 'Search by name, email or username', class: 'form-control search-text-input js-search-input', spellcheck: false + = search_field_tag :search_query, params[:search_query], placeholder: s_('AdminUsers|Search by name, email or username'), class: 'form-control search-text-input js-search-input', spellcheck: false - if @sort.present? = hidden_field_tag :sort, @sort = icon("search", class: "search-icon") - = button_tag 'Search users' if Rails.env.test? + = button_tag s_('AdminUsers|Search users') if Rails.env.test? .dropdown.user-sort-dropdown - toggle_text = if @sort.present? then users_sort_options_hash[@sort] else sort_title_name end = dropdown_toggle(toggle_text, { toggle: 'dropdown' }) %ul.dropdown-menu.dropdown-menu-right %li.dropdown-header - Sort by + = s_('AdminUsers|Sort by') %li - users_sort_options_hash.each do |value, title| = link_to admin_users_path(sort: value, filter: params[:filter], search_query: params[:search_query]) do = title - = link_to 'New user', new_admin_user_path, class: 'btn btn-success btn-search' - .top-area.scrolling-tabs-container.inner-page-scroll-tabs - .fade-left - = icon('angle-left') - .fade-right - = icon('angle-right') - %ul.nav-links.nav.nav-tabs.scrolling-tabs - = nav_link(html_options: { class: active_when(params[:filter].nil?) }) do - = link_to admin_users_path do - Active - %small.badge.badge-pill= limited_counter_with_delimiter(User.active) - = nav_link(html_options: { class: active_when(params[:filter] == 'admins') }) do - = link_to admin_users_path(filter: "admins") do - Admins - %small.badge.badge-pill= limited_counter_with_delimiter(User.admins) - = nav_link(html_options: { class: "#{active_when(params[:filter] == 'two_factor_enabled')} filter-two-factor-enabled" }) do - = link_to admin_users_path(filter: 'two_factor_enabled') do - 2FA Enabled - %small.badge.badge-pill= limited_counter_with_delimiter(User.with_two_factor) - = nav_link(html_options: { class: "#{active_when(params[:filter] == 'two_factor_disabled')} filter-two-factor-disabled" }) do - = link_to admin_users_path(filter: 'two_factor_disabled') do - 2FA Disabled - %small.badge.badge-pill= limited_counter_with_delimiter(User.without_two_factor) - = nav_link(html_options: { class: active_when(params[:filter] == 'external') }) do - = link_to admin_users_path(filter: 'external') do - External - %small.badge.badge-pill= limited_counter_with_delimiter(User.external) - = nav_link(html_options: { class: active_when(params[:filter] == 'blocked') }) do - = link_to admin_users_path(filter: "blocked") do - Blocked - %small.badge.badge-pill= limited_counter_with_delimiter(User.blocked) - = nav_link(html_options: { class: active_when(params[:filter] == 'wop') }) do - = link_to admin_users_path(filter: "wop") do - Without projects - %small.badge.badge-pill= limited_counter_with_delimiter(User.without_projects) + - if @users.empty? + .nothing-here-block.border-top-0 + = s_('AdminUsers|No users found') + - else + .table-holder + .thead-white.text-nowrap.gl-responsive-table-row.table-row-header{ role: 'row' } + .table-section.section-40{ role: 'rowheader' }= _('Name') + .table-section.section-25{ role: 'rowheader' }= _('Created on') + .table-section.section-15{ role: 'rowheader' }= _('Last activity') - %ul.flex-list.content-list - - if @users.empty? - %li - .nothing-here-block No users found. - - else = render partial: 'admin/users/user', collection: @users = paginate @users, theme: "gitlab" #delete-user-modal - diff --git a/app/views/projects/services/prometheus/_show.html.haml b/app/views/projects/services/prometheus/_show.html.haml index 9d4574c4590..6aafa85e99a 100644 --- a/app/views/projects/services/prometheus/_show.html.haml +++ b/app/views/projects/services/prometheus/_show.html.haml @@ -8,3 +8,5 @@ .col-lg-9 = render 'projects/services/prometheus/metrics', project: @project + += render_if_exists 'projects/services/prometheus/external_alerts', project: @project diff --git a/changelogs/unreleased/43681-display-last-activity-and-created-at-datetimes-for-users-in-admin-users.yml b/changelogs/unreleased/43681-display-last-activity-and-created-at-datetimes-for-users-in-admin-users.yml new file mode 100644 index 00000000000..0fbf6314a27 --- /dev/null +++ b/changelogs/unreleased/43681-display-last-activity-and-created-at-datetimes-for-users-in-admin-users.yml @@ -0,0 +1,5 @@ +--- +title: Display last activity and created at datetimes for users +merge_request: 24181 +author: +type: added diff --git a/changelogs/unreleased/56014-better-squash-commit-messages.yml b/changelogs/unreleased/56014-better-squash-commit-messages.yml new file mode 100644 index 00000000000..b08d584ac0a --- /dev/null +++ b/changelogs/unreleased/56014-better-squash-commit-messages.yml @@ -0,0 +1,6 @@ +--- +title: Default squash commit message is now selected from the longest commit when + squashing merge requests +merge_request: 24518 +author: +type: changed diff --git a/changelogs/unreleased/56424-fix-gl-form-init-tag-editing.yml b/changelogs/unreleased/56424-fix-gl-form-init-tag-editing.yml deleted file mode 100644 index b19b4d650fd..00000000000 --- a/changelogs/unreleased/56424-fix-gl-form-init-tag-editing.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix form functionality for edit tag page -merge_request: 24645 -author: -type: fixed diff --git a/changelogs/unreleased/cluster_application_version_updated.yml b/changelogs/unreleased/cluster_application_version_updated.yml new file mode 100644 index 00000000000..34fe55dcc5e --- /dev/null +++ b/changelogs/unreleased/cluster_application_version_updated.yml @@ -0,0 +1,5 @@ +--- +title: Update cluster application version on updated and installed status +merge_request: 24810 +author: +type: other diff --git a/changelogs/unreleased/fj-regression-external-wiki-url.yml b/changelogs/unreleased/fj-regression-external-wiki-url.yml deleted file mode 100644 index d4f21dab982..00000000000 --- a/changelogs/unreleased/fj-regression-external-wiki-url.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Changed external wiki query method to prevent attribute caching -merge_request: 24907 -author: -type: fixed diff --git a/changelogs/unreleased/osw-adjusts-suggestions-unable-to-be-applied.yml b/changelogs/unreleased/osw-adjusts-suggestions-unable-to-be-applied.yml deleted file mode 100644 index 3ba62b92413..00000000000 --- a/changelogs/unreleased/osw-adjusts-suggestions-unable-to-be-applied.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Adjusts suggestions unable to be applied -merge_request: 24603 -author: -type: fixed diff --git a/changelogs/unreleased/sh-fix-detect-host-keys.yml b/changelogs/unreleased/sh-fix-detect-host-keys.yml deleted file mode 100644 index 993d7c35b18..00000000000 --- a/changelogs/unreleased/sh-fix-detect-host-keys.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix Detect Host Keys not working -merge_request: 24884 -author: -type: fixed diff --git a/changelogs/unreleased/sh-fix-issue-9357.yml b/changelogs/unreleased/sh-fix-issue-9357.yml deleted file mode 100644 index 756cd6047b8..00000000000 --- a/changelogs/unreleased/sh-fix-issue-9357.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix 500 errors with legacy appearance logos -merge_request: 24615 -author: -type: fixed diff --git a/changelogs/unreleased/sh-fix-oauth2-callback-caps.yml b/changelogs/unreleased/sh-fix-oauth2-callback-caps.yml deleted file mode 100644 index 8d17900cb79..00000000000 --- a/changelogs/unreleased/sh-fix-oauth2-callback-caps.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Downcase aliased OAuth2 callback providers -merge_request: 24877 -author: -type: fixed diff --git a/changelogs/unreleased/sh-remove-bitbucket-mirror-constant.yml b/changelogs/unreleased/sh-remove-bitbucket-mirror-constant.yml deleted file mode 100644 index 8c0b000220f..00000000000 --- a/changelogs/unreleased/sh-remove-bitbucket-mirror-constant.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix import handling errors in Bitbucket Server importer -merge_request: 24499 -author: -type: fixed diff --git a/config/locales/de.yml b/config/locales/de.yml index 38c3711c6c7..554e9913faa 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -43,6 +43,7 @@ de: default: "%d.%m.%Y" long: "%e. %B %Y" short: "%e. %b" + admin: "%e %b, %Y" month_names: - - Januar diff --git a/config/locales/en.yml b/config/locales/en.yml index 0a43a1d9a6b..e8dbc033a7c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -55,6 +55,7 @@ en: default: "%Y-%m-%d" long: "%B %d, %Y" short: "%b %d" + admin: "%e %b, %Y" month_names: - - January diff --git a/config/locales/es.yml b/config/locales/es.yml index fdc52b4ae11..78c07583933 100644 --- a/config/locales/es.yml +++ b/config/locales/es.yml @@ -42,6 +42,7 @@ es: default: "%d/%m/%Y" long: "%d de %B de %Y" short: "%d de %b" + admin: "%e %b, %Y" month_names: - - enero diff --git a/doc/user/project/merge_requests/squash_and_merge.md b/doc/user/project/merge_requests/squash_and_merge.md index 1b57331dbe7..34cba867e2c 100644 --- a/doc/user/project/merge_requests/squash_and_merge.md +++ b/doc/user/project/merge_requests/squash_and_merge.md @@ -18,10 +18,14 @@ Into a single commit on merge: ![A squashed commit followed by a merge commit][squashed-commit] -The squashed commit's commit message is the merge request title. And note that -the squashed commit is still followed by a merge commit, as the merge -method for this example repository uses a merge commit. Squashing also works -with the fast-forward merge strategy, see +The squashed commit's commit message will be either: + +- Taken from the first multi-line commit message in the merge. +- The merge request's title if no multi-line commit message is found. + +Note that the squashed commit is still followed by a merge commit, +as the merge method for this example repository uses a merge commit. +Squashing also works with the fast-forward merge strategy, see [squashing and fast-forward merge](#squash-and-fast-forward-merge) for more details. @@ -34,7 +38,7 @@ you'd rather not include them in your target branch. With squash and merge, when the merge request is ready to be merged, all you have to do is enable squashing before you press merge to join -the commits include in the merge request into a single commit. +the commits in the merge request into a single commit. This way, the history of your base branch remains clean with meaningful commit messages and is simpler to [revert] if necessary. @@ -56,7 +60,7 @@ This can then be overridden at the time of accepting the merge request: The squashed commit has the following metadata: -- Message: the title of the merge request. +- Message: the message of the squash commit. - Author: the author of the merge request. - Committer: the user who initiated the squash. diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 9f1394571d8..a1f0efa3c68 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -1116,7 +1116,9 @@ module API class Release < TagRelease expose :name - expose :description_html + expose :description_html do |entity| + MarkupHelper.markdown_field(entity, :description) + end expose :created_at expose :author, using: Entities::UserBasic, if: -> (release, _) { release.author.present? } expose :commit, using: Entities::Commit diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 1caf263b9f3..9ec590f90d8 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -468,9 +468,30 @@ msgstr "" msgid "AdminSettings|When creating a new environment variable it will be protected by default." msgstr "" +msgid "AdminUsers|2FA Disabled" +msgstr "" + +msgid "AdminUsers|2FA Enabled" +msgstr "" + +msgid "AdminUsers|Active" +msgstr "" + +msgid "AdminUsers|Admin" +msgstr "" + +msgid "AdminUsers|Admins" +msgstr "" + msgid "AdminUsers|Block user" msgstr "" +msgid "AdminUsers|Blocked" +msgstr "" + +msgid "AdminUsers|Cannot unblock LDAP blocked users" +msgstr "" + msgid "AdminUsers|Delete User %{username} and contributions?" msgstr "" @@ -483,12 +504,39 @@ msgstr "" msgid "AdminUsers|Delete user and contributions" msgstr "" +msgid "AdminUsers|External" +msgstr "" + +msgid "AdminUsers|It's you!" +msgstr "" + +msgid "AdminUsers|New user" +msgstr "" + +msgid "AdminUsers|No users found" +msgstr "" + +msgid "AdminUsers|Search by name, email or username" +msgstr "" + +msgid "AdminUsers|Search users" +msgstr "" + +msgid "AdminUsers|Sort by" +msgstr "" + msgid "AdminUsers|To confirm, type %{projectName}" msgstr "" msgid "AdminUsers|To confirm, type %{username}" msgstr "" +msgid "AdminUsers|User will be blocked" +msgstr "" + +msgid "AdminUsers|Without projects" +msgstr "" + msgid "Advanced permissions, Large File Storage and Two-Factor authentication settings." msgstr "" @@ -711,6 +759,9 @@ msgstr "" msgid "Archived projects" msgstr "" +msgid "Are you sure" +msgstr "" + msgid "Are you sure you want to delete this pipeline schedule?" msgstr "" @@ -909,6 +960,9 @@ msgstr "" msgid "Avatar for %{assigneeName}" msgstr "" +msgid "Avatar for %{name}" +msgstr "" + msgid "Avatar will be removed. Are you sure?" msgstr "" @@ -1014,6 +1068,9 @@ msgstr "" msgid "Bitbucket import" msgstr "" +msgid "Block" +msgstr "" + msgid "Blocked" msgstr "" @@ -2348,6 +2405,9 @@ msgstr "" msgid "Created by me" msgstr "" +msgid "Created on" +msgstr "" + msgid "Created on:" msgstr "" @@ -4124,6 +4184,9 @@ msgstr[1] "" msgid "Last Pipeline" msgstr "" +msgid "Last activity" +msgstr "" + msgid "Last commit" msgstr "" @@ -6668,6 +6731,9 @@ msgstr "" msgid "SortOptions|Oldest joined" msgstr "" +msgid "SortOptions|Oldest last activity" +msgstr "" + msgid "SortOptions|Oldest sign in" msgstr "" @@ -6680,6 +6746,9 @@ msgstr "" msgid "SortOptions|Priority" msgstr "" +msgid "SortOptions|Recent last activity" +msgstr "" + msgid "SortOptions|Recent sign in" msgstr "" @@ -7668,6 +7737,9 @@ msgstr "" msgid "Unable to load the diff. %{button_try_again}" msgstr "" +msgid "Unblock" +msgstr "" + msgid "Undo" msgstr "" diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index ca5ff9b1e3b..79f97aa4170 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -387,6 +387,23 @@ describe Projects::MergeRequestsController do end end + context 'when a squash commit message is passed' do + let(:message) { 'My custom squash commit message' } + + it 'passes the same message to SquashService' do + params = { squash: '1', squash_commit_message: message } + + expect_next_instance_of(MergeRequests::SquashService, project, user, params.merge(merge_request: merge_request)) do |squash_service| + expect(squash_service).to receive(:execute).and_return({ + status: :success, + squash_sha: SecureRandom.hex(20) + }) + end + + merge_with_sha(params) + end + end + context 'when the pipeline succeeds is passed' do let!(:head_pipeline) do create(:ci_empty_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch, head_pipeline_of: merge_request) diff --git a/spec/factories/commits.rb b/spec/factories/commits.rb index 818f7b046f6..2bcc4b6cf52 100644 --- a/spec/factories/commits.rb +++ b/spec/factories/commits.rb @@ -16,14 +16,24 @@ FactoryBot.define do commit end + project + skip_create # Commits cannot be persisted + initialize_with do new(git_commit, project) end after(:build) do |commit, evaluator| allow(commit).to receive(:author).and_return(evaluator.author || build_stubbed(:author)) + allow(commit).to receive(:parent_ids).and_return([]) + end + + trait :merge_commit do + after(:build) do |commit| + allow(commit).to receive(:parent_ids).and_return(Array.new(2) { SecureRandom.hex(20) }) + end end trait :without_author do diff --git a/spec/features/admin/admin_users_spec.rb b/spec/features/admin/admin_users_spec.rb index 931095936a6..b1c6f308bc6 100644 --- a/spec/features/admin/admin_users_spec.rb +++ b/spec/features/admin/admin_users_spec.rb @@ -1,13 +1,13 @@ require 'spec_helper' describe "Admin::Users" do - include Spec::Support::Helpers::Features::ListRowsHelpers + include Spec::Support::Helpers::Features::ResponsiveTableHelpers let!(:user) do create(:omniauth_user, provider: 'twitter', extern_uid: '123456') end - let!(:current_user) { create(:admin) } + let!(:current_user) { create(:admin, last_activity_on: 5.days.ago) } before do sign_in(current_user) @@ -25,6 +25,8 @@ describe "Admin::Users" do it "has users list" do expect(page).to have_content(current_user.email) expect(page).to have_content(current_user.name) + expect(page).to have_content(current_user.created_at.strftime("%e %b, %Y")) + expect(page).to have_content(current_user.last_activity_on.strftime("%e %b, %Y")) expect(page).to have_content(user.email) expect(page).to have_content(user.name) expect(page).to have_link('Block', href: block_admin_user_path(user)) @@ -32,10 +34,24 @@ describe "Admin::Users" do expect(page).to have_button('Delete user and contributions') end + describe "view extra user information", :js do + it 'does not have the user popover open' do + expect(page).not_to have_selector('#__BV_popover_1__') + end + + it 'shows the user popover on hover' do + first_user_link = page.first('.js-user-link') + + first_user_link.hover + + expect(page).to have_selector('#__BV_popover_1__') + end + end + describe 'search and sort' do before do - create(:user, name: 'Foo Bar') - create(:user, name: 'Foo Baz') + create(:user, name: 'Foo Bar', last_activity_on: 3.days.ago) + create(:user, name: 'Foo Baz', last_activity_on: 2.days.ago) create(:user, name: 'Dmitriy') end @@ -75,6 +91,24 @@ describe "Admin::Users" do expect(first_row.text).to include('Foo Bar') expect(second_row.text).to include('Foo Baz') end + + it 'sorts users by recent last activity' do + visit admin_users_path(search_query: 'Foo') + + sort_by('Recent last activity') + + expect(first_row.text).to include('Foo Baz') + expect(second_row.text).to include('Foo Bar') + end + + it 'sorts users by oldest last activity' do + visit admin_users_path(search_query: 'Foo') + + sort_by('Oldest last activity') + + expect(first_row.text).to include('Foo Bar') + expect(second_row.text).to include('Foo Baz') + end end describe 'Two-factor Authentication filters' do diff --git a/spec/features/markdown/copy_as_gfm_spec.rb b/spec/features/markdown/copy_as_gfm_spec.rb index 16754035076..60ddb02da2c 100644 --- a/spec/features/markdown/copy_as_gfm_spec.rb +++ b/spec/features/markdown/copy_as_gfm_spec.rb @@ -843,6 +843,7 @@ describe 'Copy as GFM', :js do def verify(selector, gfm, target: nil) html = html_for_selector(selector) output_gfm = html_to_gfm(html, 'transformCodeSelection', target: target) + wait_for_requests expect(output_gfm.strip).to eq(gfm.strip) end end @@ -861,6 +862,9 @@ describe 'Copy as GFM', :js do def html_to_gfm(html, transformer = 'transformGFMSelection', target: nil) js = <<~JS (function(html) { + // Setting it off so the import already starts + window.CopyAsGFM.nodeToGFM(document.createElement('div')); + var transformer = window.CopyAsGFM[#{transformer.inspect}]; var node = document.createElement('div'); @@ -875,9 +879,18 @@ describe 'Copy as GFM', :js do node = transformer(node, target); if (!node) return null; - return window.CopyAsGFM.nodeToGFM(node); + + window.gfmCopytestRes = null; + window.CopyAsGFM.nodeToGFM(node) + .then((res) => { + window.gfmCopytestRes = res; + }); })("#{escape_javascript(html)}") JS - page.evaluate_script(js) + page.execute_script(js) + + loop until page.evaluate_script('window.gfmCopytestRes !== null') + + page.evaluate_script('window.gfmCopytestRes') end end diff --git a/spec/features/merge_requests/user_squashes_merge_request_spec.rb b/spec/features/merge_requests/user_squashes_merge_request_spec.rb index 47f9f10815c..bf9c55cf22c 100644 --- a/spec/features/merge_requests/user_squashes_merge_request_spec.rb +++ b/spec/features/merge_requests/user_squashes_merge_request_spec.rb @@ -14,7 +14,7 @@ describe 'User squashes a merge request', :js do latest_master_commits = project.repository.commits_between(original_head.sha, 'master').map(&:raw) squash_commit = an_object_having_attributes(sha: a_string_matching(/\h{40}/), - message: "Csv\n", + message: a_string_starting_with(project.merge_requests.first.default_squash_commit_message), author_name: user.name, committer_name: user.name) diff --git a/spec/fixtures/api/schemas/entities/merge_request_widget.json b/spec/fixtures/api/schemas/entities/merge_request_widget.json index 1bd39a46830..67c209f3fc3 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_widget.json +++ b/spec/fixtures/api/schemas/entities/merge_request_widget.json @@ -44,7 +44,7 @@ "merge_user": { "type": ["object", "null"] }, "diff_head_sha": { "type": ["string", "null"] }, "diff_head_commit_short_id": { "type": ["string", "null"] }, - "merge_commit_message": { "type": ["string", "null"] }, + "default_merge_commit_message": { "type": ["string", "null"] }, "pipeline": { "type": ["object", "null"] }, "merge_pipeline": { "type": ["object", "null"] }, "work_in_progress": { "type": "boolean" }, @@ -102,7 +102,9 @@ "new_blob_path": { "type": ["string", "null"] }, "merge_check_path": { "type": "string" }, "ci_environments_status_path": { "type": "string" }, - "merge_commit_message_with_description": { "type": "string" }, + "default_merge_commit_message_with_description": { "type": "string" }, + "default_squash_commit_message": { "type": "string" }, + "commits_without_merge_commits": { "type": "array" }, "diverged_commits_count": { "type": "integer" }, "commit_change_content_path": { "type": "string" }, "merge_commit_path": { "type": ["string", "null"] }, diff --git a/spec/helpers/users_helper_spec.rb b/spec/helpers/users_helper_spec.rb index ab67a5ab847..f3649495493 100644 --- a/spec/helpers/users_helper_spec.rb +++ b/spec/helpers/users_helper_spec.rb @@ -100,4 +100,72 @@ describe UsersHelper do end end end + + describe '#user_badges_in_admin_section' do + before do + allow(helper).to receive(:current_user).and_return(user) + end + + context 'with a blocked user' do + it "returns the blocked badge" do + blocked_user = create(:user, state: 'blocked') + + badges = helper.user_badges_in_admin_section(blocked_user) + + expect(badges).to eq([text: "Blocked", variant: "danger"]) + end + end + + context 'with an admin user' do + it "returns the admin badge" do + admin_user = create(:admin) + + badges = helper.user_badges_in_admin_section(admin_user) + + expect(badges).to eq([text: "Admin", variant: "success"]) + end + end + + context 'with an external user' do + it 'returns the external badge' do + external_user = create(:user, external: true) + + badges = helper.user_badges_in_admin_section(external_user) + + expect(badges).to eq([text: "External", variant: "secondary"]) + end + end + + context 'with the current user' do + it 'returns the "It\'s You" badge' do + badges = helper.user_badges_in_admin_section(user) + + expect(badges).to eq([text: "It's you!", variant: nil]) + end + end + + context 'with an external blocked admin' do + it 'returns the blocked, admin and external badges' do + user = create(:admin, state: 'blocked', external: true) + + badges = helper.user_badges_in_admin_section(user) + + expect(badges).to eq([ + { text: "Blocked", variant: "danger" }, + { text: "Admin", variant: "success" }, + { text: "External", variant: "secondary" } + ]) + end + end + + context 'get badges for normal user' do + it 'returns no badges' do + user = create(:user) + + badges = helper.user_badges_in_admin_section(user) + + expect(badges).to be_empty + end + end + end end diff --git a/spec/javascripts/behaviors/copy_as_gfm_spec.js b/spec/javascripts/behaviors/copy_as_gfm_spec.js index 6179a02ce16..ca849f75860 100644 --- a/spec/javascripts/behaviors/copy_as_gfm_spec.js +++ b/spec/javascripts/behaviors/copy_as_gfm_spec.js @@ -1,4 +1,4 @@ -import { CopyAsGFM } from '~/behaviors/markdown/copy_as_gfm'; +import initCopyAsGFM, { CopyAsGFM } from '~/behaviors/markdown/copy_as_gfm'; describe('CopyAsGFM', () => { describe('CopyAsGFM.pasteGFM', () => { @@ -79,27 +79,46 @@ describe('CopyAsGFM', () => { return clipboardData; }; + beforeAll(done => { + initCopyAsGFM(); + + // Fake call to nodeToGfm so the import of lazy bundle happened + CopyAsGFM.nodeToGFM(document.createElement('div')) + .then(() => { + done(); + }) + .catch(done.fail); + }); + beforeEach(() => spyOn(clipboardData, 'setData')); describe('list handling', () => { - it('uses correct gfm for unordered lists', () => { + it('uses correct gfm for unordered lists', done => { const selection = stubSelection('<li>List Item1</li><li>List Item2</li>\n', 'UL'); + spyOn(window, 'getSelection').and.returnValue(selection); simulateCopy(); - const expectedGFM = '* List Item1\n\n* List Item2'; + setTimeout(() => { + const expectedGFM = '* List Item1\n\n* List Item2'; - expect(clipboardData.setData).toHaveBeenCalledWith('text/x-gfm', expectedGFM); + expect(clipboardData.setData).toHaveBeenCalledWith('text/x-gfm', expectedGFM); + done(); + }); }); - it('uses correct gfm for ordered lists', () => { + it('uses correct gfm for ordered lists', done => { const selection = stubSelection('<li>List Item1</li><li>List Item2</li>\n', 'OL'); + spyOn(window, 'getSelection').and.returnValue(selection); simulateCopy(); - const expectedGFM = '1. List Item1\n\n1. List Item2'; + setTimeout(() => { + const expectedGFM = '1. List Item1\n\n1. List Item2'; - expect(clipboardData.setData).toHaveBeenCalledWith('text/x-gfm', expectedGFM); + expect(clipboardData.setData).toHaveBeenCalledWith('text/x-gfm', expectedGFM); + done(); + }); }); }); }); diff --git a/spec/javascripts/behaviors/shortcuts/shortcuts_issuable_spec.js b/spec/javascripts/behaviors/shortcuts/shortcuts_issuable_spec.js index fe827bb1e18..4843a0386b5 100644 --- a/spec/javascripts/behaviors/shortcuts/shortcuts_issuable_spec.js +++ b/spec/javascripts/behaviors/shortcuts/shortcuts_issuable_spec.js @@ -3,17 +3,26 @@ */ import $ from 'jquery'; -import initCopyAsGFM from '~/behaviors/markdown/copy_as_gfm'; +import initCopyAsGFM, { CopyAsGFM } from '~/behaviors/markdown/copy_as_gfm'; import ShortcutsIssuable from '~/behaviors/shortcuts/shortcuts_issuable'; -initCopyAsGFM(); - const FORM_SELECTOR = '.js-main-target-form .js-vue-comment-form'; describe('ShortcutsIssuable', function() { const fixtureName = 'snippets/show.html.raw'; preloadFixtures(fixtureName); + beforeAll(done => { + initCopyAsGFM(); + + // Fake call to nodeToGfm so the import of lazy bundle happened + CopyAsGFM.nodeToGFM(document.createElement('div')) + .then(() => { + done(); + }) + .catch(done.fail); + }); + beforeEach(() => { loadFixtures(fixtureName); $('body').append( @@ -63,17 +72,22 @@ describe('ShortcutsIssuable', function() { stubSelection('<p>Selected text.</p>'); }); - it('leaves existing input intact', () => { + it('leaves existing input intact', done => { $(FORM_SELECTOR).val('This text was already here.'); expect($(FORM_SELECTOR).val()).toBe('This text was already here.'); ShortcutsIssuable.replyWithSelectedText(true); - expect($(FORM_SELECTOR).val()).toBe('This text was already here.\n\n> Selected text.\n\n'); + setTimeout(() => { + expect($(FORM_SELECTOR).val()).toBe( + 'This text was already here.\n\n> Selected text.\n\n', + ); + done(); + }); }); - it('triggers `input`', () => { + it('triggers `input`', done => { let triggered = false; $(FORM_SELECTOR).on('input', () => { triggered = true; @@ -81,36 +95,48 @@ describe('ShortcutsIssuable', function() { ShortcutsIssuable.replyWithSelectedText(true); - expect(triggered).toBe(true); + setTimeout(() => { + expect(triggered).toBe(true); + done(); + }); }); - it('triggers `focus`', () => { + it('triggers `focus`', done => { const spy = spyOn(document.querySelector(FORM_SELECTOR), 'focus'); ShortcutsIssuable.replyWithSelectedText(true); - expect(spy).toHaveBeenCalled(); + setTimeout(() => { + expect(spy).toHaveBeenCalled(); + done(); + }); }); }); describe('with a one-line selection', () => { - it('quotes the selection', () => { + it('quotes the selection', done => { stubSelection('<p>This text has been selected.</p>'); ShortcutsIssuable.replyWithSelectedText(true); - expect($(FORM_SELECTOR).val()).toBe('> This text has been selected.\n\n'); + setTimeout(() => { + expect($(FORM_SELECTOR).val()).toBe('> This text has been selected.\n\n'); + done(); + }); }); }); describe('with a multi-line selection', () => { - it('quotes the selected lines as a group', () => { + it('quotes the selected lines as a group', done => { stubSelection( '<p>Selected line one.</p>\n<p>Selected line two.</p>\n<p>Selected line three.</p>', ); ShortcutsIssuable.replyWithSelectedText(true); - expect($(FORM_SELECTOR).val()).toBe( - '> Selected line one.\n>\n> Selected line two.\n>\n> Selected line three.\n\n', - ); + setTimeout(() => { + expect($(FORM_SELECTOR).val()).toBe( + '> Selected line one.\n>\n> Selected line two.\n>\n> Selected line three.\n\n', + ); + done(); + }); }); }); @@ -119,17 +145,23 @@ describe('ShortcutsIssuable', function() { stubSelection('<p>Selected text.</p>', true); }); - it('does not add anything to the input', () => { + it('does not add anything to the input', done => { ShortcutsIssuable.replyWithSelectedText(true); - expect($(FORM_SELECTOR).val()).toBe(''); + setTimeout(() => { + expect($(FORM_SELECTOR).val()).toBe(''); + done(); + }); }); - it('triggers `focus`', () => { + it('triggers `focus`', done => { const spy = spyOn(document.querySelector(FORM_SELECTOR), 'focus'); ShortcutsIssuable.replyWithSelectedText(true); - expect(spy).toHaveBeenCalled(); + setTimeout(() => { + expect(spy).toHaveBeenCalled(); + done(); + }); }); }); @@ -138,20 +170,26 @@ describe('ShortcutsIssuable', function() { stubSelection('<div class="md">Selected text.</div><p>Invalid selected text.</p>', true); }); - it('only adds the valid part to the input', () => { + it('only adds the valid part to the input', done => { ShortcutsIssuable.replyWithSelectedText(true); - expect($(FORM_SELECTOR).val()).toBe('> Selected text.\n\n'); + setTimeout(() => { + expect($(FORM_SELECTOR).val()).toBe('> Selected text.\n\n'); + done(); + }); }); - it('triggers `focus`', () => { + it('triggers `focus`', done => { const spy = spyOn(document.querySelector(FORM_SELECTOR), 'focus'); ShortcutsIssuable.replyWithSelectedText(true); - expect(spy).toHaveBeenCalled(); + setTimeout(() => { + expect(spy).toHaveBeenCalled(); + done(); + }); }); - it('triggers `input`', () => { + it('triggers `input`', done => { let triggered = false; $(FORM_SELECTOR).on('input', () => { triggered = true; @@ -159,7 +197,10 @@ describe('ShortcutsIssuable', function() { ShortcutsIssuable.replyWithSelectedText(true); - expect(triggered).toBe(true); + setTimeout(() => { + expect(triggered).toBe(true); + done(); + }); }); }); @@ -183,20 +224,26 @@ describe('ShortcutsIssuable', function() { }); }); - it('adds the quoted selection to the input', () => { + it('adds the quoted selection to the input', done => { ShortcutsIssuable.replyWithSelectedText(true); - expect($(FORM_SELECTOR).val()).toBe('> *Selected text.*\n\n'); + setTimeout(() => { + expect($(FORM_SELECTOR).val()).toBe('> *Selected text.*\n\n'); + done(); + }); }); - it('triggers `focus`', () => { + it('triggers `focus`', done => { const spy = spyOn(document.querySelector(FORM_SELECTOR), 'focus'); ShortcutsIssuable.replyWithSelectedText(true); - expect(spy).toHaveBeenCalled(); + setTimeout(() => { + expect(spy).toHaveBeenCalled(); + done(); + }); }); - it('triggers `input`', () => { + it('triggers `input`', done => { let triggered = false; $(FORM_SELECTOR).on('input', () => { triggered = true; @@ -204,7 +251,10 @@ describe('ShortcutsIssuable', function() { ShortcutsIssuable.replyWithSelectedText(true); - expect(triggered).toBe(true); + setTimeout(() => { + expect(triggered).toBe(true); + done(); + }); }); }); @@ -228,17 +278,23 @@ describe('ShortcutsIssuable', function() { }); }); - it('does not add anything to the input', () => { + it('does not add anything to the input', done => { ShortcutsIssuable.replyWithSelectedText(true); - expect($(FORM_SELECTOR).val()).toBe(''); + setTimeout(() => { + expect($(FORM_SELECTOR).val()).toBe(''); + done(); + }); }); - it('triggers `focus`', () => { + it('triggers `focus`', done => { const spy = spyOn(document.querySelector(FORM_SELECTOR), 'focus'); ShortcutsIssuable.replyWithSelectedText(true); - expect(spy).toHaveBeenCalled(); + setTimeout(() => { + expect(spy).toHaveBeenCalled(); + done(); + }); }); }); }); diff --git a/spec/javascripts/vue_mr_widget/mock_data.js b/spec/javascripts/vue_mr_widget/mock_data.js index 072e98fc0e8..75b197fb2ba 100644 --- a/spec/javascripts/vue_mr_widget/mock_data.js +++ b/spec/javascripts/vue_mr_widget/mock_data.js @@ -58,7 +58,7 @@ export default { merge_user: null, diff_head_sha: '104096c51715e12e7ae41f9333e9fa35b73f385d', diff_head_commit_short_id: '104096c5', - merge_commit_message: + default_merge_commit_message: "Merge branch 'daaaa' into 'master'\n\nUpdate README.md\n\nSee merge request !22", pipeline: { id: 172, @@ -213,7 +213,7 @@ export default { merge_check_path: '/root/acets-app/merge_requests/22/merge_check', ci_environments_status_url: '/root/acets-app/merge_requests/22/ci_environments_status', project_archived: false, - merge_commit_message_with_description: + default_merge_commit_message_with_description: "Merge branch 'daaaa' into 'master'\n\nUpdate README.md\n\nSee merge request !22", diverged_commits_count: 0, only_allow_merge_if_pipeline_succeeds: false, diff --git a/spec/models/clusters/applications/cert_manager_spec.rb b/spec/models/clusters/applications/cert_manager_spec.rb index 8e14abe098d..79a06c35459 100644 --- a/spec/models/clusters/applications/cert_manager_spec.rb +++ b/spec/models/clusters/applications/cert_manager_spec.rb @@ -4,20 +4,8 @@ describe Clusters::Applications::CertManager do let(:cert_manager) { create(:clusters_applications_cert_managers) } include_examples 'cluster application core specs', :clusters_applications_cert_managers - - describe '#make_installing!' do - before do - application.make_installing! - end - - context 'application install previously errored with older version' do - let(:application) { create(:clusters_applications_cert_managers, :scheduled, version: 'v0.4.0') } - - it 'updates the application version' do - expect(application.reload.version).to eq('v0.5.2') - end - end - end + include_examples 'cluster application status specs', :clusters_applications_cert_managers + include_examples 'cluster application initial status specs' describe '#install_command' do let(:cluster_issuer_file) { { "cluster_issuer.yaml": "---\napiVersion: certmanager.k8s.io/v1alpha1\nkind: ClusterIssuer\nmetadata:\n name: letsencrypt-prod\nspec:\n acme:\n server: https://acme-v02.api.letsencrypt.org/directory\n email: admin@example.com\n privateKeySecretRef:\n name: letsencrypt-prod\n http01: {}\n" } } diff --git a/spec/models/clusters/applications/ingress_spec.rb b/spec/models/clusters/applications/ingress_spec.rb index 52c347229c6..6d48131d1cc 100644 --- a/spec/models/clusters/applications/ingress_spec.rb +++ b/spec/models/clusters/applications/ingress_spec.rb @@ -8,6 +8,7 @@ describe Clusters::Applications::Ingress do include_examples 'cluster application core specs', :clusters_applications_ingress include_examples 'cluster application status specs', :clusters_applications_ingress include_examples 'cluster application helm specs', :clusters_applications_ingress + include_examples 'cluster application initial status specs' before do allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_in) @@ -26,20 +27,6 @@ describe Clusters::Applications::Ingress do it { is_expected.to contain_exactly(cluster) } end - describe '#make_installing!' do - before do - application.make_installing! - end - - context 'application install previously errored with older version' do - let(:application) { create(:clusters_applications_ingress, :scheduled, version: '0.22.0') } - - it 'updates the application version' do - expect(application.reload.version).to eq('1.1.2') - end - end - end - describe '#make_installed!' do before do application.make_installed! diff --git a/spec/models/clusters/applications/jupyter_spec.rb b/spec/models/clusters/applications/jupyter_spec.rb index 391e5425384..b73a243f6e0 100644 --- a/spec/models/clusters/applications/jupyter_spec.rb +++ b/spec/models/clusters/applications/jupyter_spec.rb @@ -2,6 +2,7 @@ require 'rails_helper' describe Clusters::Applications::Jupyter do include_examples 'cluster application core specs', :clusters_applications_jupyter + include_examples 'cluster application status specs', :clusters_applications_jupyter include_examples 'cluster application helm specs', :clusters_applications_jupyter it { is_expected.to belong_to(:oauth_application) } @@ -26,20 +27,6 @@ describe Clusters::Applications::Jupyter do end end - describe '#make_installing!' do - before do - application.make_installing! - end - - context 'application install previously errored with older version' do - let(:application) { create(:clusters_applications_jupyter, :scheduled, version: 'v0.5') } - - it 'updates the application version' do - expect(application.reload.version).to eq('v0.6') - end - end - end - describe '#install_command' do let!(:ingress) { create(:clusters_applications_ingress, :installed, external_ip: '127.0.0.1') } let!(:jupyter) { create(:clusters_applications_jupyter, cluster: ingress.cluster) } diff --git a/spec/models/clusters/applications/knative_spec.rb b/spec/models/clusters/applications/knative_spec.rb index 35818be8deb..5519615d52d 100644 --- a/spec/models/clusters/applications/knative_spec.rb +++ b/spec/models/clusters/applications/knative_spec.rb @@ -9,6 +9,7 @@ describe Clusters::Applications::Knative do include_examples 'cluster application core specs', :clusters_applications_knative include_examples 'cluster application status specs', :clusters_applications_knative include_examples 'cluster application helm specs', :clusters_applications_knative + include_examples 'cluster application initial status specs' before do allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_in) @@ -34,20 +35,6 @@ describe Clusters::Applications::Knative do it { is_expected.to contain_exactly(cluster) } end - describe '#make_installing!' do - before do - application.make_installing! - end - - context 'application install previously errored with older version' do - let(:application) { create(:clusters_applications_knative, :scheduled, version: '0.2.2') } - - it 'updates the application version' do - expect(application.reload.version).to eq('0.2.2') - end - end - end - describe '#make_installed' do subject { described_class.installed } diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index e50ba67c493..073fbded8ac 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -6,6 +6,7 @@ describe Clusters::Applications::Prometheus do include_examples 'cluster application core specs', :clusters_applications_prometheus include_examples 'cluster application status specs', :clusters_applications_prometheus include_examples 'cluster application helm specs', :clusters_applications_prometheus + include_examples 'cluster application initial status specs' describe '.installed' do subject { described_class.installed } @@ -19,20 +20,6 @@ describe Clusters::Applications::Prometheus do it { is_expected.to contain_exactly(cluster) } end - describe '#make_installing!' do - before do - application.make_installing! - end - - context 'application install previously errored with older version' do - let(:application) { create(:clusters_applications_prometheus, :scheduled, version: '6.7.2') } - - it 'updates the application version' do - expect(application.reload.version).to eq('6.7.3') - end - end - end - describe 'transition to installed' do let(:project) { create(:project) } let(:cluster) { create(:cluster, :with_installed_helm, projects: [project]) } diff --git a/spec/models/clusters/applications/runner_spec.rb b/spec/models/clusters/applications/runner_spec.rb index 8ad41e997c2..96b7b02dbaf 100644 --- a/spec/models/clusters/applications/runner_spec.rb +++ b/spec/models/clusters/applications/runner_spec.rb @@ -6,23 +6,10 @@ describe Clusters::Applications::Runner do include_examples 'cluster application core specs', :clusters_applications_runner include_examples 'cluster application status specs', :clusters_applications_runner include_examples 'cluster application helm specs', :clusters_applications_runner + include_examples 'cluster application initial status specs' it { is_expected.to belong_to(:runner) } - describe '#make_installing!' do - before do - application.make_installing! - end - - context 'application install previously errored with older version' do - let(:application) { create(:clusters_applications_runner, :scheduled, version: '0.1.30') } - - it 'updates the application version' do - expect(application.reload.version).to eq('0.1.45') - end - end - end - describe '.installed' do subject { described_class.installed } diff --git a/spec/models/commit_collection_spec.rb b/spec/models/commit_collection_spec.rb index 005005b236b..12e59b35428 100644 --- a/spec/models/commit_collection_spec.rb +++ b/spec/models/commit_collection_spec.rb @@ -35,6 +35,17 @@ describe CommitCollection do end end + describe '#without_merge_commits' do + it 'returns all commits except merge commits' do + collection = described_class.new(project, [ + build(:commit), + build(:commit, :merge_commit) + ]) + + expect(collection.without_merge_commits.size).to eq(1) + end + end + describe '#with_pipeline_status' do it 'sets the pipeline status for every commit so no additional queries are necessary' do create( diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index b62f973ad1e..afa87b8a62d 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -82,6 +82,38 @@ describe MergeRequest do end end + describe '#default_squash_commit_message' do + let(:project) { subject.project } + + def commit_collection(commit_hashes) + raw_commits = commit_hashes.map { |raw| Commit.from_hash(raw, project) } + + CommitCollection.new(project, raw_commits) + end + + it 'returns the oldest multiline commit message' do + commits = commit_collection([ + { message: 'Singleline', parent_ids: [] }, + { message: "Second multiline\nCommit message", parent_ids: [] }, + { message: "First multiline\nCommit message", parent_ids: [] } + ]) + + expect(subject).to receive(:commits).and_return(commits) + + expect(subject.default_squash_commit_message).to eq("First multiline\nCommit message") + end + + it 'returns the merge request title if there are no multiline commits' do + commits = commit_collection([ + { message: 'Singleline', parent_ids: [] } + ]) + + expect(subject).to receive(:commits).and_return(commits) + + expect(subject.default_squash_commit_message).to eq(subject.title) + end + end + describe 'modules' do subject { described_class } @@ -920,18 +952,18 @@ describe MergeRequest do end end - describe '#merge_commit_message' do + describe '#default_merge_commit_message' do it 'includes merge information as the title' do request = build(:merge_request, source_branch: 'source', target_branch: 'target') - expect(request.merge_commit_message) + expect(request.default_merge_commit_message) .to match("Merge branch 'source' into 'target'\n\n") end it 'includes its title in the body' do request = build(:merge_request, title: 'Remove all technical debt') - expect(request.merge_commit_message) + expect(request.default_merge_commit_message) .to match("Remove all technical debt\n\n") end @@ -943,34 +975,34 @@ describe MergeRequest do allow(subject.project).to receive(:default_branch).and_return(subject.target_branch) subject.cache_merge_request_closes_issues! - expect(subject.merge_commit_message) + expect(subject.default_merge_commit_message) .to match("Closes #{issue.to_reference}") end it 'includes its reference in the body' do request = build_stubbed(:merge_request) - expect(request.merge_commit_message) + expect(request.default_merge_commit_message) .to match("See merge request #{request.to_reference(full: true)}") end it 'excludes multiple linebreak runs when description is blank' do request = build(:merge_request, title: 'Title', description: nil) - expect(request.merge_commit_message).not_to match("Title\n\n\n\n") + expect(request.default_merge_commit_message).not_to match("Title\n\n\n\n") end it 'includes its description in the body' do request = build(:merge_request, description: 'By removing all code') - expect(request.merge_commit_message(include_description: true)) + expect(request.default_merge_commit_message(include_description: true)) .to match("By removing all code\n\n") end it 'does not includes its description in the body' do request = build(:merge_request, description: 'By removing all code') - expect(request.merge_commit_message) + expect(request.default_merge_commit_message) .not_to match("By removing all code\n\n") end end diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb index 811e23fb854..1f317971a66 100644 --- a/spec/requests/api/releases_spec.rb +++ b/spec/requests/api/releases_spec.rb @@ -127,6 +127,31 @@ describe API::Releases do .to match_array(release.sources.map(&:url)) end + context "when release description contains confidential issue's link" do + let(:confidential_issue) do + create(:issue, + :confidential, + project: project, + title: 'A vulnerability') + end + + let!(:release) do + create(:release, + project: project, + tag: 'v0.1', + sha: commit.id, + author: maintainer, + description: "This is confidential #{confidential_issue.to_reference}") + end + + it "does not expose confidential issue's title" do + get api("/projects/#{project.id}/releases/v0.1", maintainer) + + expect(json_response['description_html']).to include(confidential_issue.to_reference) + expect(json_response['description_html']).not_to include('A vulnerability') + end + end + context 'when release has link asset' do let!(:link) do create(:release_link, diff --git a/spec/serializers/merge_request_widget_commit_entity_spec.rb b/spec/serializers/merge_request_widget_commit_entity_spec.rb new file mode 100644 index 00000000000..ce83978c49a --- /dev/null +++ b/spec/serializers/merge_request_widget_commit_entity_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe MergeRequestWidgetCommitEntity do + let(:project) { create(:project, :repository) } + let(:commit) { project.commit } + let(:request) { double('request') } + + let(:entity) do + described_class.new(commit, request: request) + end + + context 'as json' do + subject { entity.as_json } + + it { expect(subject[:message]).to eq(commit.safe_message) } + it { expect(subject[:short_id]).to eq(commit.short_id) } + it { expect(subject[:title]).to eq(commit.title) } + end +end diff --git a/spec/serializers/merge_request_widget_entity_spec.rb b/spec/serializers/merge_request_widget_entity_spec.rb index 376698a16df..4dbd79f2fc0 100644 --- a/spec/serializers/merge_request_widget_entity_spec.rb +++ b/spec/serializers/merge_request_widget_entity_spec.rb @@ -188,9 +188,14 @@ describe MergeRequestWidgetEntity do .to eq("/#{resource.project.full_path}/merge_requests/#{resource.iid}.diff") end - it 'has merge_commit_message_with_description' do - expect(subject[:merge_commit_message_with_description]) - .to eq(resource.merge_commit_message(include_description: true)) + it 'has default_merge_commit_message_with_description' do + expect(subject[:default_merge_commit_message_with_description]) + .to eq(resource.default_merge_commit_message(include_description: true)) + end + + it 'has default_squash_commit_message' do + expect(subject[:default_squash_commit_message]) + .to eq(resource.default_squash_commit_message) end describe 'new_blob_path' do @@ -272,4 +277,15 @@ describe MergeRequestWidgetEntity do expect(entity[:rebase_path]).to be_nil end end + + describe 'commits_without_merge_commits' do + it 'should not include merge commits' do + # Mock all but the first 5 commits to be merge commits + resource.commits.each_with_index do |commit, i| + expect(commit).to receive(:merge_commit?).at_least(:once).and_return(i > 4) + end + + expect(subject[:commits_without_merge_commits].size).to eq(5) + end + end end diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index 5d96b5ce27c..04a62aa454d 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -258,7 +258,7 @@ describe MergeRequests::MergeService do it 'logs and saves error if there is an error when squashing' do error_message = 'Failed to squash. Should be done manually' - allow_any_instance_of(MergeRequests::SquashService).to receive(:squash).and_return(nil) + allow_any_instance_of(MergeRequests::SquashService).to receive(:squash!).and_return(nil) merge_request.update(squash: true) service.execute(merge_request) diff --git a/spec/services/merge_requests/squash_service_spec.rb b/spec/services/merge_requests/squash_service_spec.rb index 53bce15735c..2713652873e 100644 --- a/spec/services/merge_requests/squash_service_spec.rb +++ b/spec/services/merge_requests/squash_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe MergeRequests::SquashService do include GitHelpers - let(:service) { described_class.new(project, user, {}) } + let(:service) { described_class.new(project, user, { merge_request: merge_request }) } let(:user) { project.owner } let(:project) { create(:project, :repository) } let(:repository) { project.repository.raw } @@ -31,32 +31,49 @@ describe MergeRequests::SquashService do shared_examples 'the squash succeeds' do it 'returns the squashed commit SHA' do - result = service.execute(merge_request) + result = service.execute expect(result).to match(status: :success, squash_sha: a_string_matching(/\h{40}/)) expect(result[:squash_sha]).not_to eq(merge_request.diff_head_sha) end it 'cleans up the temporary directory' do - service.execute(merge_request) + service.execute expect(File.exist?(squash_dir_path)).to be(false) end it 'does not keep the branch push event' do - expect { service.execute(merge_request) }.not_to change { Event.count } + expect { service.execute }.not_to change { Event.count } + end + + context 'when there is a single commit in the merge request' do + before do + expect(merge_request).to receive(:commits_count).at_least(:once).and_return(1) + end + + it 'will skip performing the squash, as the outcome would be the same' do + expect(merge_request.target_project.repository).not_to receive(:squash) + + service.execute + end + + it 'will still perform the squash when a custom squash commit message has been provided' do + service = described_class.new(project, user, { merge_request: merge_request, squash_commit_message: 'A custom commit message' }) + + expect(merge_request.target_project.repository).to receive(:squash).and_return('sha') + + service.execute + end end context 'the squashed commit' do - let(:squash_sha) { service.execute(merge_request)[:squash_sha] } + let(:squash_sha) { service.execute[:squash_sha] } let(:squash_commit) { project.repository.commit(squash_sha) } - it 'copies the author info and message from the merge request' do + it 'copies the author info from the merge request' do expect(squash_commit.author_name).to eq(merge_request.author.name) expect(squash_commit.author_email).to eq(merge_request.author.email) - - # Commit messages have a trailing newline, but titles don't. - expect(squash_commit.message.chomp).to eq(merge_request.title) end it 'sets the current user as the committer' do @@ -72,21 +89,37 @@ describe MergeRequests::SquashService do expect(squash_diff.patch.length).to eq(mr_diff.patch.length) expect(squash_commit.sha).not_to eq(merge_request.diff_head_sha) end + + it 'has a default squash commit message if no message was provided' do + expect(squash_commit.message.chomp).to eq(merge_request.default_squash_commit_message.chomp) + end + + context 'if a message was provided' do + let(:service) { described_class.new(project, user, { merge_request: merge_request, squash_commit_message: message }) } + let(:message) { 'My custom message' } + let(:squash_sha) { service.execute[:squash_sha] } + + it 'has the same message as the message provided' do + expect(squash_commit.message.chomp).to eq(message) + end + end end end describe '#execute' do context 'when there is only one commit in the merge request' do + let(:merge_request) { merge_request_with_one_commit } + it 'returns that commit SHA' do - result = service.execute(merge_request_with_one_commit) + result = service.execute - expect(result).to match(status: :success, squash_sha: merge_request_with_one_commit.diff_head_sha) + expect(result).to match(status: :success, squash_sha: merge_request.diff_head_sha) end it 'does not perform any git actions' do expect(repository).not_to receive(:popen) - service.execute(merge_request_with_one_commit) + service.execute end end @@ -116,12 +149,11 @@ describe MergeRequests::SquashService do expect(service).to receive(:log_error).with(log_error) expect(service).to receive(:log_error).with(error) - service.execute(merge_request) + service.execute end it 'returns an error' do - expect(service.execute(merge_request)).to match(status: :error, - message: a_string_including('squash')) + expect(service.execute).to match(status: :error, message: a_string_including('squash')) end end end @@ -131,23 +163,22 @@ describe MergeRequests::SquashService do let(:error) { 'A test error' } before do - allow(merge_request).to receive(:commits_count).and_raise(error) + allow(merge_request.target_project.repository).to receive(:squash).and_raise(error) end it 'logs the MR reference and exception' do expect(service).to receive(:log_error).with(a_string_including("#{project.full_path}#{merge_request.to_reference}")) expect(service).to receive(:log_error).with(error) - service.execute(merge_request) + service.execute end it 'returns an error' do - expect(service.execute(merge_request)).to match(status: :error, - message: a_string_including('squash')) + expect(service.execute).to match(status: :error, message: a_string_including('squash')) end it 'cleans up the temporary directory' do - service.execute(merge_request) + service.execute expect(File.exist?(squash_dir_path)).to be(false) end diff --git a/spec/support/helpers/features/responsive_table_helpers.rb b/spec/support/helpers/features/responsive_table_helpers.rb new file mode 100644 index 00000000000..7a175219fe9 --- /dev/null +++ b/spec/support/helpers/features/responsive_table_helpers.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true +# These helpers allow you to access rows in a responsive table +# +# Usage: +# describe "..." do +# include Spec::Support::Helpers::Features::ResponsiveTableHelpers +# ... +# +# expect(first_row.text).to include("John Doe") +# expect(second_row.text).to include("John Smith") +# +# Note: +# index starts at 1 as index 0 is expected to be the table header +# +# +module Spec + module Support + module Helpers + module Features + module ResponsiveTableHelpers + def first_row + page.all('.gl-responsive-table-row')[1] + end + + def second_row + page.all('.gl-responsive-table-row')[2] + end + end + end + end + end +end diff --git a/spec/support/shared_examples/models/cluster_application_initial_status.rb b/spec/support/shared_examples/models/cluster_application_initial_status.rb new file mode 100644 index 00000000000..9775d87953c --- /dev/null +++ b/spec/support/shared_examples/models/cluster_application_initial_status.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +shared_examples 'cluster application initial status specs' do + describe '#status' do + let(:cluster) { create(:cluster, :provided_by_gcp) } + + subject { described_class.new(cluster: cluster) } + + context 'when application helm is scheduled' do + before do + create(:clusters_applications_helm, :scheduled, cluster: cluster) + end + + it 'defaults to :not_installable' do + expect(subject.status_name).to be(:not_installable) + end + end + + context 'when application is scheduled' do + before do + create(:clusters_applications_helm, :installed, cluster: cluster) + end + + it 'sets a default status' do + expect(subject.status_name).to be(:installable) + end + end + end +end diff --git a/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb b/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb index c391cc48f4e..554f2e747bc 100644 --- a/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb +++ b/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb @@ -7,26 +7,6 @@ shared_examples 'cluster application status specs' do |application_name| it 'sets a default status' do expect(subject.status_name).to be(:not_installable) end - - context 'when application helm is scheduled' do - before do - create(:clusters_applications_helm, :scheduled, cluster: cluster) - end - - it 'defaults to :not_installable' do - expect(subject.status_name).to be(:not_installable) - end - end - - context 'when application is scheduled' do - before do - create(:clusters_applications_helm, :installed, cluster: cluster) - end - - it 'sets a default status' do - expect(subject.status_name).to be(:installable) - end - end end describe 'status state machine' do @@ -58,6 +38,16 @@ shared_examples 'cluster application status specs' do |application_name| expect(subject.cluster.application_helm.version).to eq(Gitlab::Kubernetes::Helm::HELM_VERSION) end + + it 'sets the correct version of the application' do + subject.update!(version: '0.0.0') + + subject.make_installed! + + subject.reload + + expect(subject.version).to eq(subject.class.const_get(:VERSION)) + end end describe '#make_updated' do @@ -78,6 +68,16 @@ shared_examples 'cluster application status specs' do |application_name| expect(subject.cluster.application_helm.version).to eq(Gitlab::Kubernetes::Helm::HELM_VERSION) end + + it 'updates the version for the application' do + subject.update!(version: '0.0.0') + + subject.make_updated! + + subject.reload + + expect(subject.version).to eq(subject.class.const_get(:VERSION)) + end end describe '#make_errored' do |