diff options
author | Douwe Maan <douwe@gitlab.com> | 2016-10-25 10:54:48 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2016-10-25 10:54:48 +0000 |
commit | c003fc5091231c60fb9b87c842382f6078b53613 (patch) | |
tree | 5b07ab0f0597ec8b47ca4b1fa6130e748b1412f3 | |
parent | f289983db6dd0b3d237ad35f5359ecd6e09cb3ea (diff) | |
parent | fed3f718d8e2223305fb1c0ab4e72d514f50a8f6 (diff) | |
download | gitlab-ce-c003fc5091231c60fb9b87c842382f6078b53613.tar.gz |
Merge branch '23662-issue-move-user-reference-exception' into 'master'
Fix `User#to_reference`
## What does this MR do?
Fix the method signature of `User#to_reference` so that moving an issue with a user reference does not throw a "invalid number of arguments" exception.
## Why was this MR needed?
1. Changes in 8.13 require `Referable`s that don't have a project
reference to accept two arguments - `from_project` and
`target_project`.
2. `User#to_reference` was not changed to accept the
`target_project` (even though it is not used). Moving an issue
containing a user reference would throw a "invalid number of
arguments" exception.
3. The regression was introduced in [c8b2b3f7](https://gitlab.com/gitlab-org/gitlab-ce/commit/c8b2b3f7c32db873f1bebce3e3b1847ea24d235f#91fabb7ad88bd2fde6fef1c100a719c00e503047_75_79), which expects
all `Referable`s that don't respond to `:project` to have a `to_reference`
method that takes two arguments.
## Does this MR meet the acceptance criteria?
- Tests
- [x] Added for this feature/bug
- [ ] All builds are passing
- [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if it does - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
## What are the relevant issue numbers?
- Closes #23662
See merge request !7088
-rw-r--r-- | app/models/user.rb | 2 | ||||
-rw-r--r-- | spec/services/issues/move_service_spec.rb | 14 |
2 files changed, 13 insertions, 3 deletions
diff --git a/app/models/user.rb b/app/models/user.rb index 521879444d4..9e76df63d31 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -309,7 +309,7 @@ class User < ActiveRecord::Base username end - def to_reference(_from_project = nil) + def to_reference(_from_project = nil, _target_project = nil) "#{self.class.reference_prefix}#{username}" end diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb index 302eef8bf7e..f0ded06b785 100644 --- a/spec/services/issues/move_service_spec.rb +++ b/spec/services/issues/move_service_spec.rb @@ -208,10 +208,10 @@ describe Issues::MoveService, services: true do end end - describe 'rewritting references' do + describe 'rewriting references' do include_context 'issue move executed' - context 'issue reference' do + context 'issue references' do let(:another_issue) { create(:issue, project: old_project) } let(:description) { "Some description #{another_issue.to_reference}" } @@ -220,6 +220,16 @@ describe Issues::MoveService, services: true do .to eq "Some description #{old_project.to_reference}#{another_issue.to_reference}" end end + + context "user references" do + let(:another_issue) { create(:issue, project: old_project) } + let(:description) { "Some description #{user.to_reference}" } + + it "doesn't throw any errors for issues containing user references" do + expect(new_issue.description) + .to eq "Some description #{user.to_reference}" + end + end end context 'moving to same project' do |