diff options
-rw-r--r-- | app/assets/javascripts/shortcuts_issuable.js | 7 | ||||
-rw-r--r-- | app/assets/stylesheets/pages/issues.scss | 12 | ||||
-rw-r--r-- | app/finders/labels_finder.rb | 15 | ||||
-rw-r--r-- | app/helpers/blob_helper.rb | 143 | ||||
-rw-r--r-- | app/helpers/tree_helper.rb | 4 | ||||
-rw-r--r-- | app/services/issuable_base_service.rb | 14 | ||||
-rw-r--r-- | app/services/labels/find_or_create_service.rb | 22 | ||||
-rw-r--r-- | app/views/projects/blob/_header.html.haml | 4 | ||||
-rw-r--r-- | app/views/projects/diffs/_file.html.haml | 2 | ||||
-rw-r--r-- | app/views/projects/tree/_tree_header.html.haml | 2 | ||||
-rw-r--r-- | changelogs/unreleased/issue-edit-shortcut.yml | 5 | ||||
-rw-r--r-- | doc/administration/gitaly/index.md | 24 | ||||
-rw-r--r-- | lib/gitlab/git/repository.rb | 2 | ||||
-rw-r--r-- | spec/features/issues/form_spec.rb | 12 | ||||
-rw-r--r-- | spec/finders/labels_finder_spec.rb | 23 | ||||
-rw-r--r-- | spec/helpers/blob_helper_spec.rb | 8 | ||||
-rw-r--r-- | spec/lib/gitlab/git/repository_spec.rb | 14 | ||||
-rw-r--r-- | spec/services/labels/find_or_create_service_spec.rb | 78 |
18 files changed, 267 insertions, 124 deletions
diff --git a/app/assets/javascripts/shortcuts_issuable.js b/app/assets/javascripts/shortcuts_issuable.js index 689befc742e..14545824e74 100644 --- a/app/assets/javascripts/shortcuts_issuable.js +++ b/app/assets/javascripts/shortcuts_issuable.js @@ -9,13 +9,12 @@ export default class ShortcutsIssuable extends Shortcuts { super(); this.$replyField = isMergeRequest ? $('.js-main-target-form #note_note') : $('.js-main-target-form .js-vue-comment-form'); - this.editBtn = document.querySelector('.js-issuable-edit'); Mousetrap.bind('a', () => ShortcutsIssuable.openSidebarDropdown('assignee')); Mousetrap.bind('m', () => ShortcutsIssuable.openSidebarDropdown('milestone')); Mousetrap.bind('l', () => ShortcutsIssuable.openSidebarDropdown('labels')); Mousetrap.bind('r', this.replyWithSelectedText.bind(this)); - Mousetrap.bind('e', this.editIssue.bind(this)); + Mousetrap.bind('e', ShortcutsIssuable.editIssue); if (isMergeRequest) { this.enabledHelp.push('.hidden-shortcut.merge_requests'); @@ -58,10 +57,10 @@ export default class ShortcutsIssuable extends Shortcuts { return false; } - editIssue() { + static editIssue() { // Need to click the element as on issues, editing is inline // on merge request, editing is on a different page - this.editBtn.click(); + document.querySelector('.js-issuable-edit').click(); return false; } diff --git a/app/assets/stylesheets/pages/issues.scss b/app/assets/stylesheets/pages/issues.scss index 6763af4e98b..b9390450477 100644 --- a/app/assets/stylesheets/pages/issues.scss +++ b/app/assets/stylesheets/pages/issues.scss @@ -13,10 +13,20 @@ display: inline-block; } + .issuable-meta { + .author_link { + display: inline-block; + } + + .issuable-comments { + height: 18px; + } + } + .icon-merge-request-unmerged { height: 13px; margin-bottom: 3px; - } + } } } diff --git a/app/finders/labels_finder.rb b/app/finders/labels_finder.rb index f013e177c5b..5c9fce211ec 100644 --- a/app/finders/labels_finder.rb +++ b/app/finders/labels_finder.rb @@ -39,7 +39,7 @@ class LabelsFinder < UnionFinder end end elsif only_group_labels? - label_ids << Label.where(group_id: group.id) + label_ids << Label.where(group_id: group_ids) else label_ids << Label.where(group_id: projects.group_ids) label_ids << Label.where(project_id: projects.select(:id)) @@ -59,10 +59,11 @@ class LabelsFinder < UnionFinder items.where(title: title) end - def group - strong_memoize(:group) do + def group_ids + strong_memoize(:group_ids) do group = Group.find(params[:group_id]) - authorized_to_read_labels?(group) && group + groups = params[:include_ancestor_groups].present? ? group.self_and_ancestors : [group] + groups_user_can_read_labels(groups).map(&:id) end end @@ -120,4 +121,10 @@ class LabelsFinder < UnionFinder Ability.allowed?(current_user, :read_label, label_parent) end + + def groups_user_can_read_labels(groups) + DeclarativePolicy.user_scope do + groups.select { |group| authorized_to_read_labels?(group) } + end + end end diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index a6e1de6ffdc..0e806d16bc5 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -12,75 +12,42 @@ module BlobHelper def edit_blob_path(project = @project, ref = @ref, path = @path, options = {}) project_edit_blob_path(project, - tree_join(ref, path), - options[:link_opts]) - end - - def edit_blob_link(project = @project, ref = @ref, path = @path, options = {}) - blob = options.delete(:blob) - blob ||= project.repository.blob_at(ref, path) rescue nil - - return unless blob && blob.readable_text? - - common_classes = "btn js-edit-blob #{options[:extra_class]}" - - if !on_top_of_branch?(project, ref) - button_tag 'Edit', class: "#{common_classes} disabled has-tooltip", title: "You can only edit files when you are on a branch", data: { container: 'body' } - # This condition applies to anonymous or users who can edit directly - elsif !current_user || (current_user && can_modify_blob?(blob, project, ref)) - link_to 'Edit', edit_blob_path(project, ref, path, options), class: "#{common_classes} btn-sm" - elsif current_user && can?(current_user, :fork_project, project) - continue_params = { - to: edit_blob_path(project, ref, path, options), - notice: edit_in_new_fork_notice, - notice_now: edit_in_new_fork_notice_now - } - fork_path = project_forks_path(project, namespace_key: current_user.namespace.id, continue: continue_params) - - button_tag 'Edit', - class: "#{common_classes} js-edit-blob-link-fork-toggler", - data: { action: 'edit', fork_path: fork_path } - end + tree_join(ref, path), + options[:link_opts]) end def ide_edit_path(project = @project, ref = @ref, path = @path, options = {}) "#{ide_path}/project#{edit_blob_path(project, ref, path, options)}" end - def ide_edit_text - "#{_('Web IDE')}" - end + def edit_blob_button(project = @project, ref = @ref, path = @path, options = {}) + return unless blob = readable_blob(options, path, project, ref) - def ide_blob_link(project = @project, ref = @ref, path = @path, options = {}) - return unless show_new_ide? + common_classes = "btn js-edit-blob #{options[:extra_class]}" - blob = options.delete(:blob) - blob ||= project.repository.blob_at(ref, path) rescue nil + edit_button_tag(blob, + common_classes, + _('Edit'), + edit_blob_path(project, ref, path, options), + project, + ref) + end - return unless blob && blob.readable_text? + def ide_edit_button(project = @project, ref = @ref, path = @path, options = {}) + return unless show_new_ide? + return unless blob = readable_blob(options, path, project, ref) common_classes = "btn js-edit-ide #{options[:extra_class]}" - if !on_top_of_branch?(project, ref) - button_tag ide_edit_text, class: "#{common_classes} disabled has-tooltip", title: _('You can only edit files when you are on a branch'), data: { container: 'body' } - # This condition applies to anonymous or users who can edit directly - elsif current_user && can_modify_blob?(blob, project, ref) - link_to ide_edit_text, ide_edit_path(project, ref, path, options), class: "#{common_classes} btn-sm" - elsif current_user && can?(current_user, :fork_project, project) - continue_params = { - to: ide_edit_path(project, ref, path, options), - notice: edit_in_new_fork_notice, - notice_now: edit_in_new_fork_notice_now - } - fork_path = project_forks_path(project, namespace_key: current_user.namespace.id, continue: continue_params) - - button_tag ide_edit_text, - class: common_classes, - data: { fork_path: fork_path } - end + edit_button_tag(blob, + common_classes, + _('Web IDE'), + ide_edit_path(project, ref, path, options), + project, + ref) end - def modify_file_link(project = @project, ref = @ref, path = @path, label:, action:, btn_class:, modal_type:) + def modify_file_button(project = @project, ref = @ref, path = @path, label:, action:, btn_class:, modal_type:) return unless current_user blob = project.repository.blob_at(ref, path) rescue nil @@ -96,21 +63,12 @@ module BlobHelper elsif can_modify_blob?(blob, project, ref) button_tag label, class: "#{common_classes}", 'data-target' => "#modal-#{modal_type}-blob", 'data-toggle' => 'modal' elsif can?(current_user, :fork_project, project) - continue_params = { - to: request.fullpath, - notice: edit_in_new_fork_notice + " Try to #{action} this file again.", - notice_now: edit_in_new_fork_notice_now - } - fork_path = project_forks_path(project, namespace_key: current_user.namespace.id, continue: continue_params) - - button_tag label, - class: "#{common_classes} js-edit-blob-link-fork-toggler", - data: { action: action, fork_path: fork_path } + edit_fork_button_tag(common_classes, project, label, edit_modify_file_fork_params(action), action) end end def replace_blob_link(project = @project, ref = @ref, path = @path) - modify_file_link( + modify_file_button( project, ref, path, @@ -122,7 +80,7 @@ module BlobHelper end def delete_blob_link(project = @project, ref = @ref, path = @path) - modify_file_link( + modify_file_button( project, ref, path, @@ -332,4 +290,55 @@ module BlobHelper options end + + def readable_blob(options, path, project, ref) + blob = options.delete(:blob) + blob ||= project.repository.blob_at(ref, path) rescue nil + + blob if blob&.readable_text? + end + + def edit_blob_fork_params(path) + { + to: path, + notice: edit_in_new_fork_notice, + notice_now: edit_in_new_fork_notice_now + } + end + + def edit_modify_file_fork_params(action) + { + to: request.fullpath, + notice: edit_in_new_fork_notice_action(action), + notice_now: edit_in_new_fork_notice_now + } + end + + def edit_fork_button_tag(common_classes, project, label, params, action = 'edit') + fork_path = project_forks_path(project, namespace_key: current_user.namespace.id, continue: params) + + button_tag label, + class: "#{common_classes} js-edit-blob-link-fork-toggler", + data: { action: action, fork_path: fork_path } + end + + def edit_disabled_button_tag(button_text, common_classes) + button_tag(button_text, class: "#{common_classes} disabled has-tooltip", title: _('You can only edit files when you are on a branch'), data: { container: 'body' }) + end + + def edit_link_tag(link_text, edit_path, common_classes) + link_to link_text, edit_path, class: "#{common_classes} btn-sm" + end + + def edit_button_tag(blob, common_classes, text, edit_path, project, ref) + if !on_top_of_branch?(project, ref) + edit_disabled_button_tag(text, common_classes) + # This condition only applies to users who are logged in + # Web IDE (Beta) requires the user to have this feature enabled + elsif !current_user || (current_user && can_modify_blob?(blob, project, ref)) + edit_link_tag(text, edit_path, common_classes) + elsif current_user && can?(current_user, :fork_project, project) + edit_fork_button_tag(common_classes, project, text, edit_blob_fork_params(edit_path)) + end + end end diff --git a/app/helpers/tree_helper.rb b/app/helpers/tree_helper.rb index f5733b4b57c..f6a6d9bebde 100644 --- a/app/helpers/tree_helper.rb +++ b/app/helpers/tree_helper.rb @@ -83,6 +83,10 @@ module TreeHelper " A fork of this project has been created that you can make changes in, so you can submit a merge request." end + def edit_in_new_fork_notice_action(action) + edit_in_new_fork_notice + " Try to #{action} this file again." + end + def commit_in_fork_help "A new branch will be created in your fork and a new merge request will be started." end diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 66a9b1f82e0..e87fd49d193 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -77,8 +77,12 @@ class IssuableBaseService < BaseService return unless labels params[:label_ids] = labels.split(",").map do |label_name| - service = Labels::FindOrCreateService.new(current_user, project, title: label_name.strip) - label = service.execute + label = Labels::FindOrCreateService.new( + current_user, + parent, + title: label_name.strip, + available_labels: available_labels + ).execute label.try(:id) end.compact @@ -102,7 +106,7 @@ class IssuableBaseService < BaseService end def available_labels - LabelsFinder.new(current_user, project_id: @project.id).execute + @available_labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute end def merge_quick_actions_into_params!(issuable) @@ -303,4 +307,8 @@ class IssuableBaseService < BaseService def update_project_counter_caches?(issuable) issuable.state_changed? end + + def parent + project + end end diff --git a/app/services/labels/find_or_create_service.rb b/app/services/labels/find_or_create_service.rb index 940c8b333d3..079f611b3f3 100644 --- a/app/services/labels/find_or_create_service.rb +++ b/app/services/labels/find_or_create_service.rb @@ -1,8 +1,9 @@ module Labels class FindOrCreateService - def initialize(current_user, project, params = {}) + def initialize(current_user, parent, params = {}) @current_user = current_user - @project = project + @parent = parent + @available_labels = params.delete(:available_labels) @params = params.dup.with_indifferent_access end @@ -13,12 +14,13 @@ module Labels private - attr_reader :current_user, :project, :params, :skip_authorization + attr_reader :current_user, :parent, :params, :skip_authorization def available_labels @available_labels ||= LabelsFinder.new( current_user, - project_id: project.id + "#{parent_type}_id".to_sym => parent.id, + only_group_labels: parent_is_group? ).execute(skip_authorization: skip_authorization) end @@ -27,8 +29,8 @@ module Labels def find_or_create_label new_label = available_labels.find_by(title: title) - if new_label.nil? && (skip_authorization || Ability.allowed?(current_user, :admin_label, project)) - new_label = Labels::CreateService.new(params).execute(project: project) + if new_label.nil? && (skip_authorization || Ability.allowed?(current_user, :admin_label, parent)) + new_label = Labels::CreateService.new(params).execute(parent_type.to_sym => parent) end new_label @@ -37,5 +39,13 @@ module Labels def title params[:title] || params[:name] end + + def parent_type + parent.model_name.param_key + end + + def parent_is_group? + parent_type == "group" + end end end diff --git a/app/views/projects/blob/_header.html.haml b/app/views/projects/blob/_header.html.haml index 2a77dedd9a2..1b150ec3e5c 100644 --- a/app/views/projects/blob/_header.html.haml +++ b/app/views/projects/blob/_header.html.haml @@ -11,8 +11,8 @@ = view_on_environment_button(@commit.sha, @path, @environment) if @environment .btn-group{ role: "group" }< - = edit_blob_link - = ide_blob_link + = edit_blob_button + = ide_edit_button - if current_user = replace_blob_link = delete_blob_link diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml index 0b01e38d23d..47bfcb21cf4 100644 --- a/app/views/projects/diffs/_file.html.haml +++ b/app/views/projects/diffs/_file.html.haml @@ -17,7 +17,7 @@ \ - if editable_diff?(diff_file) - link_opts = @merge_request.persisted? ? { from_merge_request_iid: @merge_request.iid } : {} - = edit_blob_link(@merge_request.source_project, @merge_request.source_branch, diff_file.new_path, + = edit_blob_button(@merge_request.source_project, @merge_request.source_branch, diff_file.new_path, blob: blob, link_opts: link_opts) - if image_diff && image_replaced diff --git a/app/views/projects/tree/_tree_header.html.haml b/app/views/projects/tree/_tree_header.html.haml index 05539dfed7c..39511435508 100644 --- a/app/views/projects/tree/_tree_header.html.haml +++ b/app/views/projects/tree/_tree_header.html.haml @@ -75,7 +75,7 @@ - if show_new_ide? = succeed " " do = link_to ide_edit_path(@project, @id), class: 'btn btn-default' do - = ide_edit_text + = _('Web IDE') = link_to s_('Commits|History'), project_commits_path(@project, @id), class: 'btn' diff --git a/changelogs/unreleased/issue-edit-shortcut.yml b/changelogs/unreleased/issue-edit-shortcut.yml new file mode 100644 index 00000000000..2b29b2bc03f --- /dev/null +++ b/changelogs/unreleased/issue-edit-shortcut.yml @@ -0,0 +1,5 @@ +--- +title: Fixed issue edit shortcut not opening edit form +merge_request: +author: +type: fixed diff --git a/doc/administration/gitaly/index.md b/doc/administration/gitaly/index.md index e3b10119090..d9a61aea6ef 100644 --- a/doc/administration/gitaly/index.md +++ b/doc/administration/gitaly/index.md @@ -2,7 +2,9 @@ [Gitaly](https://gitlab.com/gitlab-org/gitaly) (introduced in GitLab 9.0) is a service that provides high-level RPC access to Git -repositories. Gitaly is a mandatory component in GitLab 9.4 and newer. +repositories. Gitaly was optional when it was first introduced in +GitLab, but since GitLab 9.4 it is a mandatory component of the +application. GitLab components that access Git repositories (gitlab-rails, gitlab-shell, gitlab-workhorse) act as clients to Gitaly. End users do @@ -184,14 +186,20 @@ Gitaly logs on your Gitaly server (`sudo gitlab-ctl tail gitaly` or coming in. One sure way to trigger a Gitaly request is to clone a repository from your GitLab server over HTTP. -## Disabling or enabling the Gitaly service +## Disabling or enabling the Gitaly service in a cluster environment If you are running Gitaly [as a remote service](#running-gitaly-on-its-own-server) you may want to disable the local Gitaly service that runs on your Gitlab server by default. -To disable the Gitaly service in your Omnibus installation, add the -following line to `/etc/gitlab/gitlab.rb`: +> 'Disabling Gitaly' only makes sense when you run GitLab in a custom +cluster configuration, where different services run on different +machines. Disabling Gitaly on all machines in the cluster is not a +valid configuration. + +If you are setting up a GitLab cluster where Gitaly does not need to +run on all machines, you can disable the Gitaly service in your +Omnibus installation, add the following line to `/etc/gitlab/gitlab.rb`: ```ruby gitaly['enable'] = false @@ -200,11 +208,13 @@ gitaly['enable'] = false When you run `gitlab-ctl reconfigure` the Gitaly service will be disabled. -To disable the Gitaly service in an installation from source, add the -following to `/etc/default/gitlab`: +To disable the Gitaly service in a GitLab cluster where you installed +GitLab from source, add the following to `/etc/default/gitlab` on the +machine where you want to disable Gitaly. ```shell gitaly_enabled=false ``` -When you run `service gitlab restart` Gitaly will be disabled. +When you run `service gitlab restart` Gitaly will be disabled on this +particular machine. diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index e3cbf017e55..ddb9cf433eb 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -2206,7 +2206,7 @@ module Gitlab with_worktree(squash_path, branch, sparse_checkout_files: diff_files, env: env) do # Apply diff of the `diff_range` to the worktree diff = run_git!(%W(diff --binary #{diff_range})) - run_git!(%w(apply --index), chdir: squash_path, env: env) do |stdin| + run_git!(%w(apply --index --whitespace=nowarn), chdir: squash_path, env: env) do |stdin| stdin.binmode stdin.write(diff) end diff --git a/spec/features/issues/form_spec.rb b/spec/features/issues/form_spec.rb index faf14be4818..c2c4b479a8a 100644 --- a/spec/features/issues/form_spec.rb +++ b/spec/features/issues/form_spec.rb @@ -271,6 +271,18 @@ describe 'New/edit issue', :js do end end + context 'inline edit' do + before do + visit project_issue_path(project, issue) + end + + it 'opens inline edit form with shortcut' do + find('body').send_keys('e') + + expect(page).to have_selector('.detail-page-description form') + end + end + describe 'sub-group project' do let(:group) { create(:group) } let(:nested_group_1) { create(:group, parent: group) } diff --git a/spec/finders/labels_finder_spec.rb b/spec/finders/labels_finder_spec.rb index 06031aee217..dc76efea35b 100644 --- a/spec/finders/labels_finder_spec.rb +++ b/spec/finders/labels_finder_spec.rb @@ -5,6 +5,8 @@ describe LabelsFinder do let(:group_1) { create(:group) } let(:group_2) { create(:group) } let(:group_3) { create(:group) } + let(:private_group_1) { create(:group, :private) } + let(:private_subgroup_1) { create(:group, :private, parent: private_group_1) } let(:project_1) { create(:project, namespace: group_1) } let(:project_2) { create(:project, namespace: group_2) } @@ -20,6 +22,8 @@ describe LabelsFinder do let!(:group_label_1) { create(:group_label, group: group_1, title: 'Label 1 (group)') } let!(:group_label_2) { create(:group_label, group: group_1, title: 'Group Label 2') } let!(:group_label_3) { create(:group_label, group: group_2, title: 'Group Label 3') } + let!(:private_group_label_1) { create(:group_label, group: private_group_1, title: 'Private Group Label 1') } + let!(:private_subgroup_label_1) { create(:group_label, group: private_subgroup_1, title: 'Private Sub Group Label 1') } let(:user) { create(:user) } @@ -66,6 +70,25 @@ describe LabelsFinder do expect(finder.execute).to eq [group_label_2, group_label_1] end end + + context 'when including labels from group ancestors', :nested_groups do + it 'returns labels from group and its ancestors' do + private_group_1.add_developer(user) + private_subgroup_1.add_developer(user) + + finder = described_class.new(user, group_id: private_subgroup_1.id, only_group_labels: true, include_ancestor_groups: true) + + expect(finder.execute).to eq [private_group_label_1, private_subgroup_label_1] + end + + it 'ignores labels from groups which user can not read' do + private_subgroup_1.add_developer(user) + + finder = described_class.new(user, group_id: private_subgroup_1.id, only_group_labels: true, include_ancestor_groups: true) + + expect(finder.execute).to eq [private_subgroup_label_1] + end + end end context 'filtering by project_id' do diff --git a/spec/helpers/blob_helper_spec.rb b/spec/helpers/blob_helper_spec.rb index a030796c54e..1fa194fe1b8 100644 --- a/spec/helpers/blob_helper_spec.rb +++ b/spec/helpers/blob_helper_spec.rb @@ -86,7 +86,7 @@ describe BlobHelper do it 'verifies blob is text' do expect(helper).not_to receive(:blob_text_viewable?) - button = edit_blob_link(project, 'refs/heads/master', 'README.md') + button = edit_blob_button(project, 'refs/heads/master', 'README.md') expect(button).to start_with('<button') end @@ -96,17 +96,17 @@ describe BlobHelper do expect(project.repository).not_to receive(:blob_at) - edit_blob_link(project, 'refs/heads/master', 'README.md', blob: blob) + edit_blob_button(project, 'refs/heads/master', 'README.md', blob: blob) end it 'returns a link with the proper route' do - link = edit_blob_link(project, 'master', 'README.md') + link = edit_blob_button(project, 'master', 'README.md') expect(Capybara.string(link).find_link('Edit')[:href]).to eq("/#{project.full_path}/edit/master/README.md") end it 'returns a link with the passed link_opts on the expected route' do - link = edit_blob_link(project, 'master', 'README.md', link_opts: { mr_id: 10 }) + link = edit_blob_button(project, 'master', 'README.md', link_opts: { mr_id: 10 }) expect(Capybara.string(link).find_link('Edit')[:href]).to eq("/#{project.full_path}/edit/master/README.md?mr_id=10") end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index d601a383a98..13358995383 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -2283,6 +2283,20 @@ describe Gitlab::Git::Repository, seed_helper: true do expect(subject).to match(/\h{40}/) end end + + context 'with trailing whitespace in an invalid patch', :skip_gitaly_mock do + let(:diff) { "diff --git a/README.md b/README.md\nindex faaf198..43c5edf 100644\n--- a/README.md\n+++ b/README.md\n@@ -1,4 +1,4 @@\n-testme\n+ \n ====== \n \n Sample repo for testing gitlab features\n" } + + it 'does not include whitespace warnings in the error' do + allow(repository).to receive(:run_git!).and_call_original + allow(repository).to receive(:run_git!).with(%W(diff --binary #{start_sha}...#{end_sha})).and_return(diff.force_encoding('ASCII-8BIT')) + + expect { subject }.to raise_error do |error| + expect(error).to be_a(described_class::GitError) + expect(error.message).not_to include('trailing whitespace') + end + end + end end end diff --git a/spec/services/labels/find_or_create_service_spec.rb b/spec/services/labels/find_or_create_service_spec.rb index 78aa5d442e7..68d5660445a 100644 --- a/spec/services/labels/find_or_create_service_spec.rb +++ b/spec/services/labels/find_or_create_service_spec.rb @@ -15,47 +15,79 @@ describe Labels::FindOrCreateService do context 'when acting on behalf of a specific user' do let(:user) { create(:user) } - subject(:service) { described_class.new(user, project, params) } - before do - project.add_developer(user) - end - context 'when label does not exist at group level' do - it 'creates a new label at project level' do - expect { service.execute }.to change(project.labels, :count).by(1) + context 'when finding labels on project level' do + subject(:service) { described_class.new(user, project, params) } + + before do + project.add_developer(user) end - end - context 'when label exists at group level' do - it 'returns the group label' do - group_label = create(:group_label, group: group, title: 'Security') + context 'when label does not exist at group level' do + it 'creates a new label at project level' do + expect { service.execute }.to change(project.labels, :count).by(1) + end + end - expect(service.execute).to eq group_label + context 'when label exists at group level' do + it 'returns the group label' do + group_label = create(:group_label, group: group, title: 'Security') + + expect(service.execute).to eq group_label + end + end + + context 'when label exists at project level' do + it 'returns the project label' do + project_label = create(:label, project: project, title: 'Security') + + expect(service.execute).to eq project_label + end end end - context 'when label does not exist at group level' do - it 'creates a new label at project leve' do - expect { service.execute }.to change(project.labels, :count).by(1) + context 'when finding labels on group level' do + subject(:service) { described_class.new(user, group, params) } + + before do + group.add_developer(user) + end + + context 'when label does not exist at group level' do + it 'creates a new label at group level' do + expect { service.execute }.to change(group.labels, :count).by(1) + end + end + + context 'when label exists at group level' do + it 'returns the group label' do + group_label = create(:group_label, group: group, title: 'Security') + + expect(service.execute).to eq group_label + end end end + end + + context 'when authorization is not required' do + context 'when finding labels on project level' do + subject(:service) { described_class.new(nil, project, params) } - context 'when label exists at project level' do it 'returns the project label' do project_label = create(:label, project: project, title: 'Security') - expect(service.execute).to eq project_label + expect(service.execute(skip_authorization: true)).to eq project_label end end - end - context 'when authorization is not required' do - subject(:service) { described_class.new(nil, project, params) } + context 'when finding labels on group level' do + subject(:service) { described_class.new(nil, group, params) } - it 'returns the project label' do - project_label = create(:label, project: project, title: 'Security') + it 'returns the group label' do + group_label = create(:group_label, group: group, title: 'Security') - expect(service.execute(skip_authorization: true)).to eq project_label + expect(service.execute(skip_authorization: true)).to eq group_label + end end end end |