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
ec66ef1b
Commit
ec66ef1b
authored
Oct 16, 2018
by
Oswaldo Ferreira
Browse files
Options
Browse Files
Download
Email Patches
Plain Diff
Comment on any expanded diff line on MRs
parent
912741cf
Changes
26
Expand all
Hide whitespace changes
Inline
Side-by-side
Showing
26 changed files
with
1233 additions
and
89 deletions
+1233
-89
app/assets/javascripts/diffs/components/diff_line_gutter_content.vue
...javascripts/diffs/components/diff_line_gutter_content.vue
+0
-6
app/assets/javascripts/diffs/components/diff_table_cell.vue
app/assets/javascripts/diffs/components/diff_table_cell.vue
+1
-9
app/assets/javascripts/diffs/components/inline_diff_table_row.vue
...ts/javascripts/diffs/components/inline_diff_table_row.vue
+0
-6
app/assets/javascripts/diffs/components/parallel_diff_table_row.vue
.../javascripts/diffs/components/parallel_diff_table_row.vue
+0
-6
app/assets/javascripts/diffs/constants.js
app/assets/javascripts/diffs/constants.js
+0
-2
app/assets/javascripts/diffs/store/mutations.js
app/assets/javascripts/diffs/store/mutations.js
+7
-1
app/controllers/projects/blob_controller.rb
app/controllers/projects/blob_controller.rb
+1
-1
app/controllers/projects/merge_requests/diffs_controller.rb
app/controllers/projects/merge_requests/diffs_controller.rb
+6
-0
app/models/diff_note.rb
app/models/diff_note.rb
+28
-17
app/services/merge_requests/reload_diffs_service.rb
app/services/merge_requests/reload_diffs_service.rb
+0
-4
app/services/notes/base_service.rb
app/services/notes/base_service.rb
+15
-0
app/services/notes/create_service.rb
app/services/notes/create_service.rb
+2
-1
app/services/notes/destroy_service.rb
app/services/notes/destroy_service.rb
+3
-1
changelogs/unreleased/osw-comment-on-any-line-on-diffs.yml
changelogs/unreleased/osw-comment-on-any-line-on-diffs.yml
+5
-0
lib/gitlab/diff/file.rb
lib/gitlab/diff/file.rb
+19
-0
lib/gitlab/diff/line.rb
lib/gitlab/diff/line.rb
+10
-3
lib/gitlab/diff/lines_unfolder.rb
lib/gitlab/diff/lines_unfolder.rb
+234
-0
lib/gitlab/diff/position.rb
lib/gitlab/diff/position.rb
+11
-1
spec/controllers/projects/blob_controller_spec.rb
spec/controllers/projects/blob_controller_spec.rb
+2
-2
spec/features/merge_request/user_posts_diff_notes_spec.rb
spec/features/merge_request/user_posts_diff_notes_spec.rb
+7
-6
spec/javascripts/diffs/store/mutations_spec.js
spec/javascripts/diffs/store/mutations_spec.js
+2
-2
spec/lib/gitlab/diff/file_spec.rb
spec/lib/gitlab/diff/file_spec.rb
+46
-0
spec/lib/gitlab/diff/lines_unfolder_spec.rb
spec/lib/gitlab/diff/lines_unfolder_spec.rb
+750
-0
spec/services/merge_requests/reload_diffs_service_spec.rb
spec/services/merge_requests/reload_diffs_service_spec.rb
+0
-21
spec/services/notes/create_service_spec.rb
spec/services/notes/create_service_spec.rb
+51
-0
spec/services/notes/destroy_service_spec.rb
spec/services/notes/destroy_service_spec.rb
+33
-0
No files found.
app/assets/javascripts/diffs/components/diff_line_gutter_content.vue
View file @
ec66ef1b
...
...
@@ -55,11 +55,6 @@ export default {
required
:
false
,
default
:
false
,
},
isContextLine
:
{
type
:
Boolean
,
required
:
false
,
default
:
false
,
},
isHover
:
{
type
:
Boolean
,
required
:
false
,
...
...
@@ -81,7 +76,6 @@ export default {
this
.
showCommentButton
&&
this
.
isHover
&&
!
this
.
isMatchLine
&&
!
this
.
isContextLine
&&
!
this
.
isMetaLine
&&
!
this
.
hasDiscussions
);
...
...
app/assets/javascripts/diffs/components/diff_table_cell.vue
View file @
ec66ef1b
...
...
@@ -3,7 +3,6 @@ import { mapGetters } from 'vuex';
import
DiffLineGutterContent
from
'
./diff_line_gutter_content.vue
'
;
import
{
MATCH_LINE_TYPE
,
CONTEXT_LINE_TYPE
,
EMPTY_CELL_TYPE
,
OLD_LINE_TYPE
,
OLD_NO_NEW_LINE_TYPE
,
...
...
@@ -71,9 +70,6 @@ export default {
isMatchLine
()
{
return
this
.
line
.
type
===
MATCH_LINE_TYPE
;
},
isContextLine
()
{
return
this
.
line
.
type
===
CONTEXT_LINE_TYPE
;
},
isMetaLine
()
{
const
{
type
}
=
this
.
line
;
...
...
@@ -88,11 +84,7 @@ export default {
[
type
]:
type
,
[
LINE_UNFOLD_CLASS_NAME
]:
this
.
isMatchLine
,
[
LINE_HOVER_CLASS_NAME
]:
this
.
isLoggedIn
&&
this
.
isHover
&&
!
this
.
isMatchLine
&&
!
this
.
isContextLine
&&
!
this
.
isMetaLine
,
this
.
isLoggedIn
&&
this
.
isHover
&&
!
this
.
isMatchLine
&&
!
this
.
isMetaLine
,
};
},
lineNumber
()
{
...
...
app/assets/javascripts/diffs/components/inline_diff_table_row.vue
View file @
ec66ef1b
...
...
@@ -4,8 +4,6 @@ import DiffTableCell from './diff_table_cell.vue';
import
{
NEW_LINE_TYPE
,
OLD_LINE_TYPE
,
CONTEXT_LINE_TYPE
,
CONTEXT_LINE_CLASS_NAME
,
PARALLEL_DIFF_VIEW_TYPE
,
LINE_POSITION_LEFT
,
LINE_POSITION_RIGHT
,
...
...
@@ -41,13 +39,9 @@ export default {
},
computed
:
{
...
mapGetters
(
'
diffs
'
,
[
'
isInlineView
'
]),
isContextLine
()
{
return
this
.
line
.
type
===
CONTEXT_LINE_TYPE
;
},
classNameMap
()
{
return
{
[
this
.
line
.
type
]:
this
.
line
.
type
,
[
CONTEXT_LINE_CLASS_NAME
]:
this
.
isContextLine
,
[
PARALLEL_DIFF_VIEW_TYPE
]:
this
.
isParallelView
,
};
},
...
...
app/assets/javascripts/diffs/components/parallel_diff_table_row.vue
View file @
ec66ef1b
...
...
@@ -5,8 +5,6 @@ import DiffTableCell from './diff_table_cell.vue';
import
{
NEW_LINE_TYPE
,
OLD_LINE_TYPE
,
CONTEXT_LINE_TYPE
,
CONTEXT_LINE_CLASS_NAME
,
OLD_NO_NEW_LINE_TYPE
,
PARALLEL_DIFF_VIEW_TYPE
,
NEW_NO_NEW_LINE_TYPE
,
...
...
@@ -43,12 +41,8 @@ export default {
};
},
computed
:
{
isContextLine
()
{
return
this
.
line
.
left
&&
this
.
line
.
left
.
type
===
CONTEXT_LINE_TYPE
;
},
classNameMap
()
{
return
{
[
CONTEXT_LINE_CLASS_NAME
]:
this
.
isContextLine
,
[
PARALLEL_DIFF_VIEW_TYPE
]:
true
,
};
},
...
...
app/assets/javascripts/diffs/constants.js
View file @
ec66ef1b
...
...
@@ -3,7 +3,6 @@ export const PARALLEL_DIFF_VIEW_TYPE = 'parallel';
export
const
MATCH_LINE_TYPE
=
'
match
'
;
export
const
OLD_NO_NEW_LINE_TYPE
=
'
old-nonewline
'
;
export
const
NEW_NO_NEW_LINE_TYPE
=
'
new-nonewline
'
;
export
const
CONTEXT_LINE_TYPE
=
'
context
'
;
export
const
EMPTY_CELL_TYPE
=
'
empty-cell
'
;
export
const
COMMENT_FORM_TYPE
=
'
commentForm
'
;
export
const
DIFF_NOTE_TYPE
=
'
DiffNote
'
;
...
...
@@ -21,7 +20,6 @@ export const LINE_SIDE_RIGHT = 'right-side';
export
const
DIFF_VIEW_COOKIE_NAME
=
'
diff_view
'
;
export
const
LINE_HOVER_CLASS_NAME
=
'
is-over
'
;
export
const
LINE_UNFOLD_CLASS_NAME
=
'
unfold js-unfold
'
;
export
const
CONTEXT_LINE_CLASS_NAME
=
'
diff-expanded
'
;
export
const
UNFOLD_COUNT
=
20
;
export
const
COUNT_OF_AVATARS_IN_GUTTER
=
3
;
...
...
app/assets/javascripts/diffs/store/mutations.js
View file @
ec66ef1b
...
...
@@ -65,7 +65,13 @@ export default {
const
{
highlightedDiffLines
,
parallelDiffLines
}
=
diffFile
;
removeMatchLine
(
diffFile
,
lineNumbers
,
bottom
);
const
lines
=
addLineReferences
(
contextLines
,
lineNumbers
,
bottom
);
const
lines
=
addLineReferences
(
contextLines
,
lineNumbers
,
bottom
).
map
(
line
=>
({
...
line
,
lineCode
:
line
.
lineCode
||
`
${
fileHash
}
_
${
line
.
oldLine
}
_
${
line
.
newLine
}
`
,
discussions
:
line
.
discussions
||
[],
}));
addContextLines
({
inlineLines
:
highlightedDiffLines
,
parallelLines
:
parallelDiffLines
,
...
...
app/controllers/projects/blob_controller.rb
View file @
ec66ef1b
...
...
@@ -122,7 +122,7 @@ class Projects::BlobController < Projects::ApplicationController
@lines
.
map!
do
|
line
|
# These are marked as context lines but are loaded from blobs.
# We also have context lines loaded from diffs in other places.
diff_line
=
Gitlab
::
Diff
::
Line
.
new
(
line
,
'context'
,
nil
,
nil
,
nil
)
diff_line
=
Gitlab
::
Diff
::
Line
.
new
(
line
,
nil
,
nil
,
nil
,
nil
)
diff_line
.
rich_text
=
line
diff_line
end
...
...
app/controllers/projects/merge_requests/diffs_controller.rb
View file @
ec66ef1b
...
...
@@ -22,6 +22,12 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
def
render_diffs
@environment
=
@merge_request
.
environments_for
(
current_user
).
last
notes_grouped_by_path
=
@notes
.
group_by
{
|
note
|
note
.
position
.
file_path
}
@diffs
.
diff_files
.
each
do
|
diff_file
|
notes
=
notes_grouped_by_path
.
fetch
(
diff_file
.
file_path
,
[])
notes
.
each
{
|
note
|
diff_file
.
unfold_diff_lines
(
note
.
position
)
}
end
@diffs
.
write_cache
...
...
app/models/diff_note.rb
View file @
ec66ef1b
...
...
@@ -64,6 +64,10 @@ class DiffNote < Note
self
.
original_position
.
diff_refs
==
diff_refs
end
def
discussion_first_note?
self
==
discussion
.
first_note
end
private
def
enqueue_diff_file_creation_job
...
...
@@ -76,26 +80,33 @@ class DiffNote < Note
end
def
should_create_diff_file?
on_text?
&&
note_diff_file
.
nil?
&&
self
==
discussion
.
first_note
on_text?
&&
note_diff_file
.
nil?
&&
discussion_first_note?
end
def
fetch_diff_file
if
note_diff_file
diff
=
Gitlab
::
Git
::
Diff
.
new
(
note_diff_file
.
to_hash
)
Gitlab
::
Diff
::
File
.
new
(
diff
,
repository:
project
.
repository
,
diff_refs:
original_position
.
diff_refs
)
elsif
created_at_diff?
(
noteable
.
diff_refs
)
# We're able to use the already persisted diffs (Postgres) if we're
# presenting a "current version" of the MR discussion diff.
# So no need to make an extra Gitaly diff request for it.
# As an extra benefit, the returned `diff_file` already
# has `highlighted_diff_lines` data set from Redis on
# `Diff::FileCollection::MergeRequestDiff`.
noteable
.
diffs
(
original_position
.
diff_options
).
diff_files
.
first
else
original_position
.
diff_file
(
self
.
project
.
repository
)
end
file
=
if
note_diff_file
diff
=
Gitlab
::
Git
::
Diff
.
new
(
note_diff_file
.
to_hash
)
Gitlab
::
Diff
::
File
.
new
(
diff
,
repository:
project
.
repository
,
diff_refs:
original_position
.
diff_refs
)
elsif
created_at_diff?
(
noteable
.
diff_refs
)
# We're able to use the already persisted diffs (Postgres) if we're
# presenting a "current version" of the MR discussion diff.
# So no need to make an extra Gitaly diff request for it.
# As an extra benefit, the returned `diff_file` already
# has `highlighted_diff_lines` data set from Redis on
# `Diff::FileCollection::MergeRequestDiff`.
noteable
.
diffs
(
original_position
.
diff_options
).
diff_files
.
first
else
original_position
.
diff_file
(
self
.
project
.
repository
)
end
# Since persisted diff files already have its content "unfolded"
# there's no need to make it pass through the unfolding process.
file
&
.
unfold_diff_lines
(
position
)
unless
note_diff_file
file
end
def
supported?
...
...
app/services/merge_requests/reload_diffs_service.rb
View file @
ec66ef1b
...
...
@@ -29,10 +29,6 @@ module MergeRequests
# rubocop: disable CodeReuse/ActiveRecord
def
clear_cache
(
new_diff
)
# Executing the iteration we cache highlighted diffs for each diff file of
# MergeRequestDiff.
cacheable_collection
(
new_diff
).
write_cache
# Remove cache for all diffs on this MR. Do not use the association on the
# model, as that will interfere with other actions happening when
# reloading the diff.
...
...
app/services/notes/base_service.rb
0 → 100644
View file @
ec66ef1b
# frozen_string_literal: true
module
Notes
class
BaseService
<
::
BaseService
def
clear_noteable_diffs_cache
(
note
)
noteable
=
note
.
noteable
if
note
.
is_a?
(
DiffNote
)
&&
note
.
discussion_first_note?
&&
note
.
position
.
unfolded_diff?
(
project
.
repository
)
noteable
.
diffs
.
clear_cache
end
end
end
end
app/services/notes/create_service.rb
View file @
ec66ef1b
# frozen_string_literal: true
module
Notes
class
CreateService
<
::
BaseService
class
CreateService
<
::
Notes
::
BaseService
def
execute
merge_request_diff_head_sha
=
params
.
delete
(
:merge_request_diff_head_sha
)
...
...
@@ -35,6 +35,7 @@ module Notes
if
!
only_commands
&&
note
.
save
todo_service
.
new_note
(
note
,
current_user
)
clear_noteable_diffs_cache
(
note
)
end
if
command_params
.
present?
...
...
app/services/notes/destroy_service.rb
View file @
ec66ef1b
# frozen_string_literal: true
module
Notes
class
DestroyService
<
BaseService
class
DestroyService
<
::
Notes
::
BaseService
def
execute
(
note
)
TodoService
.
new
.
destroy_target
(
note
)
do
|
note
|
note
.
destroy
end
clear_noteable_diffs_cache
(
note
)
end
end
end
changelogs/unreleased/osw-comment-on-any-line-on-diffs.yml
0 → 100644
View file @
ec66ef1b
---
title
:
Allow commenting on any diff line in Merge Requests
merge_request
:
author
:
type
:
added
lib/gitlab/diff/file.rb
View file @
ec66ef1b
...
...
@@ -26,6 +26,7 @@ module Gitlab
@repository
=
repository
@diff_refs
=
diff_refs
@fallback_diff_refs
=
fallback_diff_refs
@unfolded
=
false
# Ensure items are collected in the the batch
new_blob_lazy
...
...
@@ -135,6 +136,24 @@ module Gitlab
Gitlab
::
Diff
::
Parser
.
new
.
parse
(
raw_diff
.
each_line
,
diff_file:
self
).
to_a
end
# Changes diff_lines according to the given position. That is,
# it checks whether the position requires blob lines into the diff
# in order to be presented.
def
unfold_diff_lines
(
position
)
return
unless
position
unfolder
=
Gitlab
::
Diff
::
LinesUnfolder
.
new
(
self
,
position
)
if
unfolder
.
unfold_required?
@diff_lines
=
unfolder
.
unfolded_diff_lines
@unfolded
=
true
end
end
def
unfolded?
@unfolded
end
def
highlighted_diff_lines
@highlighted_diff_lines
||=
Gitlab
::
Diff
::
Highlight
.
new
(
self
,
repository:
self
.
repository
).
highlight
...
...
lib/gitlab/diff/line.rb
View file @
ec66ef1b
...
...
@@ -3,9 +3,9 @@ module Gitlab
class
Line
SERIALIZE_KEYS
=
%i(line_code rich_text text type index old_pos new_pos)
.
freeze
attr_reader
:line_code
,
:type
,
:
index
,
:
old_pos
,
:new_pos
attr_reader
:line_code
,
:type
,
:old_pos
,
:new_pos
attr_writer
:rich_text
attr_accessor
:text
attr_accessor
:text
,
:index
def
initialize
(
text
,
type
,
index
,
old_pos
,
new_pos
,
parent_file:
nil
,
line_code:
nil
,
rich_text:
nil
)
@text
,
@type
,
@index
=
text
,
type
,
index
...
...
@@ -19,7 +19,14 @@ module Gitlab
end
def
self
.
init_from_hash
(
hash
)
new
(
hash
[
:text
],
hash
[
:type
],
hash
[
:index
],
hash
[
:old_pos
],
hash
[
:new_pos
],
line_code:
hash
[
:line_code
],
rich_text:
hash
[
:rich_text
])
new
(
hash
[
:text
],
hash
[
:type
],
hash
[
:index
],
hash
[
:old_pos
],
hash
[
:new_pos
],
parent_file:
hash
[
:parent_file
],
line_code:
hash
[
:line_code
],
rich_text:
hash
[
:rich_text
])
end
def
to_hash
...
...
lib/gitlab/diff/lines_unfolder.rb
0 → 100644
View file @
ec66ef1b
# frozen_string_literal: true
# Given a position, calculates which Blob lines should be extracted, treated and
# injected in the current diff file lines in order to present a "unfolded" diff.
module
Gitlab
module
Diff
class
LinesUnfolder
include
Gitlab
::
Utils
::
StrongMemoize
UNFOLD_CONTEXT_SIZE
=
3
def
initialize
(
diff_file
,
position
)
@diff_file
=
diff_file
@blob
=
diff_file
.
old_blob
@position
=
position
@generate_top_match_line
=
true
@generate_bottom_match_line
=
true
# These methods update `@generate_top_match_line` and
# `@generate_bottom_match_line`.
@from_blob_line
=
calculate_from_blob_line!
@to_blob_line
=
calculate_to_blob_line!
end
# Returns merged diff lines with required blob lines with correct
# positions.
def
unfolded_diff_lines
strong_memoize
(
:unfolded_diff_lines
)
do
next
unless
unfold_required?
merged_diff_with_blob_lines
end
end
# Returns the extracted lines from the old blob which should be merged
# with the current diff lines.
def
blob_lines
strong_memoize
(
:blob_lines
)
do
# Blob lines, unlike diffs, doesn't start with an empty space for
# unchanged line, so the parsing and highlighting step can get fuzzy
# without the following change.
line_prefix
=
' '
blob_as_diff_lines
=
@blob
.
data
.
each_line
.
map
{
|
line
|
"
#{
line_prefix
}#{
line
}
"
}
lines
=
Gitlab
::
Diff
::
Parser
.
new
.
parse
(
blob_as_diff_lines
,
diff_file:
@diff_file
).
to_a
from
=
from_blob_line
-
1
to
=
to_blob_line
-
1
lines
[
from
..
to
]
end
end
def
unfold_required?
strong_memoize
(
:unfold_required
)
do
next
false
unless
@diff_file
.
text?
next
false
if
@blob
.
nil?
next
false
unless
@position
.
old_line
# Invalid position (MR import scenario)
next
false
if
@position
.
old_line
>
@blob
.
lines
.
size
next
false
if
@diff_file
.
diff_lines
.
empty?
next
false
if
@diff_file
.
line_for_position
(
@position
)
next
false
unless
unfold_line
true
end
end
private
attr_reader
:from_blob_line
,
:to_blob_line
def
merged_diff_with_blob_lines
lines
=
@diff_file
.
diff_lines
match_line
=
unfold_line
insert_index
=
bottom?
?
-
1
:
match_line
.
index
lines
-=
[
match_line
]
unless
bottom?
lines
.
insert
(
insert_index
,
*
blob_lines_with_matches
)
# The inserted blob lines have invalid indexes, so we need
# to reindex them.
reindex
(
lines
)
lines
end
# Returns 'unchanged' blob lines with recalculated `old_pos` and
# `new_pos` and the recalculated new match line (needed if we for instance
# we unfolded once, but there are still folded lines).
def
blob_lines_with_matches
old_pos
=
from_blob_line
new_pos
=
from_blob_line
+
offset
new_blob_lines
=
[]
new_blob_lines
.
push
(
top_blob_match_line
)
if
top_blob_match_line
blob_lines
.
each
do
|
line
|
new_blob_lines
<<
Gitlab
::
Diff
::
Line
.
new
(
line
.
text
,
line
.
type
,
nil
,
old_pos
,
new_pos
,
parent_file:
@diff_file
)
old_pos
+=
1
new_pos
+=
1
end
new_blob_lines
.
push
(
bottom_blob_match_line
)
if
bottom_blob_match_line
new_blob_lines
end
def
reindex
(
lines
)
lines
.
each_with_index
{
|
line
,
i
|
line
.
index
=
i
}
end
def
top_blob_match_line
strong_memoize
(
:top_blob_match_line
)
do
next
unless
@generate_top_match_line
old_pos
=
from_blob_line
new_pos
=
from_blob_line
+
offset
build_match_line
(
old_pos
,
new_pos
)
end
end
def
bottom_blob_match_line
strong_memoize
(
:bottom_blob_match_line
)
do
# The bottom line match addition is already handled on
# Diff::File#diff_lines_for_serializer
next
if
bottom?
next
unless
@generate_bottom_match_line
position
=
line_after_unfold_position
.
old_pos
old_pos
=
position
new_pos
=
position
+
offset
build_match_line
(
old_pos
,
new_pos
)
end
end
def
build_match_line
(
old_pos
,
new_pos
)
blob_lines_length
=
blob_lines
.
length
old_line_ref
=
[
old_pos
,
blob_lines_length
].
join
(
','
)
new_line_ref
=
[
new_pos
,
blob_lines_length
].
join
(
','
)
new_match_line_str
=
"@@ -
#{
old_line_ref
}
+
#{
new_line_ref
}
@@"
Gitlab
::
Diff
::
Line
.
new
(
new_match_line_str
,
'match'
,
nil
,
old_pos
,
new_pos
)
end
# Returns the first line position that should be extracted
# from `blob_lines`.
def
calculate_from_blob_line!
return
unless
unfold_required?
from
=
comment_position
-
UNFOLD_CONTEXT_SIZE
# There's no line before the match if it's in the top-most
# position.
prev_line_number
=
line_before_unfold_position
&
.
old_pos
||
0
if
from
<=
prev_line_number
+
1
@generate_top_match_line
=
false
from
=
prev_line_number
+
1
end
from
end
# Returns the last line position that should be extracted
# from `blob_lines`.
def
calculate_to_blob_line!
return
unless
unfold_required?
to
=
comment_position
+
UNFOLD_CONTEXT_SIZE
return
to
if
bottom?
next_line_number
=
line_after_unfold_position
.
old_pos
if
to
>=
next_line_number
-
1
@generate_bottom_match_line
=
false
to
=
next_line_number
-
1
end
to
end
def
offset
unfold_line
.
new_pos
-
unfold_line
.
old_pos
end
def
line_before_unfold_position
return
unless
index
=
unfold_line
&
.
index
@diff_file
.
diff_lines
[
index
-
1
]
if
index
>
0
end
def
line_after_unfold_position
return
unless
index
=
unfold_line
&
.
index
@diff_file
.
diff_lines
[
index
+
1
]
if
index
>=
0
end
def
bottom?
strong_memoize
(
:bottom
)
do
@position
.
old_line
>
last_line
.
old_pos
end
end
# Returns the line which needed to be expanded in order to send a comment
# in `@position`.
def
unfold_line
strong_memoize
(
:unfold_line
)
do
next
last_line
if
bottom?
@diff_file
.
diff_lines
.
find
do
|
line
|
line
.
old_pos
>
comment_position
&&
line
.
type
==
'match'
end
end
end
def
comment_position
@position
.
old_line
end
def
last_line
@diff_file
.
diff_lines
.
last
end
end
end
end
lib/gitlab/diff/position.rb
View file @
ec66ef1b
...
...
@@ -101,6 +101,10 @@ module Gitlab
@diff_refs
||=
DiffRefs
.
new
(
base_sha:
base_sha
,
start_sha:
start_sha
,
head_sha:
head_sha
)
end
def
unfolded_diff?
(
repository
)
diff_file
(
repository
)
&
.
unfolded?
end
def
diff_file
(
repository
)
return
@diff_file
if
defined?
(
@diff_file
)
...
...
@@ -134,7 +138,13 @@ module Gitlab
return
unless
diff_refs
.
complete?
return
unless
comparison
=
diff_refs
.
compare_in
(
repository
.
project
)
comparison
.
diffs
(
diff_options
).
diff_files
.
first
file
=
comparison
.
diffs
(
diff_options
).
diff_files
.
first
# We need to unfold diff lines according to the position in order
# to correctly calculate the line code and trace position changes.
file
&
.
unfold_diff_lines
(
self
)
file
end
def
get_formatter_class
(
type
)
...
...
spec/controllers/projects/blob_controller_spec.rb
View file @
ec66ef1b
...
...
@@ -157,7 +157,7 @@ describe Projects::BlobController do
match_line
=
JSON
.
parse
(
response
.
body
).
first
expect
(
match_line
[
'type'
]).
to
eq
(
'context'
)
expect
(
match_line
[
'type'
]).
to
be_nil
end
it
'adds bottom match line when "t"o is less than blob size'
do
...
...
@@ -177,7 +177,7 @@ describe Projects::BlobController do
match_line
=
JSON
.
parse
(
response
.
body
).
last
expect
(
match_line
[
'type'
]).
to
eq
(
'context'
)
expect
(
match_line
[
'type'
]).
to
be_nil
end
end
end
...
...
spec/features/merge_request/user_posts_diff_notes_spec.rb
View file @
ec66ef1b
...
...
@@ -85,12 +85,13 @@ describe 'Merge request > User posts diff notes', :js do
# `.line_holder` will be an unfolded line.
let
(
:line_holder
)
{
first
(
'#a5cc2925ca8258af241be7e5b0381edf30266302 .line_holder'
)
}
it
'
does not allow
commenting on the left side'
do
should_
not_
allow_commenting
(
line_holder
,
'left'
)
it
'
allows
commenting on the left side'
do
should_allow_commenting
(
line_holder
,
'left'
)
end
it
'does not allow commenting on the right side'
do
should_not_allow_commenting
(
line_holder
,
'right'
)
it
'allows commenting on the right side'
do
# Automatically shifts comment box to left side.
should_allow_commenting
(
line_holder
,
'right'
)
end
end
end
...
...
@@ -147,8 +148,8 @@ describe 'Merge request > User posts diff notes', :js do
# `.line_holder` will be an unfolded line.
let
(
:line_holder
)
{
first
(
'.line_holder[id="a5cc2925ca8258af241be7e5b0381edf30266302_1_1"]'
)
}
it
'
does not allow
commenting'
do
should_
not_
allow_commenting
line_holder
it
'
allows
commenting'
do
should_allow_commenting
line_holder
end
end
...
...
spec/javascripts/diffs/store/mutations_spec.js
View file @
ec66ef1b
...
...
@@ -98,7 +98,7 @@ describe('DiffsStoreMutations', () => {
it
(
'
should call utils.addContextLines with proper params
'
,
()
=>
{
const
options
=
{
lineNumbers
:
{
oldLineNumber
:
1
,
newLineNumber
:
2
},
contextLines
:
[{
oldLine
:
1
}],
contextLines
:
[{
oldLine
:
1
,
newLine
:
1
,
lineCode
:
'
ff9200_1_1
'
,
discussions
:
[]
}],
fileHash
:
'
ff9200
'
,
params
:
{
bottom
:
true
,
...
...
@@ -110,7 +110,7 @@ describe('DiffsStoreMutations', () => {
parallelDiffLines
:
[],
};
const
state
=
{
diffFiles
:
[
diffFile
]
};
const
lines
=
[{
oldLine
:
1
}];
const
lines
=
[{
oldLine
:
1
,
newLine
:
1
}];
const
findDiffFileSpy
=
spyOnDependency
(
mutations
,
'
findDiffFile
'
).
and
.
returnValue
(
diffFile
);
const
removeMatchLineSpy
=
spyOnDependency
(
mutations
,
'
removeMatchLine
'
);
...
...
spec/lib/gitlab/diff/file_spec.rb
View file @
ec66ef1b
...
...
@@ -41,6 +41,52 @@ describe Gitlab::Diff::File do
end
end
describe
'#unfold_diff_lines'
do
let
(
:unfolded_lines
)
{
double
(
'expanded-lines'
)
}
let
(
:unfolder
)
{
instance_double
(
Gitlab
::
Diff
::
LinesUnfolder
)
}
let
(
:position
)
{
instance_double
(
Gitlab
::
Diff
::
Position
,
old_line:
10
)
}
before
do
allow
(
Gitlab
::
Diff
::
LinesUnfolder
).
to
receive
(
:new
)
{
unfolder
}
end
context
'when unfold required'
do
before
do
allow
(
unfolder
).
to
receive
(
:unfold_required?
)
{
true
}
allow
(
unfolder
).
to
receive
(
:unfolded_diff_lines
)
{
unfolded_lines
}
end
it
'changes @unfolded to true'
do
diff_file
.
unfold_diff_lines
(
position
)
expect
(
diff_file
).
to
be_unfolded
end
it
'updates @diff_lines'
do
diff_file
.
unfold_diff_lines
(
position
)
expect
(
diff_file
.
diff_lines
).
to
eq
(
unfolded_lines
)
end
end
context
'when unfold not required'
do
before
do
allow
(
unfolder
).
to
receive
(
:unfold_required?
)
{
false
}
end
it
'keeps @unfolded false'
do
diff_file
.
unfold_diff_lines
(
position
)
expect
(
diff_file
).
not_to
be_unfolded
end
it
'does not update @diff_lines'
do
expect
{
diff_file
.
unfold_diff_lines
(
position
)
}
.
not_to
change
(
diff_file
,
:diff_lines
)
end
end
end
describe
'#mode_changed?'
do
it
{
expect
(
diff_file
.
mode_changed?
).
to
be_falsey
}
end
...
...
spec/lib/gitlab/diff/lines_unfolder_spec.rb
0 → 100644
View file @
ec66ef1b
This diff is collapsed.
Click to expand it.
spec/services/merge_requests/reload_diffs_service_spec.rb
View file @
ec66ef1b
...
...
@@ -31,32 +31,11 @@ describe MergeRequests::ReloadDiffsService, :use_clean_rails_memory_store_cachin
end
context
'cache clearing'
do
before
do
allow_any_instance_of
(
Gitlab
::
Diff
::
File
).
to
receive
(
:text?
).
and_return
(
true
)
allow_any_instance_of
(
Gitlab
::
Diff
::
File
).
to
receive
(
:diffable?
).
and_return
(
true
)
end
it
'retrieves the diff files to cache the highlighted result'
do
new_diff
=
merge_request
.
create_merge_request_diff
cache_key
=
new_diff
.
diffs_collection
.
cache_key
expect
(
merge_request
).
to
receive
(
:create_merge_request_diff
).
and_return
(
new_diff
)
expect
(
Rails
.
cache
).
to
receive
(
:read
).
with
(
cache_key
).
and_call_original
expect
(
Rails
.
cache
).
to
receive
(
:write
).
with
(
cache_key
,
anything
,
anything
).
and_call_original
subject
.
execute
end
it
'clears the cache for older diffs on the merge request'
do
old_diff
=
merge_request
.
merge_request_diff
old_cache_key
=
old_diff
.
diffs_collection
.
cache_key
new_diff
=
merge_request
.
create_merge_request_diff
new_cache_key
=
new_diff
.
diffs_collection
.
cache_key
expect
(
merge_request
).
to
receive
(
:create_merge_request_diff
).
and_return
(
new_diff
)
expect
(
Rails
.
cache
).
to
receive
(
:delete
).
with
(
old_cache_key
).
and_call_original
expect
(
Rails
.
cache
).
to
receive
(
:read
).
with
(
new_cache_key
).
and_call_original
expect
(
Rails
.
cache
).
to
receive
(
:write
).
with
(
new_cache_key
,
anything
,
anything
).
and_call_original
subject
.
execute
end
...
...
spec/services/notes/create_service_spec.rb
View file @
ec66ef1b
...
...
@@ -57,6 +57,57 @@ describe Notes::CreateService do
end
end
context
'noteable highlight cache clearing'
do
let
(
:project_with_repo
)
{
create
(
:project
,
:repository
)
}
let
(
:merge_request
)
do
create
(
:merge_request
,
source_project:
project_with_repo
,
target_project:
project_with_repo
)
end
let
(
:position
)
do
Gitlab
::
Diff
::
Position
.
new
(
old_path:
"files/ruby/popen.rb"
,
new_path:
"files/ruby/popen.rb"
,
old_line:
nil
,
new_line:
14
,
diff_refs:
merge_request
.
diff_refs
)
end
let
(
:new_opts
)
do
opts
.
merge
(
in_reply_to_discussion_id:
nil
,
type:
'DiffNote'
,
noteable_type:
'MergeRequest'
,
noteable_id:
merge_request
.
id
,
position:
position
.
to_h
)
end
before
do
allow_any_instance_of
(
Gitlab
::
Diff
::
Position
)
.
to
receive
(
:unfolded_diff?
)
{
true
}
end
it
'clears noteable diff cache when it was unfolded for the note position'
do
expect_any_instance_of
(
Gitlab
::
Diff
::
HighlightCache
).
to
receive
(
:clear
)
described_class
.
new
(
project_with_repo
,
user
,
new_opts
).
execute
end
it
'does not clear cache when note is not the first of the discussion'
do
prev_note
=
create
(
:diff_note_on_merge_request
,
noteable:
merge_request
,
project:
project_with_repo
)
reply_opts
=
opts
.
merge
(
in_reply_to_discussion_id:
prev_note
.
discussion_id
,
type:
'DiffNote'
,
noteable_type:
'MergeRequest'
,
noteable_id:
merge_request
.
id
,
position:
position
.
to_h
)
expect
(
merge_request
).
not_to
receive
(
:diffs
)
described_class
.
new
(
project_with_repo
,
user
,
reply_opts
).
execute
end
end
context
'note diff file'
do
let
(
:project_with_repo
)
{
create
(
:project
,
:repository
)
}
let
(
:merge_request
)
do
...
...
spec/services/notes/destroy_service_spec.rb
View file @
ec66ef1b
...
...
@@ -21,5 +21,38 @@ describe Notes::DestroyService do
expect
{
described_class
.
new
(
project
,
user
).
execute
(
note
)
}
.
to
change
{
user
.
todos_pending_count
}.
from
(
1
).
to
(
0
)
end
context
'noteable highlight cache clearing'
do
let
(
:repo_project
)
{
create
(
:project
,
:repository
)
}
let
(
:merge_request
)
do
create
(
:merge_request
,
source_project:
repo_project
,
target_project:
repo_project
)
end
let
(
:note
)
do
create
(
:diff_note_on_merge_request
,
project:
repo_project
,
noteable:
merge_request
)
end
before
do
allow
(
note
.
position
).
to
receive
(
:unfolded_diff?
)
{
true
}
end
it
'clears noteable diff cache when it was unfolded for the note position'
do
expect
(
merge_request
).
to
receive_message_chain
(
:diffs
,
:clear_cache
)
described_class
.
new
(
repo_project
,
user
).
execute
(
note
)
end
it
'does not clear cache when note is not the first of the discussion'
do
reply_note
=
create
(
:diff_note_on_merge_request
,
in_reply_to:
note
,
project:
repo_project
,
noteable:
merge_request
)
expect
(
merge_request
).
not_to
receive
(
:diffs
)
described_class
.
new
(
repo_project
,
user
).
execute
(
reply_note
)
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