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
0
Merge Requests
0
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
Léo-Paul Géneau
gitlab-ce
Commits
2247d8a4
Commit
2247d8a4
authored
Jul 27, 2016
by
Phil Hughes
Browse files
Options
Browse Files
Download
Email Patches
Plain Diff
Updates the text above discussions when resolving notes & discussions
parent
86446846
Changes
8
Show whitespace changes
Inline
Side-by-side
Showing
8 changed files
with
100 additions
and
23 deletions
+100
-23
app/assets/javascripts/diff_notes/components/resolve_btn.js.es6
...sets/javascripts/diff_notes/components/resolve_btn.js.es6
+2
-0
app/assets/javascripts/diff_notes/services/resolve.js.es6
app/assets/javascripts/diff_notes/services/resolve.js.es6
+19
-0
app/controllers/projects/discussions_controller.rb
app/controllers/projects/discussions_controller.rb
+5
-2
app/controllers/projects/notes_controller.rb
app/controllers/projects/notes_controller.rb
+9
-2
app/views/discussions/_discussion.html.haml
app/views/discussions/_discussion.html.haml
+1
-15
app/views/discussions/_headline.html.haml
app/views/discussions/_headline.html.haml
+14
-0
app/views/discussions/_parallel_diff_discussion.html.haml
app/views/discussions/_parallel_diff_discussion.html.haml
+2
-2
spec/features/merge_requests/diff_notes_resolve_spec.rb
spec/features/merge_requests/diff_notes_resolve_spec.rb
+48
-2
No files found.
app/assets/javascripts/diff_notes/components/resolve_btn.js.es6
View file @
2247d8a4
...
@@ -56,6 +56,8 @@
...
@@ -56,6 +56,8 @@
if (response.status === 200) {
if (response.status === 200) {
CommentsStore.update(this.discussionId, this.noteId, !this.isResolved, user);
CommentsStore.update(this.discussionId, this.noteId, !this.isResolved, user);
ResolveService.updateUpdatedHtml(this.discussionId, data);
}
}
this.$nextTick(this.updateTooltip);
this.$nextTick(this.updateTooltip);
...
...
app/assets/javascripts/diff_notes/services/resolve.js.es6
View file @
2247d8a4
...
@@ -58,6 +58,8 @@
...
@@ -58,6 +58,8 @@
CommentsStore.loading[discussionId] = false;
CommentsStore.loading[discussionId] = false;
CommentsStore.updateCommentsForDiscussion(discussionId, true, user);
CommentsStore.updateCommentsForDiscussion(discussionId, true, user);
this.updateUpdatedHtml(discussionId, data);
});
});
}
}
...
@@ -71,11 +73,28 @@
...
@@ -71,11 +73,28 @@
mergeRequestId,
mergeRequestId,
discussionId
discussionId
}, {}).then((response) => {
}, {}).then((response) => {
const data = response.data;
CommentsStore.loading[discussionId] = false;
CommentsStore.loading[discussionId] = false;
CommentsStore.updateCommentsForDiscussion(discussionId, false);
CommentsStore.updateCommentsForDiscussion(discussionId, false);
this.updateUpdatedHtml(discussionId, data);
});
});
}
}
updateUpdatedHtml(discussionId, data) {
const $discussionHeadline = $(`.${discussionId} .js-discussion-headline`);
if (data.updated_html) {
if ($discussionHeadline.length) {
$discussionHeadline.replaceWith(data.updated_html);
} else {
$(`.${discussionId} .discussion-header`).append(data.updated_html);
}
} else {
$discussionHeadline.remove();
}
}
}
}
w.ResolveService = new ResolveServiceClass();
w.ResolveService = new ResolveServiceClass();
...
...
app/controllers/projects/discussions_controller.rb
View file @
2247d8a4
...
@@ -10,7 +10,8 @@ class Projects::DiscussionsController < Projects::ApplicationController
...
@@ -10,7 +10,8 @@ class Projects::DiscussionsController < Projects::ApplicationController
discussion
.
resolve!
(
current_user
)
discussion
.
resolve!
(
current_user
)
render
json:
{
render
json:
{
resolved_by:
discussion
.
resolved_by
.
try
(
:name
)
resolved_by:
discussion
.
resolved_by
.
try
(
:name
),
updated_html:
view_to_html_string
(
'discussions/_headline'
,
discussion:
discussion
)
}
}
end
end
...
@@ -19,7 +20,9 @@ class Projects::DiscussionsController < Projects::ApplicationController
...
@@ -19,7 +20,9 @@ class Projects::DiscussionsController < Projects::ApplicationController
discussion
.
unresolve!
discussion
.
unresolve!
head
:ok
render
json:
{
updated_html:
view_to_html_string
(
'discussions/_headline'
,
discussion:
discussion
)
}
end
end
private
private
...
...
app/controllers/projects/notes_controller.rb
View file @
2247d8a4
...
@@ -72,8 +72,11 @@ class Projects::NotesController < Projects::ApplicationController
...
@@ -72,8 +72,11 @@ class Projects::NotesController < Projects::ApplicationController
note
.
resolve!
(
current_user
)
note
.
resolve!
(
current_user
)
discussion
=
note
.
noteable
.
discussions
.
find
{
|
d
|
d
.
id
==
note
.
discussion_id
}
||
render_404
render
json:
{
render
json:
{
resolved_by:
note
.
resolved_by
.
try
(
:name
)
resolved_by:
note
.
resolved_by
.
try
(
:name
),
updated_html:
view_to_html_string
(
'discussions/_headline'
,
discussion:
discussion
)
}
}
end
end
...
@@ -82,7 +85,11 @@ class Projects::NotesController < Projects::ApplicationController
...
@@ -82,7 +85,11 @@ class Projects::NotesController < Projects::ApplicationController
note
.
unresolve!
note
.
unresolve!
head
:ok
discussion
=
note
.
noteable
.
discussions
.
find
{
|
d
|
d
.
id
==
note
.
discussion_id
}
||
render_404
render
json:
{
updated_html:
view_to_html_string
(
'discussions/_headline'
,
discussion:
discussion
)
}
end
end
private
private
...
...
app/views/discussions/_discussion.html.haml
View file @
2247d8a4
...
@@ -38,21 +38,7 @@
...
@@ -38,21 +38,7 @@
=
time_ago_with_tooltip
(
discussion
.
created_at
,
placement:
"bottom"
,
html_class:
"note-created-ago"
)
=
time_ago_with_tooltip
(
discussion
.
created_at
,
placement:
"bottom"
,
html_class:
"note-created-ago"
)
-# TODO: Live update!
=
render
"discussions/headline"
,
discussion:
discussion
-
if
discussion
.
resolved?
.discussion-headline-light
Resolved
-
if
discussion
.
resolved_by
by
=
link_to_member
(
@project
,
discussion
.
resolved_by
,
avatar:
false
)
=
time_ago_with_tooltip
(
discussion
.
resolved_at
,
placement:
"bottom"
)
-
elsif
discussion
.
last_updated_at
!=
discussion
.
created_at
.discussion-headline-light
Last updated
-
if
discussion
.
last_updated_by
by
=
link_to_member
(
@project
,
discussion
.
last_updated_by
,
avatar:
false
)
=
time_ago_with_tooltip
(
discussion
.
last_updated_at
,
placement:
"bottom"
)
.discussion-body.js-toggle-content
{
class:
(
"hide"
unless
expanded
)
}
.discussion-body.js-toggle-content
{
class:
(
"hide"
unless
expanded
)
}
-
if
discussion
.
diff_discussion?
&&
discussion
.
diff_file
-
if
discussion
.
diff_discussion?
&&
discussion
.
diff_file
...
...
app/views/discussions/_headline.html.haml
0 → 100644
View file @
2247d8a4
-
if
discussion
.
resolved?
.discussion-headline-light.js-discussion-headline
Resolved
-
if
discussion
.
resolved_by
by
=
link_to_member
(
@project
,
discussion
.
resolved_by
,
avatar:
false
)
=
time_ago_with_tooltip
(
discussion
.
resolved_at
,
placement:
"bottom"
)
-
elsif
discussion
.
last_updated_at
!=
discussion
.
created_at
.discussion-headline-light.js-discussion-headline
Last updated
-
if
discussion
.
last_updated_by
by
=
link_to_member
(
@project
,
discussion
.
last_updated_by
,
avatar:
false
)
=
time_ago_with_tooltip
(
discussion
.
last_updated_at
,
placement:
"bottom"
)
app/views/discussions/_parallel_diff_discussion.html.haml
View file @
2247d8a4
...
@@ -11,7 +11,7 @@
...
@@ -11,7 +11,7 @@
=
link_to_reply_discussion
(
discussion_left
,
'old'
)
=
link_to_reply_discussion
(
discussion_left
,
'old'
)
.btn-group
{
role:
"group"
}
.btn-group
{
role:
"group"
}
=
render
"discussions/resolve_all"
,
discussion:
discussion_left
=
render
"discussions/resolve_all"
,
discussion:
discussion_left
=
render
"discussions/jump_to_next"
,
discussion:
discussion
=
render
"discussions/jump_to_next"
,
discussion:
discussion
_left
-
else
-
else
%td
.notes_line.old
=
""
%td
.notes_line.old
=
""
%td
.notes_content.parallel.old
=
""
%td
.notes_content.parallel.old
=
""
...
@@ -28,7 +28,7 @@
...
@@ -28,7 +28,7 @@
=
link_to_reply_discussion
(
discussion_right
,
'new'
)
=
link_to_reply_discussion
(
discussion_right
,
'new'
)
.btn-group
{
role:
"group"
}
.btn-group
{
role:
"group"
}
=
render
"discussions/resolve_all"
,
discussion:
discussion_right
=
render
"discussions/resolve_all"
,
discussion:
discussion_right
=
render
"discussions/jump_to_next"
,
discussion:
discussion
=
render
"discussions/jump_to_next"
,
discussion:
discussion
_right
-
else
-
else
%td
.notes_line.new
=
""
%td
.notes_line.new
=
""
%td
.notes_content.parallel.new
=
""
%td
.notes_content.parallel.new
=
""
spec/features/merge_requests/diff_notes_resolve_spec.rb
View file @
2247d8a4
...
@@ -119,7 +119,7 @@ feature 'Diff notes resolve', feature: true, js: true do
...
@@ -119,7 +119,7 @@ feature 'Diff notes resolve', feature: true, js: true do
page
.
find
(
'.discussion-next-btn'
).
click
page
.
find
(
'.discussion-next-btn'
).
click
end
end
expect
(
page
.
evaluate_script
(
"$('body').scrollTop()"
)).
to
be
495
expect
(
page
.
evaluate_script
(
"$('body').scrollTop()"
)).
to
be
>
0
end
end
it
'hides jump to next button when all resolved'
do
it
'hides jump to next button when all resolved'
do
...
@@ -129,6 +129,44 @@ feature 'Diff notes resolve', feature: true, js: true do
...
@@ -129,6 +129,44 @@ feature 'Diff notes resolve', feature: true, js: true do
expect
(
page
).
to
have_selector
(
'.discussion-next-btn'
,
visible:
false
)
expect
(
page
).
to
have_selector
(
'.discussion-next-btn'
,
visible:
false
)
end
end
it
'updates updated text after resolving note'
do
page
.
within
'.diff-content .note'
do
find
(
'.line-resolve-btn'
).
click
end
expect
(
page
).
to
have_content
(
"Resolved by
#{
user
.
name
}
"
)
end
end
context
'multiple notes'
do
before
do
create
(
:diff_note_on_merge_request
,
project:
project
,
noteable:
merge_request
)
end
it
'does not mark discussion as resolved when resolving single note'
do
page
.
within
'.diff-content .note'
do
first
(
'.line-resolve-btn'
).
click
end
expect
(
page
).
to
have_content
(
'Last updated'
)
page
.
within
'.line-resolve-all-container'
do
expect
(
page
).
to
have_content
(
'0/1 discussion resolved'
)
end
end
it
'resolves discussion'
do
page
.
all
(
'.note'
).
each
do
|
note
|
note
.
find
(
'.line-resolve-btn'
).
click
end
expect
(
page
).
to
have_content
(
'Resolved by'
)
page
.
within
'.line-resolve-all-container'
do
expect
(
page
).
to
have_content
(
'1/1 discussion resolved'
)
end
end
end
end
context
'muliple discussions'
do
context
'muliple discussions'
do
...
@@ -184,7 +222,15 @@ feature 'Diff notes resolve', feature: true, js: true do
...
@@ -184,7 +222,15 @@ feature 'Diff notes resolve', feature: true, js: true do
page
.
find
(
'.discussion-next-btn'
).
click
page
.
find
(
'.discussion-next-btn'
).
click
end
end
expect
(
page
.
evaluate_script
(
"$('body').scrollTop()"
)).
to
be
495
expect
(
page
.
evaluate_script
(
"$('body').scrollTop()"
)).
to
be
>
0
end
it
'updates updated text after resolving note'
do
page
.
within
first
(
'.diff-content .note'
)
do
find
(
'.line-resolve-btn'
).
click
end
expect
(
page
).
to
have_content
(
"Resolved by
#{
user
.
name
}
"
)
end
end
end
end
...
...
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