Skip to content
GitLab
Menu
Projects
Groups
Snippets
Loading...
Help
Help
Support
Community forum
Keyboard shortcuts
?
Submit feedback
Contribute to GitLab
Sign in / Register
Toggle navigation
Menu
Open sidebar
heptapod
heptapod
Commits
e46dcf081ec6
Commit
da922bf3
authored
Feb 06, 2019
by
Heinrich Lee Yu
Committed by
Phil Hughes
Feb 06, 2019
Browse files
Add reply to notes to turn into discussions
parent
d1eade0ef91e
Changes
25
Hide whitespace changes
Inline
Side-by-side
app/assets/javascripts/notes/components/note_actions.vue
View file @
e46dcf08
...
...
@@ -2,11 +2,13 @@
import
{
mapGetters
}
from
'
vuex
'
;
import
Icon
from
'
~/vue_shared/components/icon.vue
'
;
import
{
GlLoadingIcon
,
GlTooltipDirective
}
from
'
@gitlab/ui
'
;
import
ReplyButton
from
'
./note_actions/reply_button.vue
'
;
export
default
{
name
:
'
NoteActions
'
,
components
:
{
Icon
,
ReplyButton
,
GlLoadingIcon
,
},
directives
:
{
...
...
@@ -21,6 +23,11 @@ export default {
type
:
[
String
,
Number
],
required
:
true
,
},
discussionId
:
{
type
:
String
,
required
:
false
,
default
:
''
,
},
noteUrl
:
{
type
:
String
,
required
:
false
,
...
...
@@ -36,6 +43,10 @@ export default {
required
:
false
,
default
:
null
,
},
showReply
:
{
type
:
Boolean
,
required
:
true
,
},
canEdit
:
{
type
:
Boolean
,
required
:
true
,
...
...
@@ -80,6 +91,9 @@ export default {
},
computed
:
{
...
mapGetters
([
'
getUserDataByProp
'
]),
showReplyButton
()
{
return
gon
.
features
&&
gon
.
features
.
replyToIndividualNotes
&&
this
.
showReply
;
},
shouldShowActionsDropdown
()
{
return
this
.
currentUserId
&&
(
this
.
canEdit
||
this
.
canReportAsAbuse
);
},
...
...
@@ -153,6 +167,12 @@ export default {
<icon
css-classes=
"link-highlight award-control-icon-super-positive"
name=
"emoji_smiley"
/>
</a>
</div>
<reply-button
v-if=
"showReplyButton"
ref=
"replyButton"
class=
"js-reply-button"
:note-id=
"discussionId"
/>
<div
v-if=
"canEdit"
class=
"note-actions-item"
>
<button
v-gl-tooltip.bottom
...
...
app/assets/javascripts/notes/components/note_actions/reply_button.vue
0 → 100644
View file @
e46dcf08
<
script
>
import
{
mapActions
}
from
'
vuex
'
;
import
{
GlTooltipDirective
,
GlButton
}
from
'
@gitlab/ui
'
;
import
Icon
from
'
~/vue_shared/components/icon.vue
'
;
export
default
{
name
:
'
ReplyButton
'
,
components
:
{
Icon
,
GlButton
,
},
directives
:
{
GlTooltip
:
GlTooltipDirective
,
},
props
:
{
noteId
:
{
type
:
String
,
required
:
true
,
},
},
methods
:
{
...
mapActions
([
'
convertToDiscussion
'
]),
},
};
</
script
>
<
template
>
<div
class=
"note-actions-item"
>
<gl-button
ref=
"button"
v-gl-tooltip.bottom
class=
"note-action-button"
variant=
"transparent"
:title=
"__('Reply to comment')"
@
click=
"convertToDiscussion(noteId)"
>
<icon
name=
"comment"
css-classes=
"link-highlight"
/>
</gl-button>
</div>
</
template
>
app/assets/javascripts/notes/components/noteable_discussion.vue
View file @
e46dcf08
...
...
@@ -398,6 +398,7 @@ Please check your network connection and try again.`;
:line=
"line"
:commit=
"commit"
:help-page-path=
"helpPagePath"
:show-reply-button=
"canReply"
@
handleDeleteNote=
"deleteNoteHandler"
>
<note-edited-text
...
...
app/assets/javascripts/notes/components/noteable_note.vue
View file @
e46dcf08
...
...
@@ -29,6 +29,11 @@ export default {
type
:
Object
,
required
:
true
,
},
discussion
:
{
type
:
Object
,
required
:
false
,
default
:
null
,
},
line
:
{
type
:
Object
,
required
:
false
,
...
...
@@ -54,7 +59,7 @@ export default {
};
},
computed
:
{
...
mapGetters
([
'
targetNoteHash
'
,
'
getNoteableData
'
,
'
getUserData
'
]),
...
mapGetters
([
'
targetNoteHash
'
,
'
getNoteableData
'
,
'
getUserData
'
,
'
commentsDisabled
'
]),
author
()
{
return
this
.
note
.
author
;
},
...
...
@@ -80,6 +85,19 @@ export default {
isTarget
()
{
return
this
.
targetNoteHash
===
this
.
noteAnchorId
;
},
discussionId
()
{
if
(
this
.
discussion
)
{
return
this
.
discussion
.
id
;
}
return
''
;
},
showReplyButton
()
{
if
(
!
this
.
discussion
||
!
this
.
getNoteableData
.
current_user
.
can_create_note
)
{
return
false
;
}
return
this
.
discussion
.
individual_note
&&
!
this
.
commentsDisabled
;
},
actionText
()
{
if
(
!
this
.
commit
)
{
return
''
;
...
...
@@ -231,6 +249,7 @@ export default {
:note-id=
"note.id"
:note-url=
"note.noteable_note_url"
:access-level=
"note.human_access"
:show-reply=
"showReplyButton"
:can-edit=
"note.current_user.can_edit"
:can-award-emoji=
"note.current_user.can_award_emoji"
:can-delete=
"note.current_user.can_edit"
...
...
@@ -241,6 +260,7 @@ export default {
:is-resolved=
"note.resolved"
:is-resolving=
"isResolving"
:resolved-by=
"note.resolved_by"
:discussion-id=
"discussionId"
@
handleEdit=
"editHandler"
@
handleDelete=
"deleteHandler"
@
handleResolve=
"resolveHandler"
...
...
app/assets/javascripts/notes/components/notes_app.vue
View file @
e46dcf08
...
...
@@ -199,7 +199,12 @@ export default {
:key=
"discussion.id"
:note=
"discussion.notes[0]"
/>
<noteable-note
v-else
:key=
"discussion.id"
:note=
"discussion.notes[0]"
/>
<noteable-note
v-else
:key=
"discussion.id"
:note=
"discussion.notes[0]"
:discussion=
"discussion"
/>
</
template
>
<noteable-discussion
v-else
...
...
app/assets/javascripts/notes/stores/actions.js
View file @
e46dcf08
...
...
@@ -426,5 +426,8 @@ export const submitSuggestion = (
});
};
export
const
convertToDiscussion
=
({
commit
},
noteId
)
=>
commit
(
types
.
CONVERT_TO_DISCUSSION
,
noteId
);
// prevent babel-plugin-rewire from generating an invalid default during karma tests
export
default
()
=>
{};
app/assets/javascripts/notes/stores/mutation_types.js
View file @
e46dcf08
...
...
@@ -17,6 +17,7 @@ export const SET_NOTES_FETCHED_STATE = 'SET_NOTES_FETCHED_STATE';
export
const
SET_NOTES_LOADING_STATE
=
'
SET_NOTES_LOADING_STATE
'
;
export
const
DISABLE_COMMENTS
=
'
DISABLE_COMMENTS
'
;
export
const
APPLY_SUGGESTION
=
'
APPLY_SUGGESTION
'
;
export
const
CONVERT_TO_DISCUSSION
=
'
CONVERT_TO_DISCUSSION
'
;
// DISCUSSION
export
const
COLLAPSE_DISCUSSION
=
'
COLLAPSE_DISCUSSION
'
;
...
...
app/assets/javascripts/notes/stores/mutations.js
View file @
e46dcf08
...
...
@@ -264,4 +264,9 @@ export default {
).
length
;
state
.
hasUnresolvedDiscussions
=
state
.
unresolvedDiscussionsCount
>
1
;
},
[
types
.
CONVERT_TO_DISCUSSION
](
state
,
discussionId
)
{
const
discussion
=
utils
.
findNoteObjectById
(
state
.
discussions
,
discussionId
);
Object
.
assign
(
discussion
,
{
individual_note
:
false
});
},
};
app/controllers/concerns/issuable_actions.rb
View file @
e46dcf08
...
...
@@ -7,6 +7,9 @@ module IssuableActions
included
do
before_action
:authorize_destroy_issuable!
,
only: :destroy
before_action
:authorize_admin_issuable!
,
only: :bulk_update
before_action
only: :show
do
push_frontend_feature_flag
(
:reply_to_individual_notes
)
end
end
def
permitted_keys
...
...
app/models/concerns/noteable.rb
View file @
e46dcf08
# frozen_string_literal: true
module
Noteable
# Names of all implementers of `Noteable` that support resolvable notes.
extend
ActiveSupport
::
Concern
# `Noteable` class names that support resolvable notes.
RESOLVABLE_TYPES
=
%w(MergeRequest)
.
freeze
class_methods
do
# `Noteable` class names that support replying to individual notes.
def
replyable_types
%w(Issue MergeRequest)
end
end
def
base_class_name
self
.
class
.
base_class
.
name
end
...
...
@@ -26,6 +35,10 @@ def supports_discussions?
DiscussionNote
.
noteable_types
.
include?
(
base_class_name
)
end
def
supports_replying_to_individual_notes?
supports_discussions?
&&
self
.
class
.
replyable_types
.
include?
(
base_class_name
)
end
def
supports_suggestion?
false
end
...
...
app/models/discussion.rb
View file @
e46dcf08
...
...
@@ -17,6 +17,8 @@ class Discussion
:for_commit?
,
:for_merge_request?
,
:save
,
to: :first_note
def
project_id
...
...
@@ -116,6 +118,10 @@ def individual_note?
false
end
def
can_convert_to_discussion?
false
end
def
new_discussion?
notes
.
length
==
1
end
...
...
app/models/individual_note_discussion.rb
View file @
e46dcf08
...
...
@@ -13,6 +13,14 @@ def individual_note?
true
end
def
can_convert_to_discussion?
noteable
.
supports_replying_to_individual_notes?
&&
Feature
.
enabled?
(
:reply_to_individual_notes
)
end
def
convert_to_discussion!
first_note
.
becomes!
(
Discussion
.
note_class
).
to_discussion
end
def
reply_attributes
super
.
tap
{
|
attrs
|
attrs
.
delete
(
:discussion_id
)
}
end
...
...
app/models/sent_notification.rb
View file @
e46dcf08
...
...
@@ -48,7 +48,7 @@ def record(noteable, recipient_id, reply_key = self.reply_key, attrs = {})
end
def
record_note
(
note
,
recipient_id
,
reply_key
=
self
.
reply_key
,
attrs
=
{})
attrs
[
:in_reply_to_discussion_id
]
=
note
.
discussion_id
attrs
[
:in_reply_to_discussion_id
]
=
note
.
discussion_id
if
note
.
part_of_discussion?
record
(
note
.
noteable
,
recipient_id
,
reply_key
,
attrs
)
end
...
...
@@ -99,29 +99,12 @@ def create_reply(message, dryrun: false)
private
def
reply_params
attrs
=
{
{
noteable_type:
self
.
noteable_type
,
noteable_id:
self
.
noteable_id
,
commit_id:
self
.
commit_id
commit_id:
self
.
commit_id
,
in_reply_to_discussion_id:
self
.
in_reply_to_discussion_id
}
if
self
.
in_reply_to_discussion_id
.
present?
attrs
[
:in_reply_to_discussion_id
]
=
self
.
in_reply_to_discussion_id
else
# Remove in GitLab 10.0, when we will not support replying to SentNotifications
# that don't have `in_reply_to_discussion_id` anymore.
attrs
.
merge!
(
type:
self
.
note_type
,
# LegacyDiffNote
line_code:
self
.
line_code
,
# DiffNote
position:
self
.
position
.
to_json
)
end
attrs
end
def
note_valid
...
...
app/services/notes/build_service.rb
View file @
e46dcf08
...
...
@@ -15,6 +15,8 @@ def execute
return
note
end
discussion
=
discussion
.
convert_to_discussion!
if
discussion
.
can_convert_to_discussion?
params
.
merge!
(
discussion
.
reply_attributes
)
should_resolve
=
discussion
.
resolved?
end
...
...
app/services/notes/create_service.rb
View file @
e46dcf08
...
...
@@ -34,6 +34,10 @@ def execute
end
if
!
only_commands
&&
note
.
save
if
note
.
part_of_discussion?
&&
note
.
discussion
.
can_convert_to_discussion?
note
.
discussion
.
convert_to_discussion!
.
save
(
touch:
false
)
end
todo_service
.
new_note
(
note
,
current_user
)
clear_noteable_diffs_cache
(
note
)
Suggestions
::
CreateService
.
new
(
note
).
execute
...
...
locale/gitlab.pot
View file @
e46dcf08
...
...
@@ -5959,6 +5959,9 @@ msgstr ""
msgid "Reopen milestone"
msgstr ""
msgid "Reply to comment"
msgstr ""
msgid "Reply to this email directly or %{view_it_on_gitlab}."
msgstr ""
...
...
spec/features/merge_request/user_posts_notes_spec.rb
View file @
e46dcf08
...
...
@@ -66,6 +66,38 @@
is_expected
.
to
have_css
(
'.js-note-text'
,
visible:
true
)
end
end
describe
'when reply_to_individual_notes feature flag is not set'
do
before
do
stub_feature_flags
(
reply_to_individual_notes:
false
)
visit
project_merge_request_path
(
project
,
merge_request
)
end
it
'does not show a reply button'
do
expect
(
page
).
to
have_no_selector
(
'.js-reply-button'
)
end
end
describe
'when reply_to_individual_notes feature flag is set'
do
before
do
stub_feature_flags
(
reply_to_individual_notes:
true
)
visit
project_merge_request_path
(
project
,
merge_request
)
end
it
'shows a reply button'
do
reply_button
=
find
(
'.js-reply-button'
,
match: :first
)
expect
(
reply_button
).
to
have_selector
(
'.ic-comment'
)
end
it
'shows reply placeholder when clicking reply button'
do
reply_button
=
find
(
'.js-reply-button'
,
match: :first
)
reply_button
.
click
expect
(
page
).
to
have_selector
(
'.discussion-reply-holder'
)
end
end
end
describe
'when previewing a note'
do
...
...
spec/javascripts/notes/components/note_actions/reply_button_spec.js
0 → 100644
View file @
e46dcf08
import
Vuex
from
'
vuex
'
;
import
{
createLocalVue
,
mount
}
from
'
@vue/test-utils
'
;
import
ReplyButton
from
'
~/notes/components/note_actions/reply_button.vue
'
;
describe
(
'
ReplyButton
'
,
()
=>
{
const
noteId
=
'
dummy-note-id
'
;
let
wrapper
;
let
convertToDiscussion
;
beforeEach
(()
=>
{
const
localVue
=
createLocalVue
();
convertToDiscussion
=
jasmine
.
createSpy
(
'
convertToDiscussion
'
);
localVue
.
use
(
Vuex
);
const
store
=
new
Vuex
.
Store
({
actions
:
{
convertToDiscussion
,
},
});
wrapper
=
mount
(
ReplyButton
,
{
propsData
:
{
noteId
,
},
store
,
sync
:
false
,
localVue
,
});
});
afterEach
(()
=>
{
wrapper
.
destroy
();
});
it
(
'
dispatches convertToDiscussion with note ID on click
'
,
()
=>
{
const
button
=
wrapper
.
find
({
ref
:
'
button
'
});
button
.
trigger
(
'
click
'
);
expect
(
convertToDiscussion
).
toHaveBeenCalledTimes
(
1
);
const
[,
payload
]
=
convertToDiscussion
.
calls
.
argsFor
(
0
);
expect
(
payload
).
toBe
(
noteId
);
});
});
spec/javascripts/notes/components/note_actions_spec.js
View file @
e46dcf08
...
...
@@ -2,14 +2,38 @@ import Vue from 'vue';
import
{
shallowMount
,
createLocalVue
}
from
'
@vue/test-utils
'
;
import
createStore
from
'
~/notes/stores
'
;
import
noteActions
from
'
~/notes/components/note_actions.vue
'
;
import
{
TEST_HOST
}
from
'
spec/test_constants
'
;
import
{
userDataMock
}
from
'
../mock_data
'
;
describe
(
'
noteActions
'
,
()
=>
{
let
wrapper
;
let
store
;
let
props
;
const
createWrapper
=
propsData
=>
{
const
localVue
=
createLocalVue
();
return
shallowMount
(
noteActions
,
{
store
,
propsData
,
localVue
,
sync
:
false
,
});
};
beforeEach
(()
=>
{
store
=
createStore
();
props
=
{
accessLevel
:
'
Maintainer
'
,
authorId
:
26
,
canDelete
:
true
,
canEdit
:
true
,
canAwardEmoji
:
true
,
canReportAsAbuse
:
true
,
noteId
:
'
539
'
,
noteUrl
:
`
${
TEST_HOST
}
/group/project/merge_requests/1#note_1`
,
reportAbusePath
:
`
${
TEST_HOST
}
/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F7%23note_539&user_id=26`
,
showReply
:
false
,
};
});
afterEach
(()
=>
{
...
...
@@ -17,31 +41,10 @@ describe('noteActions', () => {
});
describe
(
'
user is logged in
'
,
()
=>
{
let
props
;
beforeEach
(()
=>
{
props
=
{
accessLevel
:
'
Maintainer
'
,
authorId
:
26
,
canDelete
:
true
,
canEdit
:
true
,
canAwardEmoji
:
true
,
canReportAsAbuse
:
true
,
noteId
:
'
539
'
,
noteUrl
:
'
https://localhost:3000/group/project/merge_requests/1#note_1
'
,
reportAbusePath
:
'
/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F7%23note_539&user_id=26
'
,
};
store
.
dispatch
(
'
setUserData
'
,
userDataMock
);
const
localVue
=
createLocalVue
();
wrapper
=
shallowMount
(
noteActions
,
{
store
,
propsData
:
props
,
localVue
,
sync
:
false
,
});
wrapper
=
createWrapper
(
props
);
});
it
(
'
should render access level badge
'
,
()
=>
{
...
...
@@ -91,28 +94,14 @@ describe('noteActions', () => {
});
describe
(
'
user is not logged in
'
,
()
=>
{
let
props
;
beforeEach
(()
=>
{
store
.
dispatch
(
'
setUserData
'
,
{});
props
=
{
accessLevel
:
'
Maintainer
'
,
authorId
:
26
,
wrapper
=
createWrapper
({
...
props
,
canDelete
:
false
,
canEdit
:
false
,
canAwardEmoji
:
false
,
canReportAsAbuse
:
false
,
noteId
:
'
539
'
,
noteUrl
:
'
https://localhost:3000/group/project/merge_requests/1#note_1
'
,
reportAbusePath
:
'
/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F7%23note_539&user_id=26
'
,
};
const
localVue
=
createLocalVue
();
wrapper
=
shallowMount
(
noteActions
,
{
store
,
propsData
:
props
,
localVue
,
sync
:
false
,
});
});
...
...
@@ -124,4 +113,88 @@ describe('noteActions', () => {
expect
(
wrapper
.
find
(
'
.more-actions
'
).
exists
()).
toBe
(
false
);
});
});
describe
(
'
with feature flag replyToIndividualNotes enabled
'
,
()
=>
{
beforeEach
(()
=>
{
gon
.
features
=
{
replyToIndividualNotes
:
true
,
};
});
afterEach
(()
=>
{
gon
.
features
=
{};
});
describe
(
'
for showReply = true
'
,
()
=>
{
beforeEach
(()
=>
{
wrapper
=
createWrapper
({
...
props
,
showReply
:
true
,
});
});
it
(
'
shows a reply button
'
,
()
=>
{
const
replyButton
=
wrapper
.
find
({
ref
:
'
replyButton
'
});
expect
(
replyButton
.
exists
()).
toBe
(
true
);
});
});
describe
(
'
for showReply = false
'
,
()
=>
{
beforeEach
(()
=>
{
wrapper
=
createWrapper
({
...
props
,
showReply
:
false
,
});
});
it
(
'
does not show a reply button
'
,
()
=>
{
const
replyButton
=
wrapper
.
find
({
ref
:
'
replyButton
'
});
expect
(
replyButton
.
exists
()).
toBe
(
false
);
});
});
});
describe
(
'
with feature flag replyToIndividualNotes disabled
'
,
()
=>
{
beforeEach
(()
=>
{
gon
.
features
=
{
replyToIndividualNotes
:
false
,
};
});