diff options
43 files changed, 924 insertions, 175 deletions
diff --git a/app/assets/javascripts/jobs/components/sidebar_details_block.vue b/app/assets/javascripts/jobs/components/sidebar_details_block.vue index d2adf628050..36d4a3e2bc9 100644 --- a/app/assets/javascripts/jobs/components/sidebar_details_block.vue +++ b/app/assets/javascripts/jobs/components/sidebar_details_block.vue @@ -1,14 +1,16 @@ <script> -import detailRow from './sidebar_detail_row.vue'; -import loadingIcon from '../../vue_shared/components/loading_icon.vue'; -import timeagoMixin from '../../vue_shared/mixins/timeago'; -import { timeIntervalInWords } from '../../lib/utils/datetime_utility'; +import LoadingIcon from '~/vue_shared/components/loading_icon.vue'; +import timeagoMixin from '~/vue_shared/mixins/timeago'; +import { timeIntervalInWords } from '~/lib/utils/datetime_utility'; +import Icon from '~/vue_shared/components/icon.vue'; +import DetailRow from './sidebar_detail_row.vue'; export default { name: 'SidebarDetailsBlock', components: { - detailRow, - loadingIcon, + DetailRow, + LoadingIcon, + Icon, }, mixins: [timeagoMixin], props: { @@ -20,16 +22,16 @@ export default { type: Boolean, required: true, }, - canUserRetry: { - type: Boolean, - required: false, - default: false, - }, runnerHelpUrl: { type: String, required: false, default: '', }, + terminalPath: { + type: String, + required: false, + default: null, + }, }, computed: { shouldRenderContent() { @@ -92,7 +94,7 @@ export default { {{ job.name }} </strong> <a - v-if="canUserRetry" + v-if="job.retry_path" :class="retryButtonClass" :href="job.retry_path" data-method="post" @@ -100,6 +102,16 @@ export default { > {{ __('Retry') }} </a> + <a + v-if="terminalPath" + :href="terminalPath" + class="js-terminal-link pull-right btn btn-primary + btn-inverted visible-md-block visible-lg-block" + target="_blank" + > + {{ __('Debug') }} + <icon name="external-link" /> + </a> <button :aria-label="__('Toggle Sidebar')" type="button" @@ -125,7 +137,7 @@ export default { {{ __('New issue') }} </a> <a - v-if="canUserRetry" + v-if="job.retry_path" :href="job.retry_path" class="js-retry-job btn btn-inverted-secondary" data-method="post" diff --git a/app/assets/javascripts/jobs/job_details_bundle.js b/app/assets/javascripts/jobs/job_details_bundle.js index 0db7b95636c..a84324f14b2 100644 --- a/app/assets/javascripts/jobs/job_details_bundle.js +++ b/app/assets/javascripts/jobs/job_details_bundle.js @@ -52,9 +52,9 @@ export default () => { return createElement('details-block', { props: { isLoading: this.mediator.state.isLoading, - canUserRetry: !!('canUserRetry' in detailsBlockDataset), job: this.mediator.store.state.job, runnerHelpUrl: dataset.runnerHelpUrl, + terminalPath: detailsBlockDataset.terminalPath, }, }); }, diff --git a/app/controllers/projects/autocomplete_sources_controller.rb b/app/controllers/projects/autocomplete_sources_controller.rb index 07627ffb69f..a8f73ed5cb0 100644 --- a/app/controllers/projects/autocomplete_sources_controller.rb +++ b/app/controllers/projects/autocomplete_sources_controller.rb @@ -32,13 +32,8 @@ class Projects::AutocompleteSourcesController < Projects::ApplicationController end def target - case params[:type]&.downcase - when 'issue' - IssuesFinder.new(current_user, project_id: @project.id).find_by(iid: params[:type_id]) - when 'mergerequest' - MergeRequestsFinder.new(current_user, project_id: @project.id).find_by(iid: params[:type_id]) - when 'commit' - @project.commit(params[:type_id]) - end + QuickActions::TargetService + .new(project, current_user) + .execute(params[:type], params[:type_id]) end end diff --git a/app/models/commit.rb b/app/models/commit.rb index 8b9f4490ffa..27fbdc3e386 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -448,6 +448,10 @@ class Commit true end + def to_ability_name + model_name.singular + end + def touch # no-op but needs to be defined since #persisted? is defined end diff --git a/app/models/system_note_metadata.rb b/app/models/system_note_metadata.rb index c5c77bc8333..376ef673ca8 100644 --- a/app/models/system_note_metadata.rb +++ b/app/models/system_note_metadata.rb @@ -15,7 +15,7 @@ class SystemNoteMetadata < ActiveRecord::Base commit description merge confidential visible label assignee cross_reference title time_tracking branch milestone discussion task moved opened closed merged duplicate locked unlocked - outdated + outdated tag ].freeze validates :note, presence: true diff --git a/app/models/user.rb b/app/models/user.rb index 37f2e8b680e..fb19de4b980 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -101,6 +101,10 @@ class User < ActiveRecord::Base has_many :groups, through: :group_members has_many :owned_groups, -> { where(members: { access_level: Gitlab::Access::OWNER }) }, through: :group_members, source: :group has_many :maintainers_groups, -> { where(members: { access_level: Gitlab::Access::MAINTAINER }) }, through: :group_members, source: :group + has_many :owned_or_maintainers_groups, + -> { where(members: { access_level: [Gitlab::Access::MAINTAINER, Gitlab::Access::OWNER] }) }, + through: :group_members, + source: :group alias_attribute :masters_groups, :maintainers_groups # Projects @@ -982,15 +986,7 @@ class User < ActiveRecord::Base end def manageable_groups - union_sql = Gitlab::SQL::Union.new([owned_groups.select(:id), maintainers_groups.select(:id)]).to_sql - - # Update this line to not use raw SQL when migrated to Rails 5.2. - # Either ActiveRecord or Arel constructions are fine. - # This was replaced with the raw SQL construction because of bugs in the arel gem. - # Bugs were fixed in arel 9.0.0 (Rails 5.2). - owned_and_maintainer_groups = Group.where("namespaces.id IN (#{union_sql})") # rubocop:disable GitlabSecurity/SqlInjection - - Gitlab::GroupHierarchy.new(owned_and_maintainer_groups).base_and_descendants + Gitlab::GroupHierarchy.new(owned_or_maintainers_groups).base_and_descendants end def namespaces @@ -1244,11 +1240,6 @@ class User < ActiveRecord::Base !terms_accepted? end - def owned_or_maintainers_groups - union = Gitlab::SQL::Union.new([owned_groups, maintainers_groups]) - Group.from("(#{union.to_sql}) namespaces") - end - # @deprecated alias_method :owned_or_masters_groups, :owned_or_maintainers_groups diff --git a/app/policies/commit_policy.rb b/app/policies/commit_policy.rb new file mode 100644 index 00000000000..67e9bc12804 --- /dev/null +++ b/app/policies/commit_policy.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class CommitPolicy < BasePolicy + delegate { @subject.project } +end diff --git a/app/services/commits/tag_service.rb b/app/services/commits/tag_service.rb new file mode 100644 index 00000000000..7961ba4d3c4 --- /dev/null +++ b/app/services/commits/tag_service.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module Commits + class TagService < BaseService + def execute(commit) + unless params[:tag_name] + return error('Missing parameter tag_name') + end + + tag_name = params[:tag_name] + message = params[:tag_message] + release_description = nil + + result = Tags::CreateService + .new(commit.project, current_user) + .execute(tag_name, commit.sha, message, release_description) + + if result[:status] == :success + tag = result[:tag] + SystemNoteService.tag_commit(commit, commit.project, current_user, tag.name) + end + + result + end + end +end diff --git a/app/services/notes/quick_actions_service.rb b/app/services/notes/quick_actions_service.rb index 7280449bb1c..4c14d834949 100644 --- a/app/services/notes/quick_actions_service.rb +++ b/app/services/notes/quick_actions_service.rb @@ -4,7 +4,8 @@ module Notes class QuickActionsService < BaseService UPDATE_SERVICES = { 'Issue' => Issues::UpdateService, - 'MergeRequest' => MergeRequests::UpdateService + 'MergeRequest' => MergeRequests::UpdateService, + 'Commit' => Commits::TagService }.freeze def self.noteable_update_service(note) diff --git a/app/services/preview_markdown_service.rb b/app/services/preview_markdown_service.rb index a15ee4911ef..11b996ed4b6 100644 --- a/app/services/preview_markdown_service.rb +++ b/app/services/preview_markdown_service.rb @@ -16,7 +16,7 @@ class PreviewMarkdownService < BaseService private def explain_quick_actions(text) - return text, [] unless %w(Issue MergeRequest).include?(commands_target_type) + return text, [] unless %w(Issue MergeRequest Commit).include?(commands_target_type) quick_actions_service = QuickActions::InterpretService.new(project, current_user) quick_actions_service.explain(text, find_commands_target) @@ -29,13 +29,9 @@ class PreviewMarkdownService < BaseService end def find_commands_target - if commands_target_id.present? - finder = commands_target_type == 'Issue' ? IssuesFinder : MergeRequestsFinder - finder.new(current_user, project_id: project.id).find(commands_target_id) - else - collection = commands_target_type == 'Issue' ? project.issues : project.merge_requests - collection.build - end + QuickActions::TargetService + .new(project, current_user) + .execute(commands_target_type, commands_target_id) end def commands_target_type diff --git a/app/services/projects/autocomplete_service.rb b/app/services/projects/autocomplete_service.rb index 10eb2cea4a2..5286b92ab6b 100644 --- a/app/services/projects/autocomplete_service.rb +++ b/app/services/projects/autocomplete_service.rb @@ -47,15 +47,7 @@ module Projects end def commands(noteable, type) - noteable ||= - case type - when 'Issue' - @project.issues.build - when 'MergeRequest' - @project.merge_requests.build - end - - return [] unless noteable&.is_a?(Issuable) + return [] unless noteable QuickActions::InterpretService.new(project, current_user).available_commands(noteable) end diff --git a/app/services/quick_actions/interpret_service.rb b/app/services/quick_actions/interpret_service.rb index cdc8514c47c..8838ed06324 100644 --- a/app/services/quick_actions/interpret_service.rb +++ b/app/services/quick_actions/interpret_service.rb @@ -60,7 +60,8 @@ module QuickActions "Closes this #{issuable.to_ability_name.humanize(capitalize: false)}." end condition do - issuable.persisted? && + issuable.is_a?(Issuable) && + issuable.persisted? && issuable.open? && current_user.can?(:"update_#{issuable.to_ability_name}", issuable) end @@ -75,7 +76,8 @@ module QuickActions "Reopens this #{issuable.to_ability_name.humanize(capitalize: false)}." end condition do - issuable.persisted? && + issuable.is_a?(Issuable) && + issuable.persisted? && issuable.closed? && current_user.can?(:"update_#{issuable.to_ability_name}", issuable) end @@ -149,7 +151,8 @@ module QuickActions issuable.allows_multiple_assignees? ? '@user1 @user2' : '' end condition do - issuable.persisted? && + issuable.is_a?(Issuable) && + issuable.persisted? && issuable.assignees.any? && current_user.can?(:"admin_#{issuable.to_ability_name}", project) end @@ -188,7 +191,8 @@ module QuickActions "Removes #{issuable.milestone.to_reference(format: :name)} milestone." end condition do - issuable.persisted? && + issuable.is_a?(Issuable) && + issuable.persisted? && issuable.milestone_id? && current_user.can?(:"admin_#{issuable.to_ability_name}", project) end @@ -231,7 +235,8 @@ module QuickActions end params '~label1 ~"label 2"' condition do - issuable.persisted? && + issuable.is_a?(Issuable) && + issuable.persisted? && issuable.labels.any? && current_user.can?(:"admin_#{issuable.to_ability_name}", project) end @@ -257,7 +262,8 @@ module QuickActions end params '~label1 ~"label 2"' condition do - issuable.persisted? && + issuable.is_a?(Issuable) && + issuable.persisted? && issuable.labels.any? && current_user.can?(:"admin_#{issuable.to_ability_name}", project) end @@ -295,7 +301,8 @@ module QuickActions desc 'Add a todo' explanation 'Adds a todo.' condition do - issuable.persisted? && + issuable.is_a?(Issuable) && + issuable.persisted? && !TodoService.new.todo_exist?(issuable, current_user) end command :todo do @@ -317,7 +324,8 @@ module QuickActions "Subscribes to this #{issuable.to_ability_name.humanize(capitalize: false)}." end condition do - issuable.persisted? && + issuable.is_a?(Issuable) && + issuable.persisted? && !issuable.subscribed?(current_user, project) end command :subscribe do @@ -329,7 +337,8 @@ module QuickActions "Unsubscribes from this #{issuable.to_ability_name.humanize(capitalize: false)}." end condition do - issuable.persisted? && + issuable.is_a?(Issuable) && + issuable.persisted? && issuable.subscribed?(current_user, project) end command :unsubscribe do @@ -385,7 +394,8 @@ module QuickActions end params ':emoji:' condition do - issuable.persisted? + issuable.is_a?(Issuable) && + issuable.persisted? end parse_params do |emoji_param| match = emoji_param.match(Banzai::Filter::EmojiFilter.emoji_pattern) @@ -574,6 +584,23 @@ module QuickActions @updates[:confidential] = true end + desc 'Tag this commit.' + explanation do |tag_name, message| + with_message = %{ with "#{message}"} if message.present? + "Tags this commit to #{tag_name}#{with_message}." + end + params 'v1.2.3 <message>' + parse_params do |tag_name_and_message| + tag_name_and_message.split(' ', 2) + end + condition do + issuable.is_a?(Commit) && current_user.can?(:push_code, project) + end + command :tag do |tag_name, message| + @updates[:tag_name] = tag_name + @updates[:tag_message] = message + end + def extract_users(params) return [] if params.nil? diff --git a/app/services/quick_actions/target_service.rb b/app/services/quick_actions/target_service.rb new file mode 100644 index 00000000000..d8ba52c6e50 --- /dev/null +++ b/app/services/quick_actions/target_service.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module QuickActions + class TargetService < BaseService + def execute(type, type_id) + case type&.downcase + when 'issue' + issue(type_id) + when 'mergerequest' + merge_request(type_id) + when 'commit' + commit(type_id) + end + end + + private + + def issue(type_id) + IssuesFinder.new(current_user, project_id: project.id).find_by(iid: type_id) || project.issues.build + end + + def merge_request(type_id) + MergeRequestsFinder.new(current_user, project_id: project.id).find_by(iid: type_id) || project.merge_requests.build + end + + def commit(type_id) + project.commit(type_id) + end + end +end diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index 77494295f14..dda89830179 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -32,6 +32,21 @@ module SystemNoteService create_note(NoteSummary.new(noteable, project, author, body, action: 'commit', commit_count: total_count)) end + # Called when a commit was tagged + # + # noteable - Noteable object + # project - Project owning noteable + # author - User performing the tag + # tag_name - The created tag name + # + # Returns the created Note object + def tag_commit(noteable, project, author, tag_name) + link = url_helpers.project_tag_url(project, id: tag_name) + body = "tagged commit #{noteable.sha} to [`#{tag_name}`](#{link})" + + create_note(NoteSummary.new(noteable, project, author, body, action: 'tag')) + end + # Called when the assignee of a Noteable is changed or removed # # noteable - Noteable object diff --git a/app/services/tags/create_service.rb b/app/services/tags/create_service.rb index 329722df747..35390f5082c 100644 --- a/app/services/tags/create_service.rb +++ b/app/services/tags/create_service.rb @@ -7,7 +7,7 @@ module Tags return error('Tag name invalid') unless valid_tag repository = project.repository - message&.strip! + message = message&.strip new_tag = nil diff --git a/app/views/projects/jobs/_sidebar.html.haml b/app/views/projects/jobs/_sidebar.html.haml index 759efd4e9d4..86b2b8bf2f7 100644 --- a/app/views/projects/jobs/_sidebar.html.haml +++ b/app/views/projects/jobs/_sidebar.html.haml @@ -1,12 +1,7 @@ %aside.right-sidebar.right-sidebar-expanded.build-sidebar.js-build-sidebar.js-right-sidebar{ data: { "offset-top" => "101", "spy" => "affix" } } .sidebar-container .blocks-container - - if can?(current_user, :create_build_terminal, @build) - .block - = link_to terminal_project_job_path(@project, @build), class: 'terminal-button pull-right btn visible-md-block visible-lg-block', title: 'Terminal' do - Terminal - - #js-details-block-vue{ data: { can_user_retry: can?(current_user, :update_build, @build) && @build.retryable? } } + #js-details-block-vue{ data: { terminal_path: can?(current_user, :create_build_terminal, @build) && @build.has_terminal? ? terminal_project_job_path(@project, @build) : nil } } - if can?(current_user, :read_build, @project) && (@build.artifacts? || @build.artifacts_expired?) .block diff --git a/changelogs/unreleased/25990-web-terminal-improvements.yml b/changelogs/unreleased/25990-web-terminal-improvements.yml new file mode 100644 index 00000000000..99a4a82ea66 --- /dev/null +++ b/changelogs/unreleased/25990-web-terminal-improvements.yml @@ -0,0 +1,5 @@ +--- +title: Make terminal button more visible +merge_request: +author: +type: changed diff --git a/changelogs/unreleased/45663-tag-quick-action-on-commit-comments.yml b/changelogs/unreleased/45663-tag-quick-action-on-commit-comments.yml new file mode 100644 index 00000000000..6d664511e6e --- /dev/null +++ b/changelogs/unreleased/45663-tag-quick-action-on-commit-comments.yml @@ -0,0 +1,5 @@ +--- +title: "`/tag` quick action on Commit comments" +merge_request: 20694 +author: Peter Leitzen +type: added diff --git a/changelogs/unreleased/bvl-merge-base-api.yml b/changelogs/unreleased/bvl-merge-base-api.yml new file mode 100644 index 00000000000..78fb3ce0897 --- /dev/null +++ b/changelogs/unreleased/bvl-merge-base-api.yml @@ -0,0 +1,5 @@ +--- +title: Get the merge base of two refs through the API +merge_request: 20929 +author: +type: added diff --git a/changelogs/unreleased/ce-5666-optimize_querying_manageable_groups.yml b/changelogs/unreleased/ce-5666-optimize_querying_manageable_groups.yml new file mode 100644 index 00000000000..0c6a1071cdd --- /dev/null +++ b/changelogs/unreleased/ce-5666-optimize_querying_manageable_groups.yml @@ -0,0 +1,5 @@ +--- +title: Optimize querying User#manageable_groups +merge_request: 21050 +author: +type: performance diff --git a/doc/api/repositories.md b/doc/api/repositories.md index cb816bbd712..a4fdeca162e 100644 --- a/doc/api/repositories.md +++ b/doc/api/repositories.md @@ -204,3 +204,39 @@ Response: "deletions": 244 }] ``` + +## Merge Base + +Get the common ancestor for 2 refs (commit SHAs, branch names or tags). + +``` +GET /projects/:id/repository/merge_base +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | +| `refs` | array | yes | The refs to find the common ancestor of, for now only 2 refs are supported | + +```bash +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/5/repository/merge_base?refs[]=304d257dcb821665ab5110318fc58a007bd104ed&refs[]=0031876facac3f2b2702a0e53a26e89939a42209" +``` + +Example response: + +```json +{ + "id": "1a0b36b3cdad1d2ee32457c102a8c0b7056fa863", + "short_id": "1a0b36b3", + "title": "Initial commit", + "created_at": "2014-02-27T08:03:18.000Z", + "parent_ids": [], + "message": "Initial commit\n", + "author_name": "Dmitriy Zaporozhets", + "author_email": "dmitriy.zaporozhets@gmail.com", + "authored_date": "2014-02-27T08:03:18.000Z", + "committer_name": "Dmitriy Zaporozhets", + "committer_email": "dmitriy.zaporozhets@gmail.com", + "committed_date": "2014-02-27T08:03:18.000Z" +} +``` diff --git a/doc/user/project/quick_actions.md b/doc/user/project/quick_actions.md index 0ef8eddad20..8fdfd2a6f4d 100644 --- a/doc/user/project/quick_actions.md +++ b/doc/user/project/quick_actions.md @@ -1,7 +1,7 @@ # GitLab quick actions -Quick actions are textual shortcuts for common actions on issues or merge -requests that are usually done by clicking buttons or dropdowns in GitLab's UI. +Quick actions are textual shortcuts for common actions on issues, merge requests +or commits that are usually done by clicking buttons or dropdowns in GitLab's UI. You can enter these commands while creating a new issue or merge request, and in comments. Each command should be on a separate line in order to be properly detected and executed. The commands are removed from the issue, merge request or @@ -39,7 +39,8 @@ do. | `/board_move ~column` | Move issue to column on the board | | `/duplicate #issue` | Closes this issue and marks it as a duplicate of another issue | | `/move path/to/project` | Moves issue to another project | +| `/tag v1.2.3 <message>` | Tags a commit with a given tag name and optional message | | `/tableflip` | Append the comment with `(╯°□°)╯︵ ┻━┻` | | `/shrug` | Append the comment with `¯\＿(ツ)＿/¯` | | <code>/copy_metadata #issue | !merge_request</code> | Copy labels and milestone from other issue or merge request | -| `/confidential` | Makes the issue confidential |
\ No newline at end of file +| `/confidential` | Makes the issue confidential | diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb index 33a9646ac3b..79736107bbb 100644 --- a/lib/api/repositories.rb +++ b/lib/api/repositories.rb @@ -123,6 +123,39 @@ module API not_found! end end + + desc 'Get the common ancestor between commits' do + success Entities::Commit + end + params do + # For now we just support 2 refs passed, but `merge-base` supports + # multiple defining this as an Array instead of 2 separate params will + # make sure we don't need to deprecate this API in favor of one + # supporting multiple commits when this functionality gets added to + # Gitaly + requires :refs, type: Array[String] + end + get ':id/repository/merge_base' do + refs = params[:refs] + + unless refs.size == 2 + render_api_error!('Provide exactly 2 refs', 400) + end + + merge_base = Gitlab::Git::MergeBase.new(user_project.repository, refs) + + if merge_base.unknown_refs.any? + ref_noun = 'ref'.pluralize(merge_base.unknown_refs.size) + message = "Could not find #{ref_noun}: #{merge_base.unknown_refs.join(', ')}" + render_api_error!(message, 400) + end + + if merge_base.commit + present merge_base.commit, with: Entities::Commit + else + not_found!("Merge Base") + end + end end end end diff --git a/lib/gitlab/git.rb b/lib/gitlab/git.rb index 55236a1122f..2913a3e416d 100644 --- a/lib/gitlab/git.rb +++ b/lib/gitlab/git.rb @@ -10,9 +10,11 @@ module Gitlab TAG_REF_PREFIX = "refs/tags/".freeze BRANCH_REF_PREFIX = "refs/heads/".freeze - CommandError = Class.new(StandardError) - CommitError = Class.new(StandardError) - OSError = Class.new(StandardError) + BaseError = Class.new(StandardError) + CommandError = Class.new(BaseError) + CommitError = Class.new(BaseError) + OSError = Class.new(BaseError) + UnknownRef = Class.new(BaseError) class << self include Gitlab::EncodingHelper diff --git a/lib/gitlab/git/merge_base.rb b/lib/gitlab/git/merge_base.rb new file mode 100644 index 00000000000..b27f7038c26 --- /dev/null +++ b/lib/gitlab/git/merge_base.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +module Gitlab + module Git + class MergeBase + include Gitlab::Utils::StrongMemoize + + def initialize(repository, refs) + @repository, @refs = repository, refs + end + + # Returns the SHA of the first common ancestor + def sha + if unknown_refs.any? + raise UnknownRef, "Can't find merge base for unknown refs: #{unknown_refs.inspect}" + end + + strong_memoize(:sha) do + @repository.merge_base(*commits_for_refs) + end + end + + # Returns the merge base as a Gitlab::Git::Commit + def commit + return unless sha + + @commit ||= @repository.commit_by(oid: sha) + end + + # Returns the refs passed on initialization that aren't found in + # the repository, and thus cannot be used to find a merge base. + def unknown_refs + @unknown_refs ||= Hash[@refs.zip(commits_for_refs)] + .select { |ref, commit| commit.nil? }.keys + end + + private + + def commits_for_refs + @commits_for_refs ||= @repository.commits_by(oids: @refs) + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 92f424ccdf0..e8bf7ae8f0e 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1993,6 +1993,9 @@ msgstr "" msgid "DashboardProjects|Personal" msgstr "" +msgid "Debug" +msgstr "" + msgid "Dec" msgstr "" diff --git a/spec/features/commits/user_uses_slash_commands_spec.rb b/spec/features/commits/user_uses_slash_commands_spec.rb new file mode 100644 index 00000000000..9a4b7bd2444 --- /dev/null +++ b/spec/features/commits/user_uses_slash_commands_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe 'Commit > User uses quick actions', :js do + include Spec::Support::Helpers::Features::NotesHelpers + include RepoHelpers + + let(:project) { create(:project, :public, :repository) } + let(:user) { project.creator } + let(:commit) { project.commit } + + before do + project.add_maintainer(user) + sign_in(user) + + visit project_commit_path(project, commit.id) + end + + describe 'Tagging a commit' do + let(:tag_name) { 'v1.2.3' } + let(:tag_message) { 'Stable release' } + let(:truncated_commit_sha) { Commit.truncate_sha(commit.sha) } + + it 'tags this commit' do + add_note("/tag #{tag_name} #{tag_message}") + + expect(page).to have_content 'Commands applied' + expect(page).to have_content "tagged commit #{truncated_commit_sha}" + expect(page).to have_content tag_name + + visit project_tag_path(project, tag_name) + expect(page).to have_content tag_name + expect(page).to have_content tag_message + expect(page).to have_content truncated_commit_sha + end + + describe 'preview', :js do + it 'removes quick action from note and explains it' do + preview_note("/tag #{tag_name} #{tag_message}") + + expect(page).not_to have_content '/tag' + expect(page).to have_content %{Tags this commit to #{tag_name} with "#{tag_message}"} + expect(page).to have_content tag_name + end + end + end +end diff --git a/spec/features/projects/blobs/shortcuts_blob_spec.rb b/spec/features/projects/blobs/shortcuts_blob_spec.rb index 7203c5b1c21..3925de6cfb9 100644 --- a/spec/features/projects/blobs/shortcuts_blob_spec.rb +++ b/spec/features/projects/blobs/shortcuts_blob_spec.rb @@ -18,6 +18,7 @@ describe 'Blob shortcuts', :js do describe 'pressing "y"' do it 'redirects to permalink with commit sha' do visit_blob + wait_for_requests find('body').native.send_key('y') @@ -27,6 +28,7 @@ describe 'Blob shortcuts', :js do it 'maintains fragment hash when redirecting' do fragment = "L1" visit_blob(fragment) + wait_for_requests find('body').native.send_key('y') diff --git a/spec/features/projects/files/user_browses_files_spec.rb b/spec/features/projects/files/user_browses_files_spec.rb index 612722eeaad..f3cf3a282e5 100644 --- a/spec/features/projects/files/user_browses_files_spec.rb +++ b/spec/features/projects/files/user_browses_files_spec.rb @@ -213,6 +213,7 @@ describe "User browses files" do context "when browsing a file content", :js do before do visit(tree_path_root_ref) + wait_for_requests click_link(".gitignore") end diff --git a/spec/features/projects/files/user_deletes_files_spec.rb b/spec/features/projects/files/user_deletes_files_spec.rb index 5d37877ccb3..dcb7b947c61 100644 --- a/spec/features/projects/files/user_deletes_files_spec.rb +++ b/spec/features/projects/files/user_deletes_files_spec.rb @@ -19,6 +19,7 @@ describe 'Projects > Files > User deletes files', :js do before do project.add_maintainer(user) visit(project_tree_path_root_ref) + wait_for_requests end it 'deletes the file', :js do @@ -35,10 +36,11 @@ describe 'Projects > Files > User deletes files', :js do end end - context 'when an user does not have write access' do + context 'when an user does not have write access', :js do before do project2.add_reporter(user) visit(project2_tree_path_root_ref) + wait_for_requests end it 'deletes the file in a forked project', :js do diff --git a/spec/features/projects/files/user_edits_files_spec.rb b/spec/features/projects/files/user_edits_files_spec.rb index 072dc5820c4..9eb65ec159c 100644 --- a/spec/features/projects/files/user_edits_files_spec.rb +++ b/spec/features/projects/files/user_edits_files_spec.rb @@ -29,13 +29,14 @@ describe 'Projects > Files > User edits files', :js do end end - context 'when an user has write access' do + context 'when an user has write access', :js do before do project.add_maintainer(user) visit(project_tree_path_root_ref) + wait_for_requests end - it 'inserts a content of a file', :js do + it 'inserts a content of a file' do click_link('.gitignore') find('.js-edit-blob').click find('.file-editor', match: :first) @@ -49,13 +50,14 @@ describe 'Projects > Files > User edits files', :js do it 'does not show the edit link if a file is binary' do binary_file = File.join(project.repository.root_ref, 'files/images/logo-black.png') visit(project_blob_path(project, binary_file)) + wait_for_requests page.within '.content' do expect(page).not_to have_link('edit') end end - it 'commits an edited file', :js do + it 'commits an edited file' do click_link('.gitignore') find('.js-edit-blob').click find('.file-editor', match: :first) @@ -72,7 +74,7 @@ describe 'Projects > Files > User edits files', :js do expect(page).to have_content('*.rbca') end - it 'commits an edited file to a new branch', :js do + it 'commits an edited file to a new branch' do click_link('.gitignore') find('.js-edit-blob').click @@ -91,7 +93,7 @@ describe 'Projects > Files > User edits files', :js do expect(page).to have_content('*.rbca') end - it 'shows the diff of an edited file', :js do + it 'shows the diff of an edited file' do click_link('.gitignore') find('.js-edit-blob').click find('.file-editor', match: :first) @@ -106,13 +108,14 @@ describe 'Projects > Files > User edits files', :js do it_behaves_like 'unavailable for an archived project' end - context 'when an user does not have write access' do + context 'when an user does not have write access', :js do before do project2.add_reporter(user) visit(project2_tree_path_root_ref) + wait_for_requests end - it 'inserts a content of a file in a forked project', :js do + it 'inserts a content of a file in a forked project' do click_link('.gitignore') find('.js-edit-blob').click @@ -134,7 +137,7 @@ describe 'Projects > Files > User edits files', :js do expect(evaluate_script('ace.edit("editor").getValue()')).to eq('*.rbca') end - it 'commits an edited file in a forked project', :js do + it 'commits an edited file in a forked project' do click_link('.gitignore') find('.js-edit-blob').click @@ -163,6 +166,7 @@ describe 'Projects > Files > User edits files', :js do let!(:forked_project) { fork_project(project2, user, namespace: user.namespace, repository: true) } before do visit(project2_tree_path_root_ref) + wait_for_requests end it 'links to the forked project for editing' do diff --git a/spec/features/projects/files/user_replaces_files_spec.rb b/spec/features/projects/files/user_replaces_files_spec.rb index 3f973338305..e3da28d73c3 100644 --- a/spec/features/projects/files/user_replaces_files_spec.rb +++ b/spec/features/projects/files/user_replaces_files_spec.rb @@ -21,9 +21,10 @@ describe 'Projects > Files > User replaces files', :js do before do project.add_maintainer(user) visit(project_tree_path_root_ref) + wait_for_requests end - it 'replaces an existed file with a new one', :js do + it 'replaces an existed file with a new one' do click_link('.gitignore') expect(page).to have_content('.gitignore') @@ -47,9 +48,10 @@ describe 'Projects > Files > User replaces files', :js do before do project2.add_reporter(user) visit(project2_tree_path_root_ref) + wait_for_requests end - it 'replaces an existed file with a new one in a forked project', :js do + it 'replaces an existed file with a new one in a forked project' do click_link('.gitignore') expect(page).to have_content('.gitignore') diff --git a/spec/javascripts/jobs/sidebar_details_block_spec.js b/spec/javascripts/jobs/sidebar_details_block_spec.js index 9c4454252ce..21ef5650b80 100644 --- a/spec/javascripts/jobs/sidebar_details_block_spec.js +++ b/spec/javascripts/jobs/sidebar_details_block_spec.js @@ -1,6 +1,7 @@ import Vue from 'vue'; import sidebarDetailsBlock from '~/jobs/components/sidebar_details_block.vue'; import job from './mock_data'; +import mountComponent from '../helpers/vue_mount_component_helper'; describe('Sidebar details block', () => { let SidebarComponent; @@ -20,39 +21,53 @@ describe('Sidebar details block', () => { describe('when it is loading', () => { it('should render a loading spinner', () => { - vm = new SidebarComponent({ - propsData: { - job: {}, - isLoading: true, - }, - }).$mount(); - + vm = mountComponent(SidebarComponent, { + job: {}, + isLoading: true, + }); expect(vm.$el.querySelector('.fa-spinner')).toBeDefined(); }); }); - describe("when user can't retry", () => { + describe('when there is no retry path retry', () => { it('should not render a retry button', () => { - vm = new SidebarComponent({ - propsData: { - job: {}, - canUserRetry: false, - isLoading: true, - }, - }).$mount(); + vm = mountComponent(SidebarComponent, { + job: {}, + isLoading: false, + }); expect(vm.$el.querySelector('.js-retry-job')).toBeNull(); }); }); - beforeEach(() => { - vm = new SidebarComponent({ - propsData: { + describe('without terminal path', () => { + it('does not render terminal link', () => { + vm = mountComponent(SidebarComponent, { job, - canUserRetry: true, isLoading: false, - }, - }).$mount(); + }); + + expect(vm.$el.querySelector('.js-terminal-link')).toBeNull(); + }); + }); + + describe('with terminal path', () => { + it('renders terminal link', () => { + vm = mountComponent(SidebarComponent, { + job, + isLoading: false, + terminalPath: 'job/43123/terminal', + }); + + expect(vm.$el.querySelector('.js-terminal-link')).not.toBeNull(); + }); + }); + + beforeEach(() => { + vm = mountComponent(SidebarComponent, { + job, + isLoading: false, + }); }); describe('actions', () => { @@ -102,13 +117,15 @@ describe('Sidebar details block', () => { }); it('should render runner ID', () => { - expect(trimWhitespace(vm.$el.querySelector('.js-job-runner'))).toEqual('Runner: local ci runner (#1)'); + expect(trimWhitespace(vm.$el.querySelector('.js-job-runner'))).toEqual( + 'Runner: local ci runner (#1)', + ); }); it('should render timeout information', () => { - expect( - trimWhitespace(vm.$el.querySelector('.js-job-timeout')), - ).toEqual('Timeout: 1m 40s (from runner)'); + expect(trimWhitespace(vm.$el.querySelector('.js-job-timeout'))).toEqual( + 'Timeout: 1m 40s (from runner)', + ); }); it('should render coverage', () => { diff --git a/spec/lib/gitlab/git/merge_base_spec.rb b/spec/lib/gitlab/git/merge_base_spec.rb new file mode 100644 index 00000000000..2f4e043a20f --- /dev/null +++ b/spec/lib/gitlab/git/merge_base_spec.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Git::MergeBase do + set(:project) { create(:project, :repository) } + let(:repository) { project.repository } + subject(:merge_base) { described_class.new(repository, refs) } + + shared_context 'existing refs with a merge base', :existing_refs do + let(:refs) do + %w(304d257dcb821665ab5110318fc58a007bd104ed 0031876facac3f2b2702a0e53a26e89939a42209) + end + end + + shared_context 'when passing a missing ref', :missing_ref do + let(:refs) do + %w(304d257dcb821665ab5110318fc58a007bd104ed aaaa) + end + end + + shared_context 'when passing refs that do not have a common ancestor', :no_common_ancestor do + let(:refs) { ['304d257dcb821665ab5110318fc58a007bd104ed', TestEnv::BRANCH_SHA['orphaned-branch']] } + end + + describe '#sha' do + context 'when the refs exist', :existing_refs do + it 'returns the SHA of the merge base' do + expect(merge_base.sha).not_to be_nil + end + + it 'memoizes the result' do + expect(repository).to receive(:merge_base).once.and_call_original + + 2.times { merge_base.sha } + end + end + + context 'when passing a missing ref', :missing_ref do + it 'does not call merge_base on the repository but raises an error' do + expect(repository).not_to receive(:merge_base) + + expect { merge_base.sha }.to raise_error(Gitlab::Git::UnknownRef) + end + end + + it 'returns `nil` when the refs do not have a common ancestor', :no_common_ancestor do + expect(merge_base.sha).to be_nil + end + + it 'returns a merge base when passing 2 branch names' do + merge_base = described_class.new(repository, %w(master feature)) + + expect(merge_base.sha).to be_present + end + + it 'returns a merge base when passing a tag name' do + merge_base = described_class.new(repository, %w(master v1.0.0)) + + expect(merge_base.sha).to be_present + end + end + + describe '#commit' do + context 'for existing refs with a merge base', :existing_refs do + it 'finds the commit for the merge base' do + expect(merge_base.commit).to be_a(Commit) + end + + it 'only looks up the commit once' do + expect(repository).to receive(:commit_by).once.and_call_original + + 2.times { merge_base.commit } + end + end + + it 'does not try to find the commit when there is no sha', :no_common_ancestor do + expect(repository).not_to receive(:commit_by) + + merge_base.commit + end + end + + describe '#unknown_refs', :missing_ref do + it 'returns the the refs passed that are not part of the repository' do + expect(merge_base.unknown_refs).to contain_exactly('aaaa') + end + + it 'only looks up the commits once' do + expect(merge_base).to receive(:commits_for_refs).once.and_call_original + + 2.times { merge_base.unknown_refs } + end + end +end diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index 6063afc213d..519638ebb82 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -465,4 +465,77 @@ describe API::Repositories do end end end + + describe 'GET :id/repository/merge_base' do + let(:refs) do + %w(304d257dcb821665ab5110318fc58a007bd104ed 0031876facac3f2b2702a0e53a26e89939a42209) + end + + subject(:request) do + get(api("/projects/#{project.id}/repository/merge_base", current_user), refs: refs) + end + + shared_examples 'merge base' do + it 'returns the common ancestor' do + request + + expect(response).to have_gitlab_http_status(:success) + expect(json_response['id']).to be_present + end + end + + context 'when unauthenticated', 'and project is public' do + it_behaves_like 'merge base' do + let(:project) { create(:project, :public, :repository) } + let(:current_user) { nil } + end + end + + context 'when unauthenticated', 'and project is private' do + it_behaves_like '404 response' do + let(:current_user) { nil } + let(:message) { '404 Project Not Found' } + end + end + + context 'when authenticated', 'as a developer' do + it_behaves_like 'merge base' do + let(:current_user) { user } + end + end + + context 'when authenticated', 'as a guest' do + it_behaves_like '403 response' do + let(:current_user) { guest } + end + end + + context 'when passing refs that do not exist' do + it_behaves_like '400 response' do + let(:refs) { %w(304d257dcb821665ab5110318fc58a007bd104ed missing) } + let(:current_user) { user } + let(:message) { 'Could not find ref: missing' } + end + end + + context 'when passing refs that do not have a merge base' do + it_behaves_like '404 response' do + let(:refs) { ['304d257dcb821665ab5110318fc58a007bd104ed', TestEnv::BRANCH_SHA['orphaned-branch']] } + let(:current_user) { user } + let(:message) { '404 Merge Base Not Found' } + end + end + + context 'when not enough refs are passed' do + let(:refs) { %w(only-one) } + let(:current_user) { user } + + it 'renders a bad request error' do + request + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq('Provide exactly 2 refs') + end + end + end end diff --git a/spec/services/commits/tag_service_spec.rb b/spec/services/commits/tag_service_spec.rb new file mode 100644 index 00000000000..82377a8dace --- /dev/null +++ b/spec/services/commits/tag_service_spec.rb @@ -0,0 +1,102 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Commits::TagService do + let(:project) { create(:project, :repository) } + let(:user) { create(:user) } + + let(:commit) { project.commit } + + before do + project.add_maintainer(user) + end + + describe '#execute' do + let(:service) { described_class.new(project, user, opts) } + + shared_examples 'tag failure' do + it 'returns a hash with the :error status' do + result = service.execute(commit) + + expect(result[:status]).to eq(:error) + expect(result[:message]).to eq(error_message) + end + + it 'does not add a system note' do + service.execute(commit) + + description_notes = find_notes('tag') + expect(description_notes).to be_empty + end + end + + def find_notes(action) + commit + .notes + .joins(:system_note_metadata) + .where(system_note_metadata: { action: action }) + end + + context 'valid params' do + let(:opts) do + { + tag_name: 'v1.2.3', + tag_message: 'Release' + } + end + + def find_notes(action) + commit + .notes + .joins(:system_note_metadata) + .where(system_note_metadata: { action: action }) + end + + context 'when tagging succeeds' do + it 'returns a hash with the :success status and created tag' do + result = service.execute(commit) + + expect(result[:status]).to eq(:success) + + tag = result[:tag] + expect(tag.name).to eq(opts[:tag_name]) + expect(tag.message).to eq(opts[:tag_message]) + end + + it 'adds a system note' do + service.execute(commit) + + description_notes = find_notes('tag') + expect(description_notes.length).to eq(1) + end + end + + context 'when tagging fails' do + let(:tag_error) { 'GitLab: You are not allowed to push code to this project.' } + + before do + tag_stub = instance_double(Tags::CreateService) + allow(Tags::CreateService).to receive(:new).and_return(tag_stub) + allow(tag_stub).to receive(:execute).and_return({ + status: :error, message: tag_error + }) + end + + it_behaves_like 'tag failure' do + let(:error_message) { tag_error } + end + end + end + + context 'invalid params' do + let(:opts) do + {} + end + + it_behaves_like 'tag failure' do + let(:error_message) { 'Missing parameter tag_name' } + end + end + end +end diff --git a/spec/services/notes/quick_actions_service_spec.rb b/spec/services/notes/quick_actions_service_spec.rb index 784dac55454..a8c994c101c 100644 --- a/spec/services/notes/quick_actions_service_spec.rb +++ b/spec/services/notes/quick_actions_service_spec.rb @@ -11,40 +11,6 @@ describe Notes::QuickActionsService do end end - shared_examples 'note on noteable that does not support quick actions' do - include_context 'note on noteable' - - before do - note.note = note_text - end - - describe 'note with only command' do - describe '/close, /label, /assign & /milestone' do - let(:note_text) { %(/close\n/assign @#{assignee.username}") } - - it 'saves the note and does not alter the note text' do - content, command_params = service.extract_commands(note) - - expect(content).to eq note_text - expect(command_params).to be_empty - end - end - end - - describe 'note with command & text' do - describe '/close, /label, /assign & /milestone' do - let(:note_text) { %(HELLO\n/close\n/assign @#{assignee.username}\nWORLD) } - - it 'saves the note and does not alter the note text' do - content, command_params = service.extract_commands(note) - - expect(content).to eq note_text - expect(command_params).to be_empty - end - end - end - end - shared_examples 'note on noteable that supports quick actions' do include_context 'note on noteable' @@ -147,16 +113,16 @@ describe Notes::QuickActionsService do expect(described_class.noteable_update_service(note)).to eq(Issues::UpdateService) end - it 'returns Issues::UpdateService for a note on a merge request' do + it 'returns MergeRequests::UpdateService for a note on a merge request' do note = create(:note_on_merge_request, project: project) expect(described_class.noteable_update_service(note)).to eq(MergeRequests::UpdateService) end - it 'returns nil for a note on a commit' do + it 'returns Commits::TagService for a note on a commit' do note = create(:note_on_commit, project: project) - expect(described_class.noteable_update_service(note)).to be_nil + expect(described_class.noteable_update_service(note)).to eq(Commits::TagService) end end @@ -175,7 +141,7 @@ describe Notes::QuickActionsService do let(:note) { create(:note_on_commit, project: project) } it 'returns false' do - expect(described_class.supported?(note)).to be_falsy + expect(described_class.supported?(note)).to be_truthy end end end @@ -203,10 +169,6 @@ describe Notes::QuickActionsService do it_behaves_like 'note on noteable that supports quick actions' do let(:note) { build(:note_on_merge_request, project: project) } end - - it_behaves_like 'note on noteable that does not support quick actions' do - let(:note) { build(:note_on_commit, project: project) } - end end context 'CE restriction for issue assignees' do diff --git a/spec/services/preview_markdown_service_spec.rb b/spec/services/preview_markdown_service_spec.rb index 81dc7c57f4a..507909d9231 100644 --- a/spec/services/preview_markdown_service_spec.rb +++ b/spec/services/preview_markdown_service_spec.rb @@ -65,6 +65,31 @@ describe PreviewMarkdownService do end end + context 'commit description' do + let(:project) { create(:project, :repository) } + let(:commit) { project.commit } + let(:params) do + { + text: "My work\n/tag v1.2.3 Stable release", + quick_actions_target_type: 'Commit', + quick_actions_target_id: commit.id + } + end + let(:service) { described_class.new(project, user, params) } + + it 'removes quick actions from text' do + result = service.execute + + expect(result[:text]).to eq 'My work' + end + + it 'explains quick actions effect' do + result = service.execute + + expect(result[:commands]).to eq 'Tags this commit to v1.2.3 with "Stable release".' + end + end + it 'sets correct markdown engine' do service = described_class.new(project, user, { markdown_version: CacheMarkdownField::CACHE_REDCARPET_VERSION }) result = service.execute diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb index 743e281183e..bf1c157c4a2 100644 --- a/spec/services/quick_actions/interpret_service_spec.rb +++ b/spec/services/quick_actions/interpret_service_spec.rb @@ -6,6 +6,7 @@ describe QuickActions::InterpretService do let(:developer2) { create(:user) } let(:issue) { create(:issue, project: project) } let(:milestone) { create(:milestone, project: project, title: '9.10') } + let(:commit) { create(:commit, project: project) } let(:inprogress) { create(:label, project: project, title: 'In Progress') } let(:bug) { create(:label, project: project, title: 'Bug') } let(:note) { build(:note, commit_id: merge_request.diff_head_sha) } @@ -347,6 +348,14 @@ describe QuickActions::InterpretService do end end + shared_examples 'tag command' do + it 'tags a commit' do + _, updates = service.execute(content, issuable) + + expect(updates).to eq(tag_name: tag_name, tag_message: tag_message) + end + end + it_behaves_like 'reopen command' do let(:content) { '/reopen' } let(:issuable) { issue } @@ -628,16 +637,6 @@ describe QuickActions::InterpretService do let(:issuable) { merge_request } end - it_behaves_like 'todo command' do - let(:content) { '/todo' } - let(:issuable) { issue } - end - - it_behaves_like 'todo command' do - let(:content) { '/todo' } - let(:issuable) { merge_request } - end - it_behaves_like 'done command' do let(:content) { '/done' } let(:issuable) { issue } @@ -787,6 +786,28 @@ describe QuickActions::InterpretService do let(:issuable) { issue } end + context '/todo' do + let(:content) { '/todo' } + + context 'if issuable is an Issue' do + it_behaves_like 'todo command' do + let(:issuable) { issue } + end + end + + context 'if issuable is a MergeRequest' do + it_behaves_like 'todo command' do + let(:issuable) { merge_request } + end + end + + context 'if issuable is a Commit' do + it_behaves_like 'empty command' do + let(:issuable) { commit } + end + end + end + context '/copy_metadata command' do let(:todo_label) { create(:label, project: project, title: 'To Do') } let(:inreview_label) { create(:label, project: project, title: 'In Review') } @@ -971,6 +992,12 @@ describe QuickActions::InterpretService do let(:issuable) { issue } end end + + context 'if issuable is a Commit' do + let(:content) { '/award :100:' } + let(:issuable) { commit } + it_behaves_like 'empty command' + end end context '/shrug command' do @@ -1102,6 +1129,32 @@ describe QuickActions::InterpretService do it_behaves_like 'empty command' end end + + context '/tag command' do + let(:issuable) { commit } + + context 'ignores command with no argument' do + it_behaves_like 'empty command' do + let(:content) { '/tag' } + end + end + + context 'tags a commit with a tag name' do + it_behaves_like 'tag command' do + let(:tag_name) { 'v1.2.3' } + let(:tag_message) { nil } + let(:content) { "/tag #{tag_name}" } + end + end + + context 'tags a commit with a tag name and message' do + it_behaves_like 'tag command' do + let(:tag_name) { 'v1.2.3' } + let(:tag_message) { 'Stable release' } + let(:content) { "/tag #{tag_name} #{tag_message}" } + end + end + end end describe '#explain' do @@ -1319,5 +1372,39 @@ describe QuickActions::InterpretService do expect(explanations).to eq(["Moves this issue to test/project."]) end end + + describe 'tag a commit' do + describe 'with a tag name' do + context 'without a message' do + let(:content) { '/tag v1.2.3' } + + it 'includes the tag name only' do + _, explanations = service.explain(content, commit) + + expect(explanations).to eq(["Tags this commit to v1.2.3."]) + end + end + + context 'with an empty message' do + let(:content) { '/tag v1.2.3 ' } + + it 'includes the tag name only' do + _, explanations = service.explain(content, commit) + + expect(explanations).to eq(["Tags this commit to v1.2.3."]) + end + end + end + + describe 'with a tag name and message' do + let(:content) { '/tag v1.2.3 Stable release' } + + it 'includes the tag name and message' do + _, explanations = service.explain(content, commit) + + expect(explanations).to eq(["Tags this commit to v1.2.3 with \"Stable release\"."]) + end + end + end end end diff --git a/spec/services/quick_actions/target_service_spec.rb b/spec/services/quick_actions/target_service_spec.rb new file mode 100644 index 00000000000..0aeb29cbeec --- /dev/null +++ b/spec/services/quick_actions/target_service_spec.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe QuickActions::TargetService do + let(:project) { create(:project) } + let(:user) { create(:user) } + let(:service) { described_class.new(project, user) } + + before do + project.add_maintainer(user) + end + + describe '#execute' do + shared_examples 'no target' do |type_id:| + it 'returns nil' do + target = service.execute(type, type_id) + + expect(target).to be_nil + end + end + + shared_examples 'find target' do + it 'returns the target' do + found_target = service.execute(type, target_id) + + expect(found_target).to eq(target) + end + end + + shared_examples 'build target' do |type_id:| + it 'builds a new target' do + target = service.execute(type, type_id) + + expect(target.project).to eq(project) + expect(target).to be_new_record + end + end + + context 'for issue' do + let(:target) { create(:issue, project: project) } + let(:target_id) { target.iid } + let(:type) { 'Issue' } + + it_behaves_like 'find target' + it_behaves_like 'build target', type_id: nil + it_behaves_like 'build target', type_id: -1 + end + + context 'for merge request' do + let(:target) { create(:merge_request, source_project: project) } + let(:target_id) { target.iid } + let(:type) { 'MergeRequest' } + + it_behaves_like 'find target' + it_behaves_like 'build target', type_id: nil + it_behaves_like 'build target', type_id: -1 + end + + context 'for commit' do + let(:project) { create(:project, :repository) } + let(:target) { project.commit.parent } + let(:target_id) { target.sha } + let(:type) { 'Commit' } + + it_behaves_like 'find target' + it_behaves_like 'no target', type_id: 'invalid_sha' + + context 'with nil target_id' do + let(:target) { project.commit } + let(:target_id) { nil } + + it_behaves_like 'find target' + end + end + + context 'for unknown type' do + let(:type) { 'unknown' } + + it_behaves_like 'no target', type_id: :unused + end + end +end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 57d081cffb3..442de61f69b 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -108,6 +108,25 @@ describe SystemNoteService do end end + describe '.tag_commit' do + let(:noteable) do + project.commit + end + let(:tag_name) { 'v1.2.3' } + + subject { described_class.tag_commit(noteable, project, author, tag_name) } + + it_behaves_like 'a system note' do + let(:action) { 'tag' } + end + + it 'sets the note text' do + link = "http://localhost/#{project.full_path}/tags/#{tag_name}" + + expect(subject.note).to eq "tagged commit #{noteable.sha} to [`#{tag_name}`](#{link})" + end + end + describe '.change_assignee' do subject { described_class.change_assignee(noteable, project, author, assignee) } diff --git a/spec/support/helpers/features/notes_helpers.rb b/spec/support/helpers/features/notes_helpers.rb index 2b9f8b30c60..89517fde6e2 100644 --- a/spec/support/helpers/features/notes_helpers.rb +++ b/spec/support/helpers/features/notes_helpers.rb @@ -20,6 +20,13 @@ module Spec end end end + + def preview_note(text) + page.within('.js-main-target-form') do + fill_in('note[note]', with: text) + click_on('Preview') + end + end end end end diff --git a/spec/views/shared/notes/_form.html.haml_spec.rb b/spec/views/shared/notes/_form.html.haml_spec.rb index c57319869f3..0189f926a5f 100644 --- a/spec/views/shared/notes/_form.html.haml_spec.rb +++ b/spec/views/shared/notes/_form.html.haml_spec.rb @@ -16,7 +16,7 @@ describe 'shared/notes/_form' do render end - %w[issue merge_request].each do |noteable| + %w[issue merge_request commit].each do |noteable| context "with a note on #{noteable}" do let(:note) { build(:"note_on_#{noteable}", project: project) } @@ -25,12 +25,4 @@ describe 'shared/notes/_form' do end end end - - context 'with a note on a commit' do - let(:note) { build(:note_on_commit, project: project) } - - it 'says that only markdown is supported, not quick actions' do - expect(rendered).to have_content('Markdown is supported') - end - end end |