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
6db70ad2
Commit
6db70ad2
authored
Apr 26, 2017
by
Sean McGivern
Browse files
Options
Browse Files
Download
Email Patches
Plain Diff
Fix MR approvals sentence
The backend code handled this correctly, but was no longer used.
parent
b8988cb6
Changes
5
Hide whitespace changes
Inline
Side-by-side
Showing
5 changed files
with
21 additions
and
74 deletions
+21
-74
app/assets/javascripts/merge_request_widget/approvals/components/approvals_body.js
...rge_request_widget/approvals/components/approvals_body.js
+4
-1
app/helpers/merge_requests_helper.rb
app/helpers/merge_requests_helper.rb
+0
-25
changelogs/unreleased-ee/fix-mr-approval-sentence.yml
changelogs/unreleased-ee/fix-mr-approval-sentence.yml
+4
-0
spec/helpers/merge_request_helper_spec.rb
spec/helpers/merge_request_helper_spec.rb
+0
-45
spec/javascripts/approvals/approvals_body_spec.js
spec/javascripts/approvals/approvals_body_spec.js
+13
-3
No files found.
app/assets/javascripts/merge_request_widget/approvals/components/approvals_body.js
View file @
6db70ad2
...
...
@@ -44,12 +44,15 @@ Vue.component('approvals-body', {
return
''
;
}
const
separator
=
this
.
approvalsLeft
===
approvers
.
length
?
'
and
'
:
'
or
'
;
const
serialComma
=
approvers
.
length
>
2
?
'
,
'
:
''
;
return
approvers
.
length
===
1
?
approvers
[
0
].
name
:
approvers
.
reduce
((
memo
,
curr
,
index
)
=>
{
const
nextMemo
=
`
${
memo
}${
curr
.
name
}
`
;
if
(
index
===
approvers
.
length
-
2
)
{
// second to last index
return
`
${
nextMemo
}
or
`
;
return
`
${
nextMemo
}
${
serialComma
}
${
separator
}
`
;
}
else
if
(
index
===
approvers
.
length
-
1
)
{
// last index
return
nextMemo
;
}
...
...
app/helpers/merge_requests_helper.rb
View file @
6db70ad2
...
...
@@ -98,31 +98,6 @@ module MergeRequestsHelper
end
end
# This may be able to be removed with associated specs
def
render_require_section
(
merge_request
)
str
=
if
merge_request
.
approvals_left
==
1
"Requires one more approval"
else
"Requires
#{
merge_request
.
approvals_left
}
more approvals"
end
if
merge_request
.
approvers_left
.
any?
more_approvals
=
merge_request
.
approvals_left
-
merge_request
.
approvers_left
.
count
approvers_names
=
merge_request
.
approvers_left
.
map
(
&
:name
)
str
<<
if
more_approvals
>
0
" (from
#{
render_items_list
(
approvers_names
+
[
"
#{
more_approvals
}
more"
])
}
)"
elsif
more_approvals
<
0
" (from
#{
render_items_list
(
approvers_names
,
"or"
)
}
)"
else
" (from
#{
render_items_list
(
approvers_names
)
}
)"
end
end
str
end
def
mr_assign_issues_link
issues
=
MergeRequests
::
AssignIssuesService
.
new
(
@project
,
current_user
,
...
...
changelogs/unreleased-ee/fix-mr-approval-sentence.yml
0 → 100644
View file @
6db70ad2
---
title
:
Fix MR approvals sentence when all approvers need to approve the MR
merge_request
:
author
:
spec/helpers/merge_request_helper_spec.rb
View file @
6db70ad2
...
...
@@ -54,49 +54,4 @@ describe MergeRequestsHelper do
expect
(
render_items_list
(
%w(user user1 user2)
)).
to
eq
(
"user, user1 and user2"
)
end
end
describe
'render_require_section'
do
let
(
:merge_request
)
{
create
(
:merge_request
,
target_project:
project
,
source_project:
project
)
}
let
(
:approver
)
{
create
(
:user
)
}
before
do
5
.
times
{
project
.
team
<<
[
create
(
:user
),
:developer
]
}
project
.
team
<<
[
approver
,
:developer
]
end
it
"returns correct value in case - one approval"
do
project
.
update
(
approvals_before_merge:
1
)
expect
(
render_require_section
(
merge_request
)).
to
eq
(
"Requires one more approval"
)
end
it
"returns correct value in case - two approval"
do
project
.
update
(
approvals_before_merge:
2
)
expect
(
render_require_section
(
merge_request
)).
to
eq
(
"Requires 2 more approvals"
)
end
it
"returns correct value in case - one approver"
do
project
.
update
(
approvals_before_merge:
1
)
create
(
:approver
,
user:
approver
,
target:
merge_request
)
expect
(
render_require_section
(
merge_request
)).
to
eq
(
"Requires one more approval (from
#{
approver
.
name
}
)"
)
end
it
"returns correct value in case - one approver and one more"
do
project
.
update
(
approvals_before_merge:
2
)
create
(
:approver
,
user:
approver
,
target:
merge_request
)
expect
(
render_require_section
(
merge_request
)).
to
eq
(
"Requires 2 more approvals (from
#{
approver
.
name
}
and 1 more)"
)
end
it
"returns correct value in case - two approver and one more"
do
project
.
update
(
approvals_before_merge:
3
)
approver2
=
create
(
:user
)
create
(
:approver
,
user:
approver
,
target:
merge_request
)
create
(
:approver
,
user:
approver2
,
target:
merge_request
)
expect
(
render_require_section
(
merge_request
)).
to
eq
(
"Requires 3 more approvals (from
#{
approver2
.
name
}
,
#{
approver
.
name
}
and 1 more)"
)
end
end
end
spec/javascripts/approvals/approvals_body_spec.js
View file @
6db70ad2
...
...
@@ -58,7 +58,7 @@ require('~/merge_request_widget/approvals/components/approvals_body');
expect
(
this
.
approvalsBody
.
approvalsRequiredStringified
).
toBe
(
correctText
);
});
it
(
'
should display the correct string for 2 possible approver
'
,
function
(
done
)
{
it
(
'
should display the correct string for 2 possible approver
s
'
,
function
(
done
)
{
this
.
approvalsBody
.
approvalsLeft
=
2
;
this
.
approvalsBody
.
suggestedApprovers
.
push
({
name
:
'
Approver 2
'
});
...
...
@@ -81,7 +81,6 @@ require('~/merge_request_widget/approvals/components/approvals_body');
});
it
(
'
should display the correct string for 2 possible approver names
'
,
function
(
done
)
{
this
.
approvalsBody
.
approvalsLeft
=
2
;
this
.
approvalsBody
.
suggestedApprovers
.
push
({
name
:
'
Approver 2
'
});
Vue
.
nextTick
(()
=>
{
...
...
@@ -92,12 +91,23 @@ require('~/merge_request_widget/approvals/components/approvals_body');
});
it
(
'
should display the correct string for 3 possible approver names
'
,
function
(
done
)
{
this
.
approvalsBody
.
suggestedApprovers
.
push
({
name
:
'
Approver 2
'
});
this
.
approvalsBody
.
suggestedApprovers
.
push
({
name
:
'
Approver 3
'
});
Vue
.
nextTick
(()
=>
{
const
correctText
=
'
Approver 1, Approver 2, or Approver 3
'
;
expect
(
this
.
approvalsBody
.
approverNamesStringified
).
toBe
(
correctText
);
done
();
});
});
it
(
'
should join the names with "and" when all of the remaining approvers have to approve the MR
'
,
function
(
done
)
{
this
.
approvalsBody
.
approvalsLeft
=
3
;
this
.
approvalsBody
.
suggestedApprovers
.
push
({
name
:
'
Approver 2
'
});
this
.
approvalsBody
.
suggestedApprovers
.
push
({
name
:
'
Approver 3
'
});
Vue
.
nextTick
(()
=>
{
const
correctText
=
'
Approver 1, Approver 2
or
Approver 3
'
;
const
correctText
=
'
Approver 1, Approver 2
, and
Approver 3
'
;
expect
(
this
.
approvalsBody
.
approverNamesStringified
).
toBe
(
correctText
);
done
();
});
...
...
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