Skip to content
Projects
Groups
Snippets
Help
Loading...
Help
Support
Keyboard shortcuts
?
Submit feedback
Contribute to GitLab
Sign in / Register
Toggle navigation
G
gitlab-ce
Project overview
Project overview
Details
Activity
Releases
Repository
Repository
Files
Commits
Branches
Tags
Contributors
Graph
Compare
Issues
0
Issues
0
List
Boards
Labels
Milestones
Merge Requests
1
Merge Requests
1
Analytics
Analytics
Repository
Value Stream
Wiki
Wiki
Snippets
Snippets
Members
Members
Collapse sidebar
Close sidebar
Activity
Graph
Create a new issue
Commits
Issue Boards
Open sidebar
nexedi
gitlab-ce
Commits
a62aaeb5
Commit
a62aaeb5
authored
Nov 06, 2020
by
Sean McGivern
Browse files
Options
Browse Files
Download
Email Patches
Plain Diff
Revert "Merge branch 'revert-
253da9ae
' into 'master'"
This reverts merge request !45738
parent
2c4cdee7
Changes
7
Show whitespace changes
Inline
Side-by-side
Showing
7 changed files
with
26 additions
and
134 deletions
+26
-134
app/controllers/concerns/issuable_collections.rb
app/controllers/concerns/issuable_collections.rb
+1
-1
app/finders/issuable_finder.rb
app/finders/issuable_finder.rb
+1
-5
app/finders/merge_requests_finder.rb
app/finders/merge_requests_finder.rb
+0
-7
app/models/merge_request.rb
app/models/merge_request.rb
+0
-13
ee/spec/requests/api/merge_requests_spec.rb
ee/spec/requests/api/merge_requests_spec.rb
+3
-29
spec/features/dashboard/merge_requests_spec.rb
spec/features/dashboard/merge_requests_spec.rb
+3
-16
spec/finders/merge_requests_finder_spec.rb
spec/finders/merge_requests_finder_spec.rb
+18
-63
No files found.
app/controllers/concerns/issuable_collections.rb
View file @
a62aaeb5
...
@@ -150,7 +150,7 @@ module IssuableCollections
...
@@ -150,7 +150,7 @@ module IssuableCollections
common_attributes
+
[
:project
,
project: :namespace
]
common_attributes
+
[
:project
,
project: :namespace
]
when
'MergeRequest'
when
'MergeRequest'
common_attributes
+
[
common_attributes
+
[
:target_project
,
:latest_merge_request_diff
,
:approvals
,
:approved_by_users
,
:reviewers
,
:target_project
,
:latest_merge_request_diff
,
:approvals
,
:approved_by_users
,
source_project: :route
,
head_pipeline: :project
,
target_project: :namespace
source_project: :route
,
head_pipeline: :project
,
target_project: :namespace
]
]
end
end
...
...
app/finders/issuable_finder.rb
View file @
a62aaeb5
...
@@ -406,7 +406,7 @@ class IssuableFinder
...
@@ -406,7 +406,7 @@ class IssuableFinder
elsif
params
.
filter_by_any_assignee?
elsif
params
.
filter_by_any_assignee?
items
.
assigned
items
.
assigned
elsif
params
.
assignee
elsif
params
.
assignee
items
_assigned_to
(
items
,
params
.
assignee
)
items
.
assigned_to
(
params
.
assignee
)
elsif
params
.
assignee_id?
||
params
.
assignee_username?
# assignee not found
elsif
params
.
assignee_id?
||
params
.
assignee_username?
# assignee not found
items
.
none
items
.
none
else
else
...
@@ -414,10 +414,6 @@ class IssuableFinder
...
@@ -414,10 +414,6 @@ class IssuableFinder
end
end
end
end
def
items_assigned_to
(
items
,
user
)
items
.
assigned_to
(
user
)
end
def
by_negated_assignee
(
items
)
def
by_negated_assignee
(
items
)
# We want CE users to be able to say "Issues not assigned to either PersonA nor PersonB"
# We want CE users to be able to say "Issues not assigned to either PersonA nor PersonB"
if
not_params
.
assignees
.
present?
if
not_params
.
assignees
.
present?
...
...
app/finders/merge_requests_finder.rb
View file @
a62aaeb5
...
@@ -164,13 +164,6 @@ class MergeRequestsFinder < IssuableFinder
...
@@ -164,13 +164,6 @@ class MergeRequestsFinder < IssuableFinder
end
end
# rubocop: enable CodeReuse/Finder
# rubocop: enable CodeReuse/Finder
# rubocop: disable CodeReuse/ActiveRecord
def
items_assigned_to
(
items
,
user
)
assignee_or_reviewer
=
MergeRequest
.
from_union
([
super
,
items
.
reviewer_assigned_to
(
user
)])
items
.
where
(
id:
assignee_or_reviewer
)
end
# rubocop: enable CodeReuse/ActiveRecord
def
by_deployments
(
items
)
def
by_deployments
(
items
)
env
=
params
[
:environment
]
env
=
params
[
:environment
]
before
=
params
[
:deployed_before
]
before
=
params
[
:deployed_before
]
...
...
app/models/merge_request.rb
View file @
a62aaeb5
...
@@ -303,19 +303,6 @@ class MergeRequest < ApplicationRecord
...
@@ -303,19 +303,6 @@ class MergeRequest < ApplicationRecord
includes
(
:metrics
)
includes
(
:metrics
)
end
end
scope
:reviewer_assigned_to
,
->
(
user
)
do
mr_reviewers_table
=
MergeRequestReviewer
.
arel_table
inner_sql
=
mr_reviewers_table
.
project
(
Arel
::
Nodes
::
True
.
new
)
.
where
(
mr_reviewers_table
[
:merge_request_id
].
eq
(
MergeRequest
.
arel_table
[
:id
])
.
and
(
mr_reviewers_table
[
:user_id
].
eq
(
user
.
id
))
).
exists
where
(
inner_sql
)
end
after_save
:keep_around_commit
,
unless: :importing?
after_save
:keep_around_commit
,
unless: :importing?
alias_attribute
:project
,
:target_project
alias_attribute
:project
,
:target_project
...
...
ee/spec/requests/api/merge_requests_spec.rb
View file @
a62aaeb5
...
@@ -267,20 +267,12 @@ RSpec.describe API::MergeRequests do
...
@@ -267,20 +267,12 @@ RSpec.describe API::MergeRequests do
end
end
context
'filter merge requests by approval IDs'
do
context
'filter merge requests by approval IDs'
do
let_it_be
(
:user3
)
{
create
(
:user
)
}
let!
(
:merge_request_with_approval
)
do
let_it_be
(
:merge_request_with_approval
)
do
create
(
:merge_request
,
author:
user
,
source_project:
project
,
target_project:
project
,
source_branch:
'other-branch'
).
tap
do
|
mr
|
create
(
:merge_request
,
author:
user
,
source_project:
project
,
target_project:
project
,
source_branch:
'other-branch'
).
tap
do
|
mr
|
create
(
:approval
,
merge_request:
mr
,
user:
user2
)
create
(
:approval
,
merge_request:
mr
,
user:
user2
)
end
end
end
end
let_it_be
(
:merge_request_with_multiple_approvals
)
do
create
(
:merge_request
,
author:
user
,
source_project:
project
,
target_project:
project
,
source_branch:
'another-branch'
).
tap
do
|
mr
|
create
(
:approval
,
merge_request:
mr
,
user:
user2
)
create
(
:approval
,
merge_request:
mr
,
user:
user3
)
end
end
before
do
before
do
get
api
(
'/merge_requests'
,
user
),
params:
{
approved_by_ids:
approvals_param
,
scope: :all
}
get
api
(
'/merge_requests'
,
user
),
params:
{
approved_by_ids:
approvals_param
,
scope: :all
}
end
end
...
@@ -289,25 +281,7 @@ RSpec.describe API::MergeRequests do
...
@@ -289,25 +281,7 @@ RSpec.describe API::MergeRequests do
let
(
:approvals_param
)
{
[
user2
.
id
]
}
let
(
:approvals_param
)
{
[
user2
.
id
]
}
it
'returns an array of merge requests which have specified the user as an approver'
do
it
'returns an array of merge requests which have specified the user as an approver'
do
expect_response_contain_exactly
(
merge_request_with_approval
.
id
,
merge_request_with_multiple_approvals
.
id
)
expect_response_contain_exactly
(
merge_request_with_approval
.
id
)
end
end
context
'with multiple specified approved_by ids'
do
context
'when approved by all users'
do
let
(
:approvals_param
)
{
[
user2
.
id
,
user3
.
id
]
}
it
'returns an array of merge requests which have specified the user as an approver'
do
expect_response_contain_exactly
(
merge_request_with_multiple_approvals
.
id
)
end
end
context
'when approved by only one user'
do
let
(
:approvals_param
)
{
[
user
.
id
,
user2
.
id
]
}
it
'does not returns any merge request'
do
expect_empty_array_response
end
end
end
end
end
...
@@ -323,7 +297,7 @@ RSpec.describe API::MergeRequests do
...
@@ -323,7 +297,7 @@ RSpec.describe API::MergeRequests do
let
(
:approvals_param
)
{
'Any'
}
let
(
:approvals_param
)
{
'Any'
}
it
'returns an array of merge requests with any approver'
do
it
'returns an array of merge requests with any approver'
do
expect_response_contain_exactly
(
merge_request_with_approval
.
id
,
merge_request_with_multiple_approvals
.
id
)
expect_response_contain_exactly
(
merge_request_with_approval
.
id
)
end
end
end
end
...
...
spec/features/dashboard/merge_requests_spec.rb
View file @
a62aaeb5
...
@@ -52,29 +52,20 @@ RSpec.describe 'Dashboard Merge Requests' do
...
@@ -52,29 +52,20 @@ RSpec.describe 'Dashboard Merge Requests' do
end
end
context
'merge requests exist'
do
context
'merge requests exist'
do
let_it_be
(
:author_user
)
{
create
(
:user
)
}
let
(
:label
)
{
create
(
:label
)
}
let
(
:label
)
{
create
(
:label
)
}
let!
(
:assigned_merge_request
)
do
let!
(
:assigned_merge_request
)
do
create
(
:merge_request
,
create
(
:merge_request
,
assignees:
[
current_user
],
assignees:
[
current_user
],
source_project:
project
,
source_project:
project
,
author:
author_user
)
author:
create
(
:user
))
end
let!
(
:review_requested_merge_request
)
do
create
(
:merge_request
,
reviewers:
[
current_user
],
source_branch:
'review'
,
source_project:
project
,
author:
author_user
)
end
end
let!
(
:assigned_merge_request_from_fork
)
do
let!
(
:assigned_merge_request_from_fork
)
do
create
(
:merge_request
,
create
(
:merge_request
,
source_branch:
'markdown'
,
assignees:
[
current_user
],
source_branch:
'markdown'
,
assignees:
[
current_user
],
target_project:
public_project
,
source_project:
forked_project
,
target_project:
public_project
,
source_project:
forked_project
,
author:
author_user
)
author:
create
(
:user
)
)
end
end
let!
(
:authored_merge_request
)
do
let!
(
:authored_merge_request
)
do
...
@@ -103,7 +94,7 @@ RSpec.describe 'Dashboard Merge Requests' do
...
@@ -103,7 +94,7 @@ RSpec.describe 'Dashboard Merge Requests' do
create
(
:merge_request
,
create
(
:merge_request
,
source_branch:
'fix'
,
source_branch:
'fix'
,
source_project:
project
,
source_project:
project
,
author:
author_user
)
author:
create
(
:user
)
)
end
end
before
do
before
do
...
@@ -120,10 +111,6 @@ RSpec.describe 'Dashboard Merge Requests' do
...
@@ -120,10 +111,6 @@ RSpec.describe 'Dashboard Merge Requests' do
expect
(
page
).
not_to
have_content
(
labeled_merge_request
.
title
)
expect
(
page
).
not_to
have_content
(
labeled_merge_request
.
title
)
end
end
it
'shows review requested merge requests'
do
expect
(
page
).
to
have_content
(
review_requested_merge_request
.
title
)
end
it
'shows authored merge requests'
,
:js
do
it
'shows authored merge requests'
,
:js
do
reset_filters
reset_filters
input_filtered_search
(
"author:=
#{
current_user
.
to_reference
}
"
)
input_filtered_search
(
"author:=
#{
current_user
.
to_reference
}
"
)
...
...
spec/finders/merge_requests_finder_spec.rb
View file @
a62aaeb5
...
@@ -333,8 +333,6 @@ RSpec.describe MergeRequestsFinder do
...
@@ -333,8 +333,6 @@ RSpec.describe MergeRequestsFinder do
end
end
context
'assignee filtering'
do
context
'assignee filtering'
do
let_it_be
(
:user3
)
{
create
(
:user
)
}
let
(
:issuables
)
{
described_class
.
new
(
user
,
params
).
execute
}
let
(
:issuables
)
{
described_class
.
new
(
user
,
params
).
execute
}
it_behaves_like
'assignee ID filter'
do
it_behaves_like
'assignee ID filter'
do
...
@@ -353,6 +351,7 @@ RSpec.describe MergeRequestsFinder do
...
@@ -353,6 +351,7 @@ RSpec.describe MergeRequestsFinder do
merge_request3
.
assignees
=
[
user2
,
user3
]
merge_request3
.
assignees
=
[
user2
,
user3
]
end
end
let_it_be
(
:user3
)
{
create
(
:user
)
}
let
(
:params
)
{
{
assignee_username:
[
user2
.
username
,
user3
.
username
]
}
}
let
(
:params
)
{
{
assignee_username:
[
user2
.
username
,
user3
.
username
]
}
}
let
(
:expected_issuables
)
{
[
merge_request3
]
}
let
(
:expected_issuables
)
{
[
merge_request3
]
}
end
end
...
@@ -367,6 +366,7 @@ RSpec.describe MergeRequestsFinder do
...
@@ -367,6 +366,7 @@ RSpec.describe MergeRequestsFinder do
end
end
it_behaves_like
'no assignee filter'
do
it_behaves_like
'no assignee filter'
do
let_it_be
(
:user3
)
{
create
(
:user
)
}
let
(
:expected_issuables
)
{
[
merge_request4
,
merge_request5
]
}
let
(
:expected_issuables
)
{
[
merge_request4
,
merge_request5
]
}
end
end
...
@@ -374,31 +374,6 @@ RSpec.describe MergeRequestsFinder do
...
@@ -374,31 +374,6 @@ RSpec.describe MergeRequestsFinder do
let
(
:expected_issuables
)
{
[
merge_request1
,
merge_request2
,
merge_request3
]
}
let
(
:expected_issuables
)
{
[
merge_request1
,
merge_request2
,
merge_request3
]
}
end
end
context
'with just reviewers'
do
it_behaves_like
'assignee username filter'
do
before
do
merge_request4
.
reviewers
=
[
user3
]
merge_request4
.
assignees
=
[]
end
let
(
:params
)
{
{
assignee_username:
[
user3
.
username
]
}
}
let
(
:expected_issuables
)
{
[
merge_request4
]
}
end
end
context
'with an additional reviewer'
do
it_behaves_like
'assignee username filter'
do
before
do
merge_request3
.
assignees
=
[
user3
]
merge_request4
.
reviewers
=
[
user3
]
end
let
(
:params
)
{
{
assignee_username:
[
user3
.
username
]
}
}
let
(
:expected_issuables
)
{
[
merge_request3
,
merge_request4
]
}
end
end
end
context
'filtering by group milestone'
do
context
'filtering by group milestone'
do
let
(
:group_milestone
)
{
create
(
:milestone
,
group:
group
)
}
let
(
:group_milestone
)
{
create
(
:milestone
,
group:
group
)
}
...
@@ -425,6 +400,7 @@ RSpec.describe MergeRequestsFinder do
...
@@ -425,6 +400,7 @@ RSpec.describe MergeRequestsFinder do
end
end
end
end
end
end
end
context
'filtering by created_at/updated_at'
do
context
'filtering by created_at/updated_at'
do
let
(
:new_project
)
{
create
(
:project
,
forked_from_project:
project1
)
}
let
(
:new_project
)
{
create
(
:project
,
forked_from_project:
project1
)
}
...
@@ -587,27 +563,6 @@ RSpec.describe MergeRequestsFinder do
...
@@ -587,27 +563,6 @@ RSpec.describe MergeRequestsFinder do
expect
(
mrs
).
to
eq
([
mr2
])
expect
(
mrs
).
to
eq
([
mr2
])
end
end
end
end
it
'does not raise any exception with complex filters'
do
# available filters from MergeRequest dashboard UI
params
=
{
project_id:
project1
.
id
,
scope:
'authored'
,
state:
'opened'
,
author_username:
user
.
username
,
assignee_username:
user
.
username
,
approver_usernames:
[
user
.
username
],
approved_by_usernames:
[
user
.
username
],
milestone_title:
'none'
,
release_tag:
'none'
,
label_names:
'none'
,
my_reaction_emoji:
'none'
,
draft:
'no'
}
merge_requests
=
described_class
.
new
(
user
,
params
).
execute
expect
{
merge_requests
.
load
}.
not_to
raise_error
end
end
end
describe
'#row_count'
,
:request_store
do
describe
'#row_count'
,
:request_store
do
...
...
Write
Preview
Markdown
is supported
0%
Try again
or
attach a new file
Attach a file
Cancel
You are about to add
0
people
to the discussion. Proceed with caution.
Finish editing this message first!
Cancel
Please
register
or
sign in
to comment