summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMike Greiling <mike@pixelcog.com>2018-09-07 23:27:24 +0000
committerMike Greiling <mike@pixelcog.com>2018-09-07 23:27:24 +0000
commit6d1b5850342a91ebdff07bb57722ce28e31b773f (patch)
treea3e36284caef0fd582ca9ed1cd3012085657ba3c
parentd7c694b6e979c6ed570fd1981f78b39d0b0a5fcb (diff)
parentf32ec46ede6198ad3bbc6f57d404a8a8e22cd008 (diff)
downloadgitlab-ce-6d1b5850342a91ebdff07bb57722ce28e31b773f.tar.gz
Merge branch 'tz-mr-new-datastructure' into 'master'
Vue MR Page - Data structure update See merge request gitlab-org/gitlab-ce!21062
-rw-r--r--app/assets/javascripts/diff_notes/components/resolve_discussion_btn.js22
-rw-r--r--app/assets/javascripts/diff_notes/services/resolve.js9
-rw-r--r--app/assets/javascripts/diffs/components/app.vue29
-rw-r--r--app/assets/javascripts/diffs/components/diff_discussions.vue10
-rw-r--r--app/assets/javascripts/diffs/components/diff_file.vue35
-rw-r--r--app/assets/javascripts/diffs/components/diff_line_gutter_content.vue39
-rw-r--r--app/assets/javascripts/diffs/components/diff_line_note_form.vue8
-rw-r--r--app/assets/javascripts/diffs/components/diff_table_cell.vue37
-rw-r--r--app/assets/javascripts/diffs/components/inline_diff_comment_row.vue11
-rw-r--r--app/assets/javascripts/diffs/components/inline_diff_table_row.vue7
-rw-r--r--app/assets/javascripts/diffs/components/inline_diff_view.vue21
-rw-r--r--app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue50
-rw-r--r--app/assets/javascripts/diffs/components/parallel_diff_table_row.vue119
-rw-r--r--app/assets/javascripts/diffs/components/parallel_diff_view.vue41
-rw-r--r--app/assets/javascripts/diffs/store/actions.js54
-rw-r--r--app/assets/javascripts/diffs/store/getters.js55
-rw-r--r--app/assets/javascripts/diffs/store/mutation_types.js2
-rw-r--r--app/assets/javascripts/diffs/store/mutations.js116
-rw-r--r--app/assets/javascripts/diffs/store/utils.js43
-rw-r--r--app/assets/javascripts/notes.js53
-rw-r--r--app/assets/javascripts/notes/components/noteable_discussion.vue17
-rw-r--r--app/assets/javascripts/notes/components/noteable_note.vue1
-rw-r--r--app/assets/javascripts/notes/components/notes_app.vue1
-rw-r--r--app/assets/javascripts/notes/stores/actions.js63
-rw-r--r--app/assets/javascripts/notes/stores/getters.js14
-rw-r--r--app/assets/javascripts/notes/stores/mutations.js27
-rw-r--r--app/assets/javascripts/notes/stores/utils.js21
-rw-r--r--spec/features/merge_request/user_posts_diff_notes_spec.rb7
-rw-r--r--spec/javascripts/diffs/components/diff_file_spec.js4
-rw-r--r--spec/javascripts/diffs/components/diff_line_gutter_content_spec.js46
-rw-r--r--spec/javascripts/diffs/components/parallel_diff_view_spec.js8
-rw-r--r--spec/javascripts/diffs/mock_data/diff_file.js16
-rw-r--r--spec/javascripts/diffs/store/actions_spec.js194
-rw-r--r--spec/javascripts/diffs/store/getters_spec.js78
-rw-r--r--spec/javascripts/diffs/store/mutations_spec.js106
-rw-r--r--spec/javascripts/diffs/store/utils_spec.js44
-rw-r--r--spec/javascripts/lazy_loader_spec.js12
37 files changed, 890 insertions, 530 deletions
diff --git a/app/assets/javascripts/diff_notes/components/resolve_discussion_btn.js b/app/assets/javascripts/diff_notes/components/resolve_discussion_btn.js
index 5ed13488788..6fcad187b35 100644
--- a/app/assets/javascripts/diff_notes/components/resolve_discussion_btn.js
+++ b/app/assets/javascripts/diff_notes/components/resolve_discussion_btn.js
@@ -1,4 +1,4 @@
-/* eslint-disable object-shorthand, func-names, comma-dangle, no-else-return, quotes */
+/* eslint-disable object-shorthand, func-names, no-else-return */
/* global CommentsStore */
/* global ResolveService */
@@ -25,44 +25,44 @@ const ResolveDiscussionBtn = Vue.extend({
};
},
computed: {
- showButton: function () {
+ showButton: function() {
if (this.discussion) {
return this.discussion.isResolvable();
} else {
return false;
}
},
- isDiscussionResolved: function () {
+ isDiscussionResolved: function() {
if (this.discussion) {
return this.discussion.isResolved();
} else {
return false;
}
},
- buttonText: function () {
+ buttonText: function() {
if (this.isDiscussionResolved) {
- return "Unresolve discussion";
+ return 'Unresolve discussion';
} else {
- return "Resolve discussion";
+ return 'Resolve discussion';
}
},
- loading: function () {
+ loading: function() {
if (this.discussion) {
return this.discussion.loading;
} else {
return false;
}
- }
+ },
},
- created: function () {
+ created: function() {
CommentsStore.createDiscussion(this.discussionId, this.canResolve);
this.discussion = CommentsStore.state[this.discussionId];
},
methods: {
- resolve: function () {
+ resolve: function() {
ResolveService.toggleResolveForDiscussion(this.mergeRequestId, this.discussionId);
- }
+ },
},
});
diff --git a/app/assets/javascripts/diff_notes/services/resolve.js b/app/assets/javascripts/diff_notes/services/resolve.js
index 0b3568e432d..e69eaad4423 100644
--- a/app/assets/javascripts/diff_notes/services/resolve.js
+++ b/app/assets/javascripts/diff_notes/services/resolve.js
@@ -8,9 +8,7 @@ window.gl = window.gl || {};
class ResolveServiceClass {
constructor(root) {
- this.noteResource = Vue.resource(
- `${root}/notes{/noteId}/resolve?html=true`,
- );
+ this.noteResource = Vue.resource(`${root}/notes{/noteId}/resolve?html=true`);
this.discussionResource = Vue.resource(
`${root}/merge_requests{/mergeRequestId}/discussions{/discussionId}/resolve?html=true`,
);
@@ -51,10 +49,7 @@ class ResolveServiceClass {
discussion.updateHeadline(data);
})
.catch(
- () =>
- new Flash(
- 'An error occurred when trying to resolve a discussion. Please try again.',
- ),
+ () => new Flash('An error occurred when trying to resolve a discussion. Please try again.'),
);
}
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue
index b5b05df4d34..4261a99c52b 100644
--- a/app/assets/javascripts/diffs/components/app.vue
+++ b/app/assets/javascripts/diffs/components/app.vue
@@ -59,7 +59,7 @@ export default {
emailPatchPath: state => state.diffs.emailPatchPath,
}),
...mapGetters('diffs', ['isParallelView']),
- ...mapGetters(['isNotesFetched']),
+ ...mapGetters(['isNotesFetched', 'discussionsStructuredByLineCode']),
targetBranch() {
return {
branchName: this.targetBranchName,
@@ -112,13 +112,26 @@ export default {
},
created() {
this.adjustView();
+ eventHub.$once('fetchedNotesData', this.setDiscussions);
},
methods: {
- ...mapActions('diffs', ['setBaseConfig', 'fetchDiffFiles', 'startRenderDiffsQueue']),
+ ...mapActions('diffs', [
+ 'setBaseConfig',
+ 'fetchDiffFiles',
+ 'startRenderDiffsQueue',
+ 'assignDiscussionsToDiff',
+ ]),
+
fetchData() {
this.fetchDiffFiles()
.then(() => {
- requestIdleCallback(this.startRenderDiffsQueue, { timeout: 1000 });
+ requestIdleCallback(
+ () => {
+ this.setDiscussions();
+ this.startRenderDiffsQueue();
+ },
+ { timeout: 1000 },
+ );
})
.catch(() => {
createFlash(__('Something went wrong on our end. Please try again!'));
@@ -128,6 +141,16 @@ export default {
eventHub.$emit('fetchNotesData');
}
},
+ setDiscussions() {
+ if (this.isNotesFetched) {
+ requestIdleCallback(
+ () => {
+ this.assignDiscussionsToDiff(this.discussionsStructuredByLineCode);
+ },
+ { timeout: 1000 },
+ );
+ }
+ },
adjustView() {
if (this.shouldShow && this.isParallelView) {
window.mrTabs.expandViewContainer();
diff --git a/app/assets/javascripts/diffs/components/diff_discussions.vue b/app/assets/javascripts/diffs/components/diff_discussions.vue
index e64d5511d78..cddbe554fbd 100644
--- a/app/assets/javascripts/diffs/components/diff_discussions.vue
+++ b/app/assets/javascripts/diffs/components/diff_discussions.vue
@@ -1,4 +1,5 @@
<script>
+import { mapActions } from 'vuex';
import noteableDiscussion from '../../notes/components/noteable_discussion.vue';
export default {
@@ -11,6 +12,14 @@ export default {
required: true,
},
},
+ methods: {
+ ...mapActions('diffs', ['removeDiscussionsFromDiff']),
+ deleteNoteHandler(discussion) {
+ if (discussion.notes.length <= 1) {
+ this.removeDiscussionsFromDiff(discussion);
+ }
+ },
+ },
};
</script>
@@ -31,6 +40,7 @@ export default {
:render-diff-file="false"
:always-expanded="true"
:discussions-by-diff-order="true"
+ @noteDeleted="deleteNoteHandler"
/>
</ul>
</div>
diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue
index 59e9ba08b8b..67e85c4eee3 100644
--- a/app/assets/javascripts/diffs/components/diff_file.vue
+++ b/app/assets/javascripts/diffs/components/diff_file.vue
@@ -1,5 +1,5 @@
<script>
-import { mapActions } from 'vuex';
+import { mapActions, mapGetters } from 'vuex';
import _ from 'underscore';
import { __, sprintf } from '~/locale';
import createFlash from '~/flash';
@@ -30,6 +30,7 @@ export default {
};
},
computed: {
+ ...mapGetters(['isNotesFetched', 'discussionsStructuredByLineCode']),
isCollapsed() {
return this.file.collapsed || false;
},
@@ -44,23 +45,23 @@ export default {
);
},
showExpandMessage() {
- return this.isCollapsed && !this.isLoadingCollapsedDiff && !this.file.tooLarge;
+ return (
+ !this.isCollapsed &&
+ !this.file.highlightedDiffLines &&
+ !this.isLoadingCollapsedDiff &&
+ !this.file.tooLarge &&
+ this.file.text
+ );
},
showLoadingIcon() {
return this.isLoadingCollapsedDiff || (!this.file.renderIt && !this.isCollapsed);
},
},
methods: {
- ...mapActions('diffs', ['loadCollapsedDiff']),
+ ...mapActions('diffs', ['loadCollapsedDiff', 'assignDiscussionsToDiff']),
handleToggle() {
- const { collapsed, highlightedDiffLines, parallelDiffLines } = this.file;
-
- if (
- collapsed &&
- !highlightedDiffLines &&
- parallelDiffLines !== undefined &&
- !parallelDiffLines.length
- ) {
+ const { highlightedDiffLines, parallelDiffLines } = this.file;
+ if (!highlightedDiffLines && parallelDiffLines !== undefined && !parallelDiffLines.length) {
this.handleLoadCollapsedDiff();
} else {
this.file.collapsed = !this.file.collapsed;
@@ -76,6 +77,14 @@ export default {
this.file.collapsed = false;
this.file.renderIt = true;
})
+ .then(() => {
+ requestIdleCallback(
+ () => {
+ this.assignDiscussionsToDiff(this.discussionsStructuredByLineCode);
+ },
+ { timeout: 1000 },
+ );
+ })
.catch(() => {
this.isLoadingCollapsedDiff = false;
createFlash(__('Something went wrong on our end. Please try again!'));
@@ -136,11 +145,11 @@ export default {
:diff-file="file"
/>
<loading-icon
- v-else-if="showLoadingIcon"
+ v-if="showLoadingIcon"
class="diff-content loading"
/>
<div
- v-if="showExpandMessage"
+ v-else-if="showExpandMessage"
class="nothing-here-block diff-collapsed"
>
{{ __('This diff is collapsed.') }}
diff --git a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue
index 8ad1ea34245..6eff3013dcd 100644
--- a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue
+++ b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue
@@ -13,6 +13,10 @@ export default {
Icon,
},
props: {
+ line: {
+ type: Object,
+ required: true,
+ },
fileHash: {
type: String,
required: true,
@@ -21,31 +25,16 @@ export default {
type: String,
required: true,
},
- lineType: {
- type: String,
- required: false,
- default: '',
- },
lineNumber: {
type: Number,
required: false,
default: 0,
},
- lineCode: {
- type: String,
- required: false,
- default: '',
- },
linePosition: {
type: String,
required: false,
default: '',
},
- metaData: {
- type: Object,
- required: false,
- default: () => ({}),
- },
showCommentButton: {
type: Boolean,
required: false,
@@ -76,11 +65,6 @@ export default {
required: false,
default: false,
},
- discussions: {
- type: Array,
- required: false,
- default: () => [],
- },
},
computed: {
...mapState({
@@ -89,7 +73,7 @@ export default {
}),
...mapGetters(['isLoggedIn']),
lineHref() {
- return this.lineCode ? `#${this.lineCode}` : '#';
+ return `#${this.line.lineCode || ''}`;
},
shouldShowCommentButton() {
return (
@@ -103,20 +87,19 @@ export default {
);
},
hasDiscussions() {
- return this.discussions.length > 0;
+ return this.line.discussions && this.line.discussions.length > 0;
},
shouldShowAvatarsOnGutter() {
- if (!this.lineType && this.linePosition === LINE_POSITION_RIGHT) {
+ if (!this.line.type && this.linePosition === LINE_POSITION_RIGHT) {
return false;
}
-
return this.showCommentButton && this.hasDiscussions;
},
},
methods: {
...mapActions('diffs', ['loadMoreLines', 'showCommentForm']),
handleCommentButton() {
- this.showCommentForm({ lineCode: this.lineCode });
+ this.showCommentForm({ lineCode: this.line.lineCode });
},
handleLoadMoreLines() {
if (this.isRequesting) {
@@ -125,8 +108,8 @@ export default {
this.isRequesting = true;
const endpoint = this.contextLinesPath;
- const oldLineNumber = this.metaData.oldPos || 0;
- const newLineNumber = this.metaData.newPos || 0;
+ const oldLineNumber = this.line.metaData.oldPos || 0;
+ const newLineNumber = this.line.metaData.newPos || 0;
const offset = newLineNumber - oldLineNumber;
const bottom = this.isBottom;
const { fileHash } = this;
@@ -201,7 +184,7 @@ export default {
</a>
<diff-gutter-avatars
v-if="shouldShowAvatarsOnGutter"
- :discussions="discussions"
+ :discussions="line.discussions"
/>
</template>
</div>
diff --git a/app/assets/javascripts/diffs/components/diff_line_note_form.vue b/app/assets/javascripts/diffs/components/diff_line_note_form.vue
index cbe4551d06b..a0dc381ccc7 100644
--- a/app/assets/javascripts/diffs/components/diff_line_note_form.vue
+++ b/app/assets/javascripts/diffs/components/diff_line_note_form.vue
@@ -6,6 +6,7 @@ import noteForm from '../../notes/components/note_form.vue';
import { getNoteFormData } from '../store/utils';
import autosave from '../../notes/mixins/autosave';
import { DIFF_NOTE_TYPE } from '../constants';
+import { reduceDiscussionsToLineCodes } from '../../notes/stores/utils';
export default {
components: {
@@ -52,7 +53,7 @@ export default {
}
},
methods: {
- ...mapActions('diffs', ['cancelCommentForm']),
+ ...mapActions('diffs', ['cancelCommentForm', 'assignDiscussionsToDiff']),
...mapActions(['saveNote', 'refetchDiscussionById']),
handleCancelCommentForm(shouldConfirm, isDirty) {
if (shouldConfirm && isDirty) {
@@ -88,7 +89,10 @@ export default {
const endpoint = this.getNotesDataByProp('discussionsPath');
this.refetchDiscussionById({ path: endpoint, discussionId: result.discussion_id })
- .then(() => {
+ .then(selectedDiscussion => {
+ const lineCodeDiscussions = reduceDiscussionsToLineCodes([selectedDiscussion]);
+ this.assignDiscussionsToDiff(lineCodeDiscussions);
+
this.handleCancelCommentForm();
})
.catch(() => {
diff --git a/app/assets/javascripts/diffs/components/diff_table_cell.vue b/app/assets/javascripts/diffs/components/diff_table_cell.vue
index 33bc8d9971e..5d9a0b123fe 100644
--- a/app/assets/javascripts/diffs/components/diff_table_cell.vue
+++ b/app/assets/javascripts/diffs/components/diff_table_cell.vue
@@ -11,8 +11,6 @@ import {
LINE_HOVER_CLASS_NAME,
LINE_UNFOLD_CLASS_NAME,
INLINE_DIFF_VIEW_TYPE,
- LINE_POSITION_LEFT,
- LINE_POSITION_RIGHT,
} from '../constants';
export default {
@@ -67,42 +65,24 @@ export default {
required: false,
default: false,
},
- discussions: {
- type: Array,
- required: false,
- default: () => [],
- },
},
computed: {
...mapGetters(['isLoggedIn']),
- normalizedLine() {
- let normalizedLine;
-
- if (this.diffViewType === INLINE_DIFF_VIEW_TYPE) {
- normalizedLine = this.line;
- } else if (this.linePosition === LINE_POSITION_LEFT) {
- normalizedLine = this.line.left;
- } else if (this.linePosition === LINE_POSITION_RIGHT) {
- normalizedLine = this.line.right;
- }
-
- return normalizedLine;
- },
isMatchLine() {
- return this.normalizedLine.type === MATCH_LINE_TYPE;
+ return this.line.type === MATCH_LINE_TYPE;
},
isContextLine() {
- return this.normalizedLine.type === CONTEXT_LINE_TYPE;
+ return this.line.type === CONTEXT_LINE_TYPE;
},
isMetaLine() {
- const { type } = this.normalizedLine;
+ const { type } = this.line;
return (
type === OLD_NO_NEW_LINE_TYPE || type === NEW_NO_NEW_LINE_TYPE || type === EMPTY_CELL_TYPE
);
},
classNameMap() {
- const { type } = this.normalizedLine;
+ const { type } = this.line;
return {
[type]: type,
@@ -116,9 +96,9 @@ export default {
};
},
lineNumber() {
- const { lineType, normalizedLine } = this;
+ const { lineType } = this;
- return lineType === OLD_LINE_TYPE ? normalizedLine.oldLine : normalizedLine.newLine;
+ return lineType === OLD_LINE_TYPE ? this.line.oldLine : this.line.newLine;
},
},
};
@@ -129,20 +109,17 @@ export default {
:class="classNameMap"
>
<diff-line-gutter-content
+ :line="line"
:file-hash="fileHash"
:context-lines-path="contextLinesPath"
- :line-type="normalizedLine.type"
- :line-code="normalizedLine.lineCode"
:line-position="linePosition"
:line-number="lineNumber"
- :meta-data="normalizedLine.metaData"
:show-comment-button="showCommentButton"
:is-hover="isHover"
:is-bottom="isBottom"
:is-match-line="isMatchLine"
:is-context-line="isContentLine"
:is-meta-line="isMetaLine"
- :discussions="discussions"
/>
</td>
</template>
diff --git a/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue b/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue
index 6348f32d36d..46a51859da5 100644
--- a/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue
+++ b/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue
@@ -21,18 +21,13 @@ export default {
type: Number,
required: true,
},
- discussions: {
- type: Array,
- required: false,
- default: () => [],
- },
},
computed: {
...mapState({
diffLineCommentForms: state => state.diffs.diffLineCommentForms,
}),
className() {
- return this.discussions.length ? '' : 'js-temp-notes-holder';
+ return this.line.discussions.length ? '' : 'js-temp-notes-holder';
},
},
};
@@ -49,8 +44,8 @@ export default {
>
<div class="content">
<diff-discussions
- v-if="discussions.length"
- :discussions="discussions"
+ v-if="line.discussions.length"
+ :discussions="line.discussions"
/>
<diff-line-note-form
v-if="diffLineCommentForms[line.lineCode]"
diff --git a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue
index 32d65ff994f..0e306f39a9f 100644
--- a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue
+++ b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue
@@ -33,11 +33,6 @@ export default {
required: false,
default: false,
},
- discussions: {
- type: Array,
- required: false,
- default: () => [],
- },
},
data() {
return {
@@ -94,7 +89,6 @@ export default {
:is-bottom="isBottom"
:is-hover="isHover"
:show-comment-button="true"
- :discussions="discussions"
class="diff-line-num old_line"
/>
<diff-table-cell
@@ -104,7 +98,6 @@ export default {
:line-type="newLineType"
:is-bottom="isBottom"
:is-hover="isHover"
- :discussions="discussions"
class="diff-line-num new_line"
/>
<td
diff --git a/app/assets/javascripts/diffs/components/inline_diff_view.vue b/app/assets/javascripts/diffs/components/inline_diff_view.vue
index e7d789734c3..947e7c98fae 100644
--- a/app/assets/javascripts/diffs/components/inline_diff_view.vue
+++ b/app/assets/javascripts/diffs/components/inline_diff_view.vue
@@ -2,7 +2,6 @@
import { mapGetters, mapState } from 'vuex';
import inlineDiffTableRow from './inline_diff_table_row.vue';
import inlineDiffCommentRow from './inline_diff_comment_row.vue';
-import { trimFirstCharOfLineContent } from '../store/utils';
export default {
components: {
@@ -20,29 +19,17 @@ export default {
},
},
computed: {
- ...mapGetters('diffs', [
- 'commitId',
- 'shouldRenderInlineCommentRow',
- 'singleDiscussionByLineCode',
- ]),
+ ...mapGetters('diffs', ['commitId', 'shouldRenderInlineCommentRow']),
...mapState({
diffLineCommentForms: state => state.diffs.diffLineCommentForms,
}),
- normalizedDiffLines() {
- return this.diffLines.map(line => (line.richText ? trimFirstCharOfLineContent(line) : line));
- },
diffLinesLength() {
- return this.normalizedDiffLines.length;
+ return this.diffLines.length;
},
userColorScheme() {
return window.gon.user_color_scheme;
},
},
- methods: {
- discussionsList(line) {
- return line.lineCode !== undefined ? this.singleDiscussionByLineCode(line.lineCode) : [];
- },
- },
};
</script>
@@ -53,7 +40,7 @@ export default {
class="code diff-wrap-lines js-syntax-highlight text-file js-diff-inline-view">
<tbody>
<template
- v-for="(line, index) in normalizedDiffLines"
+ v-for="(line, index) in diffLines"
>
<inline-diff-table-row
:file-hash="diffFile.fileHash"
@@ -61,7 +48,6 @@ export default {
:line="line"
:is-bottom="index + 1 === diffLinesLength"
:key="line.lineCode"
- :discussions="discussionsList(line)"
/>
<inline-diff-comment-row
v-if="shouldRenderInlineCommentRow(line)"
@@ -69,7 +55,6 @@ export default {
:line="line"
:line-index="index"
:key="index"
- :discussions="discussionsList(line)"
/>
</template>
</tbody>
diff --git a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue
index 48b8feeb0b4..26417c350cb 100644
--- a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue
+++ b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue
@@ -21,51 +21,49 @@ export default {
type: Number,
required: true,
},
- leftDiscussions: {
- type: Array,
- required: false,
- default: () => [],
- },
- rightDiscussions: {
- type: Array,
- required: false,
- default: () => [],
- },
},
computed: {
...mapState({
diffLineCommentForms: state => state.diffs.diffLineCommentForms,
}),
leftLineCode() {
- return this.line.left.lineCode;
+ return this.line.left && this.line.left.lineCode;
},
rightLineCode() {
- return this.line.right.lineCode;
+ return this.line.right && this.line.right.lineCode;
},
hasExpandedDiscussionOnLeft() {
- const discussions = this.leftDiscussions;
-
- return discussions ? discussions.every(discussion => discussion.expanded) : false;
+ return this.line.left && this.line.left.discussions
+ ? this.line.left.discussions.every(discussion => discussion.expanded)
+ : false;
},
hasExpandedDiscussionOnRight() {
- const discussions = this.rightDiscussions;
-
- return discussions ? discussions.every(discussion => discussion.expanded) : false;
+ return this.line.right && this.line.right.discussions
+ ? this.line.right.discussions.every(discussion => discussion.expanded)
+ : false;
},
hasAnyExpandedDiscussion() {
return this.hasExpandedDiscussionOnLeft || this.hasExpandedDiscussionOnRight;
},
shouldRenderDiscussionsOnLeft() {
- return this.leftDiscussions && this.hasExpandedDiscussionOnLeft;
+ return this.line.left && this.line.left.discussions && this.hasExpandedDiscussionOnLeft;
},
shouldRenderDiscussionsOnRight() {
- return this.rightDiscussions && this.hasExpandedDiscussionOnRight && this.line.right.type;
+ return (
+ this.line.right &&
+ this.line.right.discussions &&
+ this.hasExpandedDiscussionOnRight &&
+ this.line.right.type
+ );
},
showRightSideCommentForm() {
- return this.line.right.type && this.diffLineCommentForms[this.rightLineCode];
+ return (
+ this.line.right && this.line.right.type && this.diffLineCommentForms[this.rightLineCode]
+ );
},
className() {
- return this.leftDiscussions.length > 0 || this.rightDiscussions.length > 0
+ return (this.left && this.line.left.discussions.length > 0) ||
+ (this.right && this.line.right.discussions.length > 0)
? ''
: 'js-temp-notes-holder';
},
@@ -85,8 +83,8 @@ export default {
class="content"
>
<diff-discussions
- v-if="leftDiscussions.length"
- :discussions="leftDiscussions"
+ v-if="line.left.discussions.length"
+ :discussions="line.left.discussions"
/>
</div>
<diff-line-note-form
@@ -104,8 +102,8 @@ export default {
class="content"
>
<diff-discussions
- v-if="rightDiscussions.length"
- :discussions="rightDiscussions"
+ v-if="line.right.discussions.length"
+ :discussions="line.right.discussions"
/>
</div>
<diff-line-note-form
diff --git a/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue
index d4e54c2bd00..fb68d191091 100644
--- a/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue
+++ b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue
@@ -1,6 +1,5 @@
<script>
import $ from 'jquery';
-import { mapGetters } from 'vuex';
import DiffTableCell from './diff_table_cell.vue';
import {
NEW_LINE_TYPE,
@@ -10,8 +9,7 @@ import {
OLD_NO_NEW_LINE_TYPE,
PARALLEL_DIFF_VIEW_TYPE,
NEW_NO_NEW_LINE_TYPE,
- LINE_POSITION_LEFT,
- LINE_POSITION_RIGHT,
+ EMPTY_CELL_TYPE,
} from '../constants';
export default {
@@ -36,16 +34,6 @@ export default {
required: false,
default: false,
},
- leftDiscussions: {
- type: Array,
- required: false,
- default: () => [],
- },
- rightDiscussions: {
- type: Array,
- required: false,
- default: () => [],
- },
},
data() {
return {
@@ -54,29 +42,26 @@ export default {
};
},
computed: {
- ...mapGetters('diffs', ['isParallelView']),
isContextLine() {
- return this.line.left.type === CONTEXT_LINE_TYPE;
+ return this.line.left && this.line.left.type === CONTEXT_LINE_TYPE;
},
classNameMap() {
return {
[CONTEXT_LINE_CLASS_NAME]: this.isContextLine,
- [PARALLEL_DIFF_VIEW_TYPE]: this.isParallelView,
+ [PARALLEL_DIFF_VIEW_TYPE]: true,
};
},
parallelViewLeftLineType() {
- if (this.line.right.type === NEW_NO_NEW_LINE_TYPE) {
+ if (this.line.right && this.line.right.type === NEW_NO_NEW_LINE_TYPE) {
return OLD_NO_NEW_LINE_TYPE;
}
- return this.line.left.type;
+ return this.line.left ? this.line.left.type : EMPTY_CELL_TYPE;
},
},
created() {
this.newLineType = NEW_LINE_TYPE;
this.oldLineType = OLD_LINE_TYPE;
- this.linePositionLeft = LINE_POSITION_LEFT;
- this.linePositionRight = LINE_POSITION_RIGHT;
this.parallelDiffViewType = PARALLEL_DIFF_VIEW_TYPE;
},
methods: {
@@ -116,47 +101,57 @@ export default {
@mouseover="handleMouseMove"
@mouseout="handleMouseMove"
>
- <diff-table-cell
- :file-hash="fileHash"
- :context-lines-path="contextLinesPath"
- :line="line"
- :line-type="oldLineType"
- :line-position="linePositionLeft"
- :is-bottom="isBottom"
- :is-hover="isLeftHover"
- :show-comment-button="true"
- :diff-view-type="parallelDiffViewType"
- :discussions="leftDiscussions"
- class="diff-line-num old_line"
- />
- <td
- :id="line.left.lineCode"
- :class="parallelViewLeftLineType"
- class="line_content parallel left-side"
- @mousedown.native="handleParallelLineMouseDown"
- v-html="line.left.richText"
- >
- </td>
- <diff-table-cell
- :file-hash="fileHash"
- :context-lines-path="contextLinesPath"
- :line="line"
- :line-type="newLineType"
- :line-position="linePositionRight"
- :is-bottom="isBottom"
- :is-hover="isRightHover"
- :show-comment-button="true"
- :diff-view-type="parallelDiffViewType"
- :discussions="rightDiscussions"
- class="diff-line-num new_line"
- />
- <td
- :id="line.right.lineCode"
- :class="line.right.type"
- class="line_content parallel right-side"
- @mousedown.native="handleParallelLineMouseDown"
- v-html="line.right.richText"
- >
- </td>
+ <template v-if="line.left">
+ <diff-table-cell
+ :file-hash="fileHash"
+ :context-lines-path="contextLinesPath"
+ :line="line.left"
+ :line-type="oldLineType"
+ :is-bottom="isBottom"
+ :is-hover="isLeftHover"
+ :show-comment-button="true"
+ :diff-view-type="parallelDiffViewType"
+ line-position="left"
+ class="diff-line-num old_line"
+ />
+ <td
+ :id="line.left.lineCode"
+ :class="parallelViewLeftLineType"
+ class="line_content parallel left-side"
+ @mousedown.native="handleParallelLineMouseDown"
+ v-html="line.left.richText"
+ >
+ </td>
+ </template>
+ <template v-else>
+ <td class="diff-line-num old_line empty-cell"></td>
+ <td class="line_content parallel left-side empty-cell"></td>
+ </template>
+ <template v-if="line.right">
+ <diff-table-cell
+ :file-hash="fileHash"
+ :context-lines-path="contextLinesPath"
+ :line="line.right"
+ :line-type="newLineType"
+ :is-bottom="isBottom"
+ :is-hover="isRightHover"
+ :show-comment-button="true"
+ :diff-view-type="parallelDiffViewType"
+ line-position="right"
+ class="diff-line-num new_line"
+ />
+ <td
+ :id="line.right.lineCode"
+ :class="line.right.type"
+ class="line_content parallel right-side"
+ @mousedown.native="handleParallelLineMouseDown"
+ v-html="line.right.richText"
+ >
+ </td>
+ </template>
+ <template v-else>
+ <td class="diff-line-num old_line empty-cell"></td>
+ <td class="line_content parallel right-side empty-cell"></td>
+ </template>
</tr>
</template>
diff --git a/app/assets/javascripts/diffs/components/parallel_diff_view.vue b/app/assets/javascripts/diffs/components/parallel_diff_view.vue
index 24ceb52a04a..501bd4450d8 100644
--- a/app/assets/javascripts/diffs/components/parallel_diff_view.vue
+++ b/app/assets/javascripts/diffs/components/parallel_diff_view.vue
@@ -2,8 +2,6 @@
import { mapState, mapGetters } from 'vuex';
import parallelDiffTableRow from './parallel_diff_table_row.vue';
import parallelDiffCommentRow from './parallel_diff_comment_row.vue';
-import { EMPTY_CELL_TYPE } from '../constants';
-import { trimFirstCharOfLineContent } from '../store/utils';
export default {
components: {
@@ -21,46 +19,17 @@ export default {
},
},
computed: {
- ...mapGetters('diffs', [
- 'commitId',
- 'singleDiscussionByLineCode',
- 'shouldRenderParallelCommentRow',
- ]),
+ ...mapGetters('diffs', ['commitId', 'shouldRenderParallelCommentRow']),
...mapState({
diffLineCommentForms: state => state.diffs.diffLineCommentForms,
}),
- parallelDiffLines() {
- return this.diffLines.map(line => {
- const parallelLine = Object.assign({}, line);
-
- if (line.left) {
- parallelLine.left = trimFirstCharOfLineContent(line.left);
- } else {
- parallelLine.left = { type: EMPTY_CELL_TYPE };
- }
-
- if (line.right) {
- parallelLine.right = trimFirstCharOfLineContent(line.right);
- } else {
- parallelLine.right = { type: EMPTY_CELL_TYPE };
- }
-
- return parallelLine;
- });
- },
diffLinesLength() {
- return this.parallelDiffLines.length;
+ return this.diffLines.length;
},
userColorScheme() {
return window.gon.user_color_scheme;
},
},
- methods: {
- discussionsByLine(line, leftOrRight) {
- return line[leftOrRight] && line[leftOrRight].lineCode !== undefined ?
- this.singleDiscussionByLineCode(line[leftOrRight].lineCode) : [];
- },
- },
};
</script>
@@ -73,7 +42,7 @@ export default {
<table>
<tbody>
<template
- v-for="(line, index) in parallelDiffLines"
+ v-for="(line, index) in diffLines"
>
<parallel-diff-table-row
:file-hash="diffFile.fileHash"
@@ -81,8 +50,6 @@ export default {
:line="line"
:is-bottom="index + 1 === diffLinesLength"
:key="index"
- :left-discussions="discussionsByLine(line, 'left')"
- :right-discussions="discussionsByLine(line, 'right')"
/>
<parallel-diff-comment-row
v-if="shouldRenderParallelCommentRow(line)"
@@ -90,8 +57,6 @@ export default {
:line="line"
:diff-file-hash="diffFile.fileHash"
:line-index="index"
- :left-discussions="discussionsByLine(line, 'left')"
- :right-discussions="discussionsByLine(line, 'right')"
/>
</template>
</tbody>
diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js
index 4ab6ceb249a..184a90c6033 100644
--- a/app/assets/javascripts/diffs/store/actions.js
+++ b/app/assets/javascripts/diffs/store/actions.js
@@ -29,25 +29,47 @@ export const fetchDiffFiles = ({ state, commit }) => {
.then(handleLocationHash);
};
+// This is adding line discussions to the actual lines in the diff tree
+// once for parallel and once for inline mode
+export const assignDiscussionsToDiff = ({ commit }, allLineDiscussions) => {
+ Object.values(allLineDiscussions).forEach(discussions => {
+ if (discussions.length > 0) {
+ const { fileHash } = discussions[0];
+ commit(types.SET_LINE_DISCUSSIONS_FOR_FILE, { fileHash, discussions });
+ }
+ });
+};
+
+export const removeDiscussionsFromDiff = ({ commit }, removeDiscussion) => {
+ const { fileHash, line_code } = removeDiscussion;
+ commit(types.REMOVE_LINE_DISCUSSIONS_FOR_FILE, { fileHash, lineCode: line_code });
+};
+
export const startRenderDiffsQueue = ({ state, commit }) => {
- const checkItem = () => {
- const nextFile = state.diffFiles.find(
- file => !file.renderIt && (!file.collapsed || !file.text),
- );
- if (nextFile) {
- requestAnimationFrame(() => {
- commit(types.RENDER_FILE, nextFile);
- });
- requestIdleCallback(
- () => {
- checkItem();
- },
- { timeout: 1000 },
+ const checkItem = () =>
+ new Promise(resolve => {
+ const nextFile = state.diffFiles.find(
+ file => !file.renderIt && (!file.collapsed || !file.text),
);
- }
- };
- checkItem();
+ if (nextFile) {
+ requestAnimationFrame(() => {
+ commit(types.RENDER_FILE, nextFile);
+ });
+ requestIdleCallback(
+ () => {
+ checkItem()
+ .then(resolve)
+ .catch(() => {});
+ },
+ { timeout: 1000 },
+ );
+ } else {
+ resolve();
+ }
+ });
+
+ return checkItem();
};
export const setInlineDiffViewType = ({ commit }) => {
diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js
index 4a47646d7fa..968ba3c5e13 100644
--- a/app/assets/javascripts/diffs/store/getters.js
+++ b/app/assets/javascripts/diffs/store/getters.js
@@ -17,7 +17,10 @@ export const commitId = state => (state.commit && state.commit.id ? state.commit
export const diffHasAllExpandedDiscussions = (state, getters) => diff => {
const discussions = getters.getDiffFileDiscussions(diff);
- return (discussions.length && discussions.every(discussion => discussion.expanded)) || false;
+ return (
+ (discussions && discussions.length && discussions.every(discussion => discussion.expanded)) ||
+ false
+ );
};
/**
@@ -28,7 +31,10 @@ export const diffHasAllExpandedDiscussions = (state, getters) => diff => {
export const diffHasAllCollpasedDiscussions = (state, getters) => diff => {
const discussions = getters.getDiffFileDiscussions(diff);
- return (discussions.length && discussions.every(discussion => !discussion.expanded)) || false;
+ return (
+ (discussions && discussions.length && discussions.every(discussion => !discussion.expanded)) ||
+ false
+ );
};
/**
@@ -40,7 +46,9 @@ export const diffHasExpandedDiscussions = (state, getters) => diff => {
const discussions = getters.getDiffFileDiscussions(diff);
return (
- (discussions.length && discussions.find(discussion => discussion.expanded) !== undefined) ||
+ (discussions &&
+ discussions.length &&
+ discussions.find(discussion => discussion.expanded) !== undefined) ||
false
);
};
@@ -64,45 +72,38 @@ export const getDiffFileDiscussions = (state, getters, rootState, rootGetters) =
discussion.diff_discussion && _.isEqual(discussion.diff_file.file_hash, diff.fileHash),
) || [];
-export const singleDiscussionByLineCode = (state, getters, rootState, rootGetters) => lineCode => {
- if (!lineCode || lineCode === undefined) return [];
- const discussions = rootGetters.discussionsByLineCode;
- return discussions[lineCode] || [];
-};
-
-export const shouldRenderParallelCommentRow = (state, getters) => line => {
- const leftLineCode = line.left.lineCode;
- const rightLineCode = line.right.lineCode;
- const leftDiscussions = getters.singleDiscussionByLineCode(leftLineCode);
- const rightDiscussions = getters.singleDiscussionByLineCode(rightLineCode);
- const hasDiscussion = leftDiscussions.length || rightDiscussions.length;
+export const shouldRenderParallelCommentRow = state => line => {
+ const hasDiscussion =
+ (line.left && line.left.discussions && line.left.discussions.length) ||
+ (line.right && line.right.discussions && line.right.discussions.length);
- const hasExpandedDiscussionOnLeft = leftDiscussions.length
- ? leftDiscussions.every(discussion => discussion.expanded)
- : false;
- const hasExpandedDiscussionOnRight = rightDiscussions.length
- ? rightDiscussions.every(discussion => discussion.expanded)
- : false;
+ const hasExpandedDiscussionOnLeft =
+ line.left && line.left.discussions && line.left.discussions.length
+ ? line.left.discussions.every(discussion => discussion.expanded)
+ : false;
+ const hasExpandedDiscussionOnRight =
+ line.right && line.right.discussions && line.right.discussions.length
+ ? line.right.discussions.every(discussion => discussion.expanded)
+ : false;
if (hasDiscussion && (hasExpandedDiscussionOnLeft || hasExpandedDiscussionOnRight)) {
return true;
}
- const hasCommentFormOnLeft = state.diffLineCommentForms[leftLineCode];
- const hasCommentFormOnRight = state.diffLineCommentForms[rightLineCode];
+ const hasCommentFormOnLeft = line.left && state.diffLineCommentForms[line.left.lineCode];
+ const hasCommentFormOnRight = line.right && state.diffLineCommentForms[line.right.lineCode];
return hasCommentFormOnLeft || hasCommentFormOnRight;
};
-export const shouldRenderInlineCommentRow = (state, getters) => line => {
+export const shouldRenderInlineCommentRow = state => line => {
if (state.diffLineCommentForms[line.lineCode]) return true;
- const lineDiscussions = getters.singleDiscussionByLineCode(line.lineCode);
- if (lineDiscussions.length === 0) {
+ if (!line.discussions || line.discussions.length === 0) {
return false;
}
- return lineDiscussions.every(discussion => discussion.expanded);
+ return line.discussions.every(discussion => discussion.expanded);
};
// prevent babel-plugin-rewire from generating an invalid default during karma∂ tests
diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js
index c999d637d50..f61efbe6e1e 100644
--- a/app/assets/javascripts/diffs/store/mutation_types.js
+++ b/app/assets/javascripts/diffs/store/mutation_types.js
@@ -9,3 +9,5 @@ export const ADD_CONTEXT_LINES = 'ADD_CONTEXT_LINES';
export const ADD_COLLAPSED_DIFFS = 'ADD_COLLAPSED_DIFFS';
export const EXPAND_ALL_FILES = 'EXPAND_ALL_FILES';
export const RENDER_FILE = 'RENDER_FILE';
+export const SET_LINE_DISCUSSIONS_FOR_FILE = 'SET_LINE_DISCUSSIONS_FOR_FILE';
+export const REMOVE_LINE_DISCUSSIONS_FOR_FILE = 'REMOVE_LINE_DISCUSSIONS_FOR_FILE';
diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js
index bc69ae30777..676c4dab1dd 100644
--- a/app/assets/javascripts/diffs/store/mutations.js
+++ b/app/assets/javascripts/diffs/store/mutations.js
@@ -1,8 +1,12 @@
import Vue from 'vue';
-import _ from 'underscore';
import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
-import { findDiffFile, addLineReferences, removeMatchLine, addContextLines } from './utils';
-import { LINES_TO_BE_RENDERED_DIRECTLY, MAX_LINES_TO_BE_RENDERED } from '../constants';
+import {
+ findDiffFile,
+ addLineReferences,
+ removeMatchLine,
+ addContextLines,
+ prepareDiffData,
+} from './utils';
import * as types from './mutation_types';
export default {
@@ -17,38 +21,7 @@ export default {
[types.SET_DIFF_DATA](state, data) {
const diffData = convertObjectPropsToCamelCase(data, { deep: true });
- let showingLines = 0;
- const filesLength = diffData.diffFiles.length;
- let i;
- for (i = 0; i < filesLength; i += 1) {
- const file = diffData.diffFiles[i];
- if (file.parallelDiffLines) {
- const linesLength = file.parallelDiffLines.length;
- let u = 0;
- for (u = 0; u < linesLength; u += 1) {
- const line = file.parallelDiffLines[u];
- if (line.left) delete line.left.text;
- if (line.right) delete line.right.text;
- }
- }
-
- if (file.highlightedDiffLines) {
- const linesLength = file.highlightedDiffLines.length;
- let u;
- for (u = 0; u < linesLength; u += 1) {
- const line = file.highlightedDiffLines[u];
- delete line.text;
- }
- }
-
- if (file.highlightedDiffLines) {
- showingLines += file.parallelDiffLines.length;
- }
- Object.assign(file, {
- renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY,
- collapsed: file.text && showingLines > MAX_LINES_TO_BE_RENDERED,
- });
- }
+ prepareDiffData(diffData);
Object.assign(state, {
...diffData,
@@ -98,12 +71,10 @@ export default {
[types.ADD_COLLAPSED_DIFFS](state, { file, data }) {
const normalizedData = convertObjectPropsToCamelCase(data, { deep: true });
+ prepareDiffData(normalizedData);
const [newFileData] = normalizedData.diffFiles.filter(f => f.fileHash === file.fileHash);
-
- if (newFileData) {
- const index = _.findIndex(state.diffFiles, f => f.fileHash === file.fileHash);
- state.diffFiles.splice(index, 1, newFileData);
- }
+ const selectedFile = state.diffFiles.find(f => f.fileHash === file.fileHash);
+ Object.assign(selectedFile, { ...newFileData });
},
[types.EXPAND_ALL_FILES](state) {
@@ -112,4 +83,69 @@ export default {
collapsed: false,
}));
},
+
+ [types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, discussions }) {
+ const selectedFile = state.diffFiles.find(f => f.fileHash === fileHash);
+ if (selectedFile) {
+ const firstDiscussion = discussions[0];
+ const targetLine = selectedFile.parallelDiffLines.find(
+ line =>
+ (line.left && line.left.lineCode === firstDiscussion.line_code) ||
+ (line.right && line.right.lineCode === firstDiscussion.line_code),
+ );
+ if (targetLine) {
+ if (targetLine.left && targetLine.left.lineCode === firstDiscussion.line_code) {
+ Object.assign(targetLine.left, {
+ discussions,
+ });
+ } else {
+ Object.assign(targetLine.right, {
+ discussions,
+ });
+ }
+ }
+
+ if (selectedFile.highlightedDiffLines) {
+ const targetInlineLine = selectedFile.highlightedDiffLines.find(
+ line => line.lineCode === firstDiscussion.line_code,
+ );
+
+ if (targetInlineLine) {
+ Object.assign(targetInlineLine, {
+ discussions,
+ });
+ }
+ }
+ }
+ },
+
+ [types.REMOVE_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, lineCode }) {
+ const selectedFile = state.diffFiles.find(f => f.fileHash === fileHash);
+ if (selectedFile) {
+ const targetLine = selectedFile.parallelDiffLines.find(
+ line =>
+ (line.left && line.left.lineCode === lineCode) ||
+ (line.right && line.right.lineCode === lineCode),
+ );
+ if (targetLine) {
+ const side = targetLine.left && targetLine.left.lineCode === lineCode ? 'left' : 'right';
+
+ Object.assign(targetLine[side], {
+ discussions: [],
+ });
+ }
+
+ if (selectedFile.highlightedDiffLines) {
+ const targetInlineLine = selectedFile.highlightedDiffLines.find(
+ line => line.lineCode === lineCode,
+ );
+
+ if (targetInlineLine) {
+ Object.assign(targetInlineLine, {
+ discussions: [],
+ });
+ }
+ }
+ }
+ },
};
diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js
index 82082ac508a..6b1659a412c 100644
--- a/app/assets/javascripts/diffs/store/utils.js
+++ b/app/assets/javascripts/diffs/store/utils.js
@@ -8,6 +8,8 @@ import {
NEW_LINE_TYPE,
OLD_LINE_TYPE,
MATCH_LINE_TYPE,
+ LINES_TO_BE_RENDERED_DIRECTLY,
+ MAX_LINES_TO_BE_RENDERED,
} from '../constants';
export function findDiffFile(files, hash) {
@@ -161,6 +163,11 @@ export function addContextLines(options) {
* @returns {Object}
*/
export function trimFirstCharOfLineContent(line = {}) {
+ // eslint-disable-next-line no-param-reassign
+ delete line.text;
+ // eslint-disable-next-line no-param-reassign
+ line.discussions = [];
+
const parsedLine = Object.assign({}, line);
if (line.richText) {
@@ -174,6 +181,42 @@ export function trimFirstCharOfLineContent(line = {}) {
return parsedLine;
}
+// This prepares and optimizes the incoming diff data from the server
+// by setting up incremental rendering and removing unneeded data
+export function prepareDiffData(diffData) {
+ const filesLength = diffData.diffFiles.length;
+ let showingLines = 0;
+ for (let i = 0; i < filesLength; i += 1) {
+ const file = diffData.diffFiles[i];
+
+ if (file.parallelDiffLines) {
+ const linesLength = file.parallelDiffLines.length;
+ for (let u = 0; u < linesLength; u += 1) {
+ const line = file.parallelDiffLines[u];
+ if (line.left) {
+ line.left = trimFirstCharOfLineContent(line.left);
+ }
+ if (line.right) {
+ line.right = trimFirstCharOfLineContent(line.right);
+ }
+ }
+ }
+
+ if (file.highlightedDiffLines) {
+ const linesLength = file.highlightedDiffLines.length;
+ for (let u = 0; u < linesLength; u += 1) {
+ trimFirstCharOfLineContent(file.highlightedDiffLines[u]);
+ }
+ showingLines += file.parallelDiffLines.length;
+ }
+
+ Object.assign(file, {
+ renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY,
+ collapsed: file.text && showingLines > MAX_LINES_TO_BE_RENDERED,
+ });
+ }
+}
+
export function getDiffRefsByLineCode(diffFiles) {
return diffFiles.reduce((acc, diffFile) => {
const { baseSha, headSha, startSha } = diffFile.diffRefs;
diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js
index 1e168742667..8b1d8f6055e 100644
--- a/app/assets/javascripts/notes.js
+++ b/app/assets/javascripts/notes.js
@@ -154,7 +154,11 @@ export default class Notes {
this.$wrapperEl.on('click', '.system-note-commit-list-toggler', this.toggleCommitList);
this.$wrapperEl.on('click', '.js-toggle-lazy-diff', this.loadLazyDiff);
- this.$wrapperEl.on('click', '.js-toggle-lazy-diff-retry-button', this.onClickRetryLazyLoad.bind(this));
+ this.$wrapperEl.on(
+ 'click',
+ '.js-toggle-lazy-diff-retry-button',
+ this.onClickRetryLazyLoad.bind(this),
+ );
// fetch notes when tab becomes visible
this.$wrapperEl.on('visibilitychange', this.visibilityChange);
@@ -252,9 +256,7 @@ export default class Notes {
discussionNoteForm = $textarea.closest('.js-discussion-note-form');
if (discussionNoteForm.length) {
if ($textarea.val() !== '') {
- if (
- !window.confirm('Are you sure you want to cancel creating this comment?')
- ) {
+ if (!window.confirm('Are you sure you want to cancel creating this comment?')) {
return;
}
}
@@ -266,9 +268,7 @@ export default class Notes {
originalText = $textarea.closest('form').data('originalNote');
newText = $textarea.val();
if (originalText !== newText) {
- if (
- !window.confirm('Are you sure you want to cancel editing this comment?')
- ) {
+ if (!window.confirm('Are you sure you want to cancel editing this comment?')) {
return;
}
}
@@ -1316,8 +1316,7 @@ export default class Notes {
$retryButton.prop('disabled', true);
- return this.loadLazyDiff(e)
- .then(() => {
+ return this.loadLazyDiff(e).then(() => {
$retryButton.prop('disabled', false);
});
}
@@ -1343,18 +1342,18 @@ export default class Notes {
*/
if (url) {
return axios
- .get(url)
- .then(({ data }) => {
- // Reset state in case last request returned error
- $successContainer.removeClass('hidden');
- $errorContainer.addClass('hidden');
-
- Notes.renderDiffContent($container, data);
- })
- .catch(() => {
- $successContainer.addClass('hidden');
- $errorContainer.removeClass('hidden');
- });
+ .get(url)
+ .then(({ data }) => {
+ // Reset state in case last request returned error
+ $successContainer.removeClass('hidden');
+ $errorContainer.addClass('hidden');
+
+ Notes.renderDiffContent($container, data);
+ })
+ .catch(() => {
+ $successContainer.addClass('hidden');
+ $errorContainer.removeClass('hidden');
+ });
}
return Promise.resolve();
}
@@ -1545,12 +1544,8 @@ export default class Notes {
<div class="note-header">
<div class="note-header-info">
<a href="/${_.escape(currentUsername)}">
- <span class="d-none d-sm-inline-block">${_.escape(
- currentUsername,
- )}</span>
- <span class="note-headline-light">${_.escape(
- currentUsername,
- )}</span>
+ <span class="d-none d-sm-inline-block">${_.escape(currentUsername)}</span>
+ <span class="note-headline-light">${_.escape(currentUsername)}</span>
</a>
</div>
</div>
@@ -1565,9 +1560,7 @@ export default class Notes {
);
$tempNote.find('.d-none.d-sm-inline-block').text(_.escape(currentUserFullname));
- $tempNote
- .find('.note-headline-light')
- .text(`@${_.escape(currentUsername)}`);
+ $tempNote.find('.note-headline-light').text(`@${_.escape(currentUsername)}`);
return $tempNote;
}
diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue
index 0fe1c16854a..afe86911230 100644
--- a/app/assets/javascripts/notes/components/noteable_discussion.vue
+++ b/app/assets/javascripts/notes/components/noteable_discussion.vue
@@ -137,8 +137,10 @@ export default {
return this.unresolvedDiscussions.length > 1;
},
showJumpToNextDiscussion() {
- return this.hasMultipleUnresolvedDiscussions &&
- !this.isLastUnresolvedDiscussion(this.discussion.id, this.discussionsByDiffOrder);
+ return (
+ this.hasMultipleUnresolvedDiscussions &&
+ !this.isLastUnresolvedDiscussion(this.discussion.id, this.discussionsByDiffOrder)
+ );
},
shouldRenderDiffs() {
const { diffDiscussion, diffFile } = this.transformedDiscussion;
@@ -256,11 +258,16 @@ Please check your network connection and try again.`;
});
},
jumpToNextDiscussion() {
- const nextId =
- this.nextUnresolvedDiscussionId(this.discussion.id, this.discussionsByDiffOrder);
+ const nextId = this.nextUnresolvedDiscussionId(
+ this.discussion.id,
+ this.discussionsByDiffOrder,
+ );
this.jumpToDiscussion(nextId);
},
+ deleteNoteHandler(note) {
+ this.$emit('noteDeleted', this.discussion, note);
+ },
},
};
</script>
@@ -270,6 +277,7 @@ Please check your network connection and try again.`;
<div class="timeline-entry-inner">
<div class="timeline-icon">
<user-avatar-link
+ v-if="author"
:link-href="author.path"
:img-src="author.avatar_url"
:img-alt="author.name"
@@ -344,6 +352,7 @@ Please check your network connection and try again.`;
:is="componentName(note)"
:note="componentData(note)"
:key="note.id"
+ @handleDeleteNote="deleteNoteHandler"
/>
</ul>
<div
diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue
index 4ebeb5599f2..7579fc852c6 100644
--- a/app/assets/javascripts/notes/components/noteable_note.vue
+++ b/app/assets/javascripts/notes/components/noteable_note.vue
@@ -86,6 +86,7 @@ export default {
// eslint-disable-next-line no-alert
if (window.confirm('Are you sure you want to delete this comment?')) {
this.isDeleting = true;
+ this.$emit('handleDeleteNote', this.note);
this.deleteNote(this.note)
.then(() => {
diff --git a/app/assets/javascripts/notes/components/notes_app.vue b/app/assets/javascripts/notes/components/notes_app.vue
index 9b8713b40fb..7f9d23b211b 100644
--- a/app/assets/javascripts/notes/components/notes_app.vue
+++ b/app/assets/javascripts/notes/components/notes_app.vue
@@ -138,6 +138,7 @@ export default {
.then(() => {
this.isLoading = false;
this.setNotesFetchedState(true);
+ eventHub.$emit('fetchedNotesData');
})
.then(() => this.$nextTick())
.then(() => this.checkLocationHash())
diff --git a/app/assets/javascripts/notes/stores/actions.js b/app/assets/javascripts/notes/stores/actions.js
index 3eefbe11c37..9a2ec15debd 100644
--- a/app/assets/javascripts/notes/stores/actions.js
+++ b/app/assets/javascripts/notes/stores/actions.js
@@ -43,14 +43,23 @@ export const fetchDiscussions = ({ commit }, path) =>
commit(types.SET_INITIAL_DISCUSSIONS, discussions);
});
-export const refetchDiscussionById = ({ commit }, { path, discussionId }) =>
- service
- .fetchDiscussions(path)
- .then(res => res.json())
- .then(discussions => {
- const selectedDiscussion = discussions.find(discussion => discussion.id === discussionId);
- if (selectedDiscussion) commit(types.UPDATE_DISCUSSION, selectedDiscussion);
- });
+export const refetchDiscussionById = ({ commit, state }, { path, discussionId }) =>
+ new Promise(resolve => {
+ service
+ .fetchDiscussions(path)
+ .then(res => res.json())
+ .then(discussions => {
+ const selectedDiscussion = discussions.find(discussion => discussion.id === discussionId);
+ if (selectedDiscussion) {
+ commit(types.UPDATE_DISCUSSION, selectedDiscussion);
+ // We need to refetch as it is now the transformed one in state
+ const discussion = utils.findNoteObjectById(state.discussions, discussionId);
+
+ resolve(discussion);
+ }
+ })
+ .catch(() => {});
+ });
export const deleteNote = ({ commit }, note) =>
service.deleteNote(note.path).then(() => {
@@ -152,26 +161,28 @@ export const saveNote = ({ commit, dispatch }, noteData) => {
const replyId = noteData.data.in_reply_to_discussion_id;
const methodToDispatch = replyId ? 'replyToDiscussion' : 'createNewNote';
- commit(types.REMOVE_PLACEHOLDER_NOTES); // remove previous placeholders
$('.notes-form .flash-container').hide(); // hide previous flash notification
+ commit(types.REMOVE_PLACEHOLDER_NOTES); // remove previous placeholders
- if (hasQuickActions) {
- placeholderText = utils.stripQuickActions(placeholderText);
- }
+ if (replyId) {
+ if (hasQuickActions) {
+ placeholderText = utils.stripQuickActions(placeholderText);
+ }
- if (placeholderText.length) {
- commit(types.SHOW_PLACEHOLDER_NOTE, {
- noteBody: placeholderText,
- replyId,
- });
- }
+ if (placeholderText.length) {
+ commit(types.SHOW_PLACEHOLDER_NOTE, {
+ noteBody: placeholderText,
+ replyId,
+ });
+ }
- if (hasQuickActions) {
- commit(types.SHOW_PLACEHOLDER_NOTE, {
- isSystemNote: true,
- noteBody: utils.getQuickActionText(note),
- replyId,
- });
+ if (hasQuickActions) {
+ commit(types.SHOW_PLACEHOLDER_NOTE, {
+ isSystemNote: true,
+ noteBody: utils.getQuickActionText(note),
+ replyId,
+ });
+ }
}
return dispatch(methodToDispatch, noteData).then(res => {
@@ -211,7 +222,9 @@ export const saveNote = ({ commit, dispatch }, noteData) => {
if (errors && errors.commands_only) {
Flash(errors.commands_only, 'notice', noteData.flashContainer);
}
- commit(types.REMOVE_PLACEHOLDER_NOTES);
+ if (replyId) {
+ commit(types.REMOVE_PLACEHOLDER_NOTES);
+ }
return res;
});
diff --git a/app/assets/javascripts/notes/stores/getters.js b/app/assets/javascripts/notes/stores/getters.js
index 5b3b9f8776f..d4babf1fab2 100644
--- a/app/assets/javascripts/notes/stores/getters.js
+++ b/app/assets/javascripts/notes/stores/getters.js
@@ -1,5 +1,6 @@
import _ from 'underscore';
import * as constants from '../constants';
+import { reduceDiscussionsToLineCodes } from './utils';
import { collapseSystemNotes } from './collapse_utils';
export const discussions = state => collapseSystemNotes(state.discussions);
@@ -28,17 +29,8 @@ export const notesById = state =>
return acc;
}, {});
-export const discussionsByLineCode = state =>
- state.discussions.reduce((acc, note) => {
- if (note.diff_discussion && note.line_code && note.resolvable) {
- // For context about line notes: there might be multiple notes with the same line code
- const items = acc[note.line_code] || [];
- items.push(note);
-
- Object.assign(acc, { [note.line_code]: items });
- }
- return acc;
- }, {});
+export const discussionsStructuredByLineCode = state =>
+ reduceDiscussionsToLineCodes(state.discussions);
export const noteableType = state => {
const { ISSUE_NOTEABLE_TYPE, MERGE_REQUEST_NOTEABLE_TYPE, EPIC_NOTEABLE_TYPE } = constants;
diff --git a/app/assets/javascripts/notes/stores/mutations.js b/app/assets/javascripts/notes/stores/mutations.js
index ab6a95e2601..2c04bfea122 100644
--- a/app/assets/javascripts/notes/stores/mutations.js
+++ b/app/assets/javascripts/notes/stores/mutations.js
@@ -54,13 +54,12 @@ export default {
[types.EXPAND_DISCUSSION](state, { discussionId }) {
const discussion = utils.findNoteObjectById(state.discussions, discussionId);
-
- discussion.expanded = true;
+ Object.assign(discussion, { expanded: true });
},
[types.COLLAPSE_DISCUSSION](state, { discussionId }) {
const discussion = utils.findNoteObjectById(state.discussions, discussionId);
- discussion.expanded = false;
+ Object.assign(discussion, { expanded: false });
},
[types.REMOVE_PLACEHOLDER_NOTES](state) {
@@ -95,10 +94,15 @@ export default {
[types.SET_USER_DATA](state, data) {
Object.assign(state, { userData: data });
},
+
[types.SET_INITIAL_DISCUSSIONS](state, discussionsData) {
const discussions = [];
discussionsData.forEach(discussion => {
+ if (discussion.diff_file) {
+ Object.assign(discussion, { fileHash: discussion.diff_file.file_hash });
+ }
+
// To support legacy notes, should be very rare case.
if (discussion.individual_note && discussion.notes.length > 1) {
discussion.notes.forEach(n => {
@@ -168,8 +172,7 @@ export default {
[types.TOGGLE_DISCUSSION](state, { discussionId }) {
const discussion = utils.findNoteObjectById(state.discussions, discussionId);
-
- discussion.expanded = !discussion.expanded;
+ Object.assign(discussion, { expanded: !discussion.expanded });
},
[types.UPDATE_NOTE](state, note) {
@@ -185,16 +188,12 @@ export default {
[types.UPDATE_DISCUSSION](state, noteData) {
const note = noteData;
- let index = 0;
-
- state.discussions.forEach((n, i) => {
- if (n.id === note.id) {
- index = i;
- }
- });
-
+ const selectedDiscussion = state.discussions.find(disc => disc.id === note.id);
note.expanded = true; // override expand flag to prevent collapse
- state.discussions.splice(index, 1, note);
+ if (note.diff_file) {
+ Object.assign(note, { fileHash: note.diff_file.file_hash });
+ }
+ Object.assign(selectedDiscussion, { ...note });
},
[types.CLOSE_ISSUE](state) {
diff --git a/app/assets/javascripts/notes/stores/utils.js b/app/assets/javascripts/notes/stores/utils.js
index a0e096ebfaf..8ccbdb4c130 100644
--- a/app/assets/javascripts/notes/stores/utils.js
+++ b/app/assets/javascripts/notes/stores/utils.js
@@ -2,13 +2,11 @@ import AjaxCache from '~/lib/utils/ajax_cache';
const REGEX_QUICK_ACTIONS = /^\/\w+.*$/gm;
-export const findNoteObjectById = (notes, id) =>
- notes.filter(n => n.id === id)[0];
+export const findNoteObjectById = (notes, id) => notes.filter(n => n.id === id)[0];
export const getQuickActionText = note => {
let text = 'Applying command';
- const quickActions =
- AjaxCache.get(gl.GfmAutoComplete.dataSources.commands) || [];
+ const quickActions = AjaxCache.get(gl.GfmAutoComplete.dataSources.commands) || [];
const executedCommands = quickActions.filter(command => {
const commandRegex = new RegExp(`/${command.name}`);
@@ -27,7 +25,18 @@ export const getQuickActionText = note => {
return text;
};
+export const reduceDiscussionsToLineCodes = selectedDiscussions =>
+ selectedDiscussions.reduce((acc, note) => {
+ if (note.diff_discussion && note.line_code && note.resolvable) {
+ // For context about line notes: there might be multiple notes with the same line code
+ const items = acc[note.line_code] || [];
+ items.push(note);
+
+ Object.assign(acc, { [note.line_code]: items });
+ }
+ return acc;
+ }, {});
+
export const hasQuickActions = note => REGEX_QUICK_ACTIONS.test(note);
-export const stripQuickActions = note =>
- note.replace(REGEX_QUICK_ACTIONS, '').trim();
+export const stripQuickActions = note => note.replace(REGEX_QUICK_ACTIONS, '').trim();
diff --git a/spec/features/merge_request/user_posts_diff_notes_spec.rb b/spec/features/merge_request/user_posts_diff_notes_spec.rb
index 77261f9375c..b6ed3686de2 100644
--- a/spec/features/merge_request/user_posts_diff_notes_spec.rb
+++ b/spec/features/merge_request/user_posts_diff_notes_spec.rb
@@ -186,11 +186,8 @@ describe 'Merge request > User posts diff notes', :js do
describe 'posting a note' do
it 'adds as discussion' do
- expect(page).to have_css('.js-temp-notes-holder', count: 2)
-
should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]'), asset_form_reset: false)
expect(page).to have_css('.notes_holder .note.note-discussion', count: 1)
- expect(page).to have_css('.js-temp-notes-holder', count: 1)
expect(page).to have_button('Reply...')
end
end
@@ -267,7 +264,7 @@ describe 'Merge request > User posts diff notes', :js do
def assert_comment_persistence(line_holder, asset_form_reset:)
notes_holder_saved = line_holder.find(:xpath, notes_holder_input_xpath)
- expect(notes_holder_saved[:class]).not_to include(notes_holder_input_class)
+ expect(notes_holder_saved[:class]).not_to include('note-edit-form')
expect(notes_holder_saved).to have_content test_note_comment
assert_form_is_reset if asset_form_reset
@@ -281,6 +278,6 @@ describe 'Merge request > User posts diff notes', :js do
end
def assert_form_is_reset
- expect(page).to have_no_css('.js-temp-notes-holder')
+ expect(page).to have_no_css('.note-edit-form')
end
end
diff --git a/spec/javascripts/diffs/components/diff_file_spec.js b/spec/javascripts/diffs/components/diff_file_spec.js
index 44a38f7ca82..845fef23db6 100644
--- a/spec/javascripts/diffs/components/diff_file_spec.js
+++ b/spec/javascripts/diffs/components/diff_file_spec.js
@@ -51,7 +51,9 @@ describe('DiffFile', () => {
});
it('should have collapsed text and link', done => {
- vm.file.collapsed = true;
+ vm.file.renderIt = true;
+ vm.file.collapsed = false;
+ vm.file.highlightedDiffLines = null;
vm.$nextTick(() => {
expect(vm.$el.innerText).toContain('This diff is collapsed');
diff --git a/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js b/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js
index a1a37b342b7..663c0680845 100644
--- a/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js
+++ b/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js
@@ -6,61 +6,61 @@ import discussionsMockData from '../mock_data/diff_discussions';
import diffFileMockData from '../mock_data/diff_file';
describe('DiffLineGutterContent', () => {
- const getDiscussionsMockData = () => [Object.assign({}, discussionsMockData)];
const getDiffFileMock = () => Object.assign({}, diffFileMockData);
const createComponent = (options = {}) => {
const cmp = Vue.extend(DiffLineGutterContent);
const props = Object.assign({}, options);
+ props.line = {
+ lineCode: 'LC_42',
+ type: 'new',
+ oldLine: null,
+ newLine: 1,
+ discussions: [],
+ text: '+<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n',
+ richText: '+<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n',
+ metaData: null,
+ };
props.fileHash = getDiffFileMock().fileHash;
props.contextLinesPath = '/context/lines/path';
return createComponentWithStore(cmp, store, props).$mount();
};
- const setDiscussions = component => {
- component.$store.dispatch('setInitialNotes', getDiscussionsMockData());
- };
-
- const resetDiscussions = component => {
- component.$store.dispatch('setInitialNotes', []);
- };
describe('computed', () => {
describe('lineHref', () => {
it('should prepend # to lineCode', () => {
const lineCode = 'LC_42';
- const component = createComponent({ lineCode });
+ const component = createComponent();
expect(component.lineHref).toEqual(`#${lineCode}`);
});
it('should return # if there is no lineCode', () => {
- const component = createComponent({ lineCode: null });
+ const component = createComponent();
+ component.line.lineCode = '';
expect(component.lineHref).toEqual('#');
});
});
describe('discussions, hasDiscussions, shouldShowAvatarsOnGutter', () => {
it('should return empty array when there is no discussion', () => {
- const component = createComponent({ lineCode: 'LC_42' });
- expect(component.discussions).toEqual([]);
+ const component = createComponent();
expect(component.hasDiscussions).toEqual(false);
expect(component.shouldShowAvatarsOnGutter).toEqual(false);
});
it('should return discussions for the given lineCode', () => {
- const { lineCode } = getDiffFileMock().highlightedDiffLines[1];
- const component = createComponent({
- lineCode,
+ const cmp = Vue.extend(DiffLineGutterContent);
+ const props = {
+ line: getDiffFileMock().highlightedDiffLines[1],
+ fileHash: getDiffFileMock().fileHash,
showCommentButton: true,
- discussions: getDiscussionsMockData(),
- });
+ contextLinesPath: '/context/lines/path',
+ };
+ props.line.discussions = [Object.assign({}, discussionsMockData)];
+ const component = createComponentWithStore(cmp, store, props).$mount();
- setDiscussions(component);
-
- expect(component.discussions).toEqual(getDiscussionsMockData());
expect(component.hasDiscussions).toEqual(true);
expect(component.shouldShowAvatarsOnGutter).toEqual(true);
-
- resetDiscussions(component);
});
});
});
@@ -104,9 +104,7 @@ describe('DiffLineGutterContent', () => {
lineCode: getDiffFileMock().highlightedDiffLines[1].lineCode,
});
- setDiscussions(component);
expect(component.$el.querySelector('.diff-comment-avatar-holders')).toBeDefined();
- resetDiscussions(component);
});
});
});
diff --git a/spec/javascripts/diffs/components/parallel_diff_view_spec.js b/spec/javascripts/diffs/components/parallel_diff_view_spec.js
index 165e4b69b6c..091e01868d3 100644
--- a/spec/javascripts/diffs/components/parallel_diff_view_spec.js
+++ b/spec/javascripts/diffs/components/parallel_diff_view_spec.js
@@ -18,11 +18,11 @@ describe('ParallelDiffView', () => {
}).$mount();
});
- describe('computed', () => {
- describe('parallelDiffLines', () => {
+ describe('assigned', () => {
+ describe('diffLines', () => {
it('should normalize lines for empty cells', () => {
- expect(component.parallelDiffLines[0].left.type).toEqual(constants.EMPTY_CELL_TYPE);
- expect(component.parallelDiffLines[1].left.type).toEqual(constants.EMPTY_CELL_TYPE);
+ expect(component.diffLines[0].left.type).toEqual(constants.EMPTY_CELL_TYPE);
+ expect(component.diffLines[1].left.type).toEqual(constants.EMPTY_CELL_TYPE);
});
});
});
diff --git a/spec/javascripts/diffs/mock_data/diff_file.js b/spec/javascripts/diffs/mock_data/diff_file.js
index cce36ecc91f..372b8f066cf 100644
--- a/spec/javascripts/diffs/mock_data/diff_file.js
+++ b/spec/javascripts/diffs/mock_data/diff_file.js
@@ -49,6 +49,7 @@ export default {
type: 'new',
oldLine: null,
newLine: 1,
+ discussions: [],
text: '+<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n',
richText: '+<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n',
metaData: null,
@@ -58,6 +59,7 @@ export default {
type: 'new',
oldLine: null,
newLine: 2,
+ discussions: [],
text: '+<span id="LC2" class="line" lang="plaintext"></span>\n',
richText: '+<span id="LC2" class="line" lang="plaintext"></span>\n',
metaData: null,
@@ -67,6 +69,7 @@ export default {
type: null,
oldLine: 1,
newLine: 3,
+ discussions: [],
text: ' <span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
richText: ' <span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
metaData: null,
@@ -76,6 +79,7 @@ export default {
type: null,
oldLine: 2,
newLine: 4,
+ discussions: [],
text: ' <span id="LC4" class="line" lang="plaintext"></span>\n',
richText: ' <span id="LC4" class="line" lang="plaintext"></span>\n',
metaData: null,
@@ -85,6 +89,7 @@ export default {
type: null,
oldLine: 3,
newLine: 5,
+ discussions: [],
text: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
richText: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
metaData: null,
@@ -94,6 +99,7 @@ export default {
type: 'match',
oldLine: null,
newLine: null,
+ discussions: [],
text: '',
richText: '',
metaData: {
@@ -112,6 +118,7 @@ export default {
type: 'new',
oldLine: null,
newLine: 1,
+ discussions: [],
text: '+<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n',
richText: '<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n',
metaData: null,
@@ -126,6 +133,7 @@ export default {
type: 'new',
oldLine: null,
newLine: 2,
+ discussions: [],
text: '+<span id="LC2" class="line" lang="plaintext"></span>\n',
richText: '<span id="LC2" class="line" lang="plaintext"></span>\n',
metaData: null,
@@ -137,6 +145,7 @@ export default {
type: null,
oldLine: 1,
newLine: 3,
+ discussions: [],
text: ' <span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
richText: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
metaData: null,
@@ -146,6 +155,7 @@ export default {
type: null,
oldLine: 1,
newLine: 3,
+ discussions: [],
text: ' <span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
richText: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
metaData: null,
@@ -157,6 +167,7 @@ export default {
type: null,
oldLine: 2,
newLine: 4,
+ discussions: [],
text: ' <span id="LC4" class="line" lang="plaintext"></span>\n',
richText: '<span id="LC4" class="line" lang="plaintext"></span>\n',
metaData: null,
@@ -166,6 +177,7 @@ export default {
type: null,
oldLine: 2,
newLine: 4,
+ discussions: [],
text: ' <span id="LC4" class="line" lang="plaintext"></span>\n',
richText: '<span id="LC4" class="line" lang="plaintext"></span>\n',
metaData: null,
@@ -177,6 +189,7 @@ export default {
type: null,
oldLine: 3,
newLine: 5,
+ discussions: [],
text: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
richText: '<span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
metaData: null,
@@ -186,6 +199,7 @@ export default {
type: null,
oldLine: 3,
newLine: 5,
+ discussions: [],
text: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
richText: '<span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
metaData: null,
@@ -197,6 +211,7 @@ export default {
type: 'match',
oldLine: null,
newLine: null,
+ discussions: [],
text: '',
richText: '',
metaData: {
@@ -209,6 +224,7 @@ export default {
type: 'match',
oldLine: null,
newLine: null,
+ discussions: [],
text: '',
richText: '',
metaData: {
diff --git a/spec/javascripts/diffs/store/actions_spec.js b/spec/javascripts/diffs/store/actions_spec.js
index c1560dac1a0..c162fc965ba 100644
--- a/spec/javascripts/diffs/store/actions_spec.js
+++ b/spec/javascripts/diffs/store/actions_spec.js
@@ -7,10 +7,30 @@ import {
} from '~/diffs/constants';
import * as actions from '~/diffs/store/actions';
import * as types from '~/diffs/store/mutation_types';
+import { reduceDiscussionsToLineCodes } from '~/notes/stores/utils';
import axios from '~/lib/utils/axios_utils';
import testAction from '../../helpers/vuex_action_helper';
describe('DiffsStoreActions', () => {
+ const originalMethods = {
+ requestAnimationFrame: global.requestAnimationFrame,
+ requestIdleCallback: global.requestIdleCallback,
+ };
+
+ beforeEach(() => {
+ ['requestAnimationFrame', 'requestIdleCallback'].forEach(method => {
+ global[method] = cb => {
+ cb();
+ };
+ });
+ });
+
+ afterEach(() => {
+ ['requestAnimationFrame', 'requestIdleCallback'].forEach(method => {
+ global[method] = originalMethods[method];
+ });
+ });
+
describe('setBaseConfig', () => {
it('should set given endpoint and project path', done => {
const endpoint = '/diffs/set/endpoint';
@@ -53,6 +73,162 @@ describe('DiffsStoreActions', () => {
});
});
+ describe('assignDiscussionsToDiff', () => {
+ it('should merge discussions into diffs', done => {
+ const state = {
+ diffFiles: [
+ {
+ fileHash: 'ABC',
+ parallelDiffLines: [
+ {
+ left: {
+ lineCode: 'ABC_1_1',
+ discussions: [],
+ },
+ right: {
+ lineCode: 'ABC_1_1',
+ discussions: [],
+ },
+ },
+ ],
+ highlightedDiffLines: [
+ {
+ lineCode: 'ABC_1_1',
+ discussions: [],
+ },
+ ],
+ },
+ ],
+ };
+
+ const singleDiscussion = {
+ line_code: 'ABC_1_1',
+ diff_discussion: {},
+ diff_file: {
+ file_hash: 'ABC',
+ },
+ resolvable: true,
+ fileHash: 'ABC',
+ };
+
+ const discussions = reduceDiscussionsToLineCodes([singleDiscussion]);
+
+ testAction(
+ actions.assignDiscussionsToDiff,
+ discussions,
+ state,
+ [
+ {
+ type: types.SET_LINE_DISCUSSIONS_FOR_FILE,
+ payload: {
+ fileHash: 'ABC',
+ discussions: [singleDiscussion],
+ },
+ },
+ ],
+ [],
+ () => {
+ done();
+ },
+ );
+ });
+ });
+
+ describe('removeDiscussionsFromDiff', () => {
+ it('should remove discussions from diffs', done => {
+ const state = {
+ diffFiles: [
+ {
+ fileHash: 'ABC',
+ parallelDiffLines: [
+ {
+ left: {
+ lineCode: 'ABC_1_1',
+ discussions: [
+ {
+ id: 1,
+ },
+ ],
+ },
+ right: {
+ lineCode: 'ABC_1_1',
+ discussions: [],
+ },
+ },
+ ],
+ highlightedDiffLines: [
+ {
+ lineCode: 'ABC_1_1',
+ discussions: [],
+ },
+ ],
+ },
+ ],
+ };
+ const singleDiscussion = {
+ fileHash: 'ABC',
+ line_code: 'ABC_1_1',
+ };
+
+ testAction(
+ actions.removeDiscussionsFromDiff,
+ singleDiscussion,
+ state,
+ [
+ {
+ type: types.REMOVE_LINE_DISCUSSIONS_FOR_FILE,
+ payload: {
+ fileHash: 'ABC',
+ lineCode: 'ABC_1_1',
+ },
+ },
+ ],
+ [],
+ () => {
+ done();
+ },
+ );
+ });
+ });
+
+ describe('startRenderDiffsQueue', () => {
+ it('should set all files to RENDER_FILE', done => {
+ const state = {
+ diffFiles: [
+ {
+ id: 1,
+ renderIt: false,
+ collapsed: false,
+ },
+ {
+ id: 2,
+ renderIt: false,
+ collapsed: false,
+ },
+ ],
+ };
+
+ const pseudoCommit = (commitType, file) => {
+ expect(commitType).toBe(types.RENDER_FILE);
+ Object.assign(file, {
+ renderIt: true,
+ });
+ };
+
+ actions
+ .startRenderDiffsQueue({ state, commit: pseudoCommit })
+ .then(() => {
+ expect(state.diffFiles[0].renderIt).toBeTruthy();
+ expect(state.diffFiles[1].renderIt).toBeTruthy();
+
+ done();
+ })
+ .catch(() => {
+ done.fail();
+ });
+ });
+ });
+
describe('setInlineDiffViewType', () => {
it('should set diff view type to inline and also set the cookie properly', done => {
testAction(
@@ -204,7 +380,11 @@ describe('DiffsStoreActions', () => {
actions.toggleFileDiscussions({ getters, dispatch });
- expect(dispatch).toHaveBeenCalledWith('collapseDiscussion', { discussionId: 1 }, { root: true });
+ expect(dispatch).toHaveBeenCalledWith(
+ 'collapseDiscussion',
+ { discussionId: 1 },
+ { root: true },
+ );
});
it('should dispatch expandDiscussion when all discussions are collapsed', () => {
@@ -218,7 +398,11 @@ describe('DiffsStoreActions', () => {
actions.toggleFileDiscussions({ getters, dispatch });
- expect(dispatch).toHaveBeenCalledWith('expandDiscussion', { discussionId: 1 }, { root: true });
+ expect(dispatch).toHaveBeenCalledWith(
+ 'expandDiscussion',
+ { discussionId: 1 },
+ { root: true },
+ );
});
it('should dispatch expandDiscussion when some discussions are collapsed and others are expanded for the collapsed discussion', () => {
@@ -232,7 +416,11 @@ describe('DiffsStoreActions', () => {
actions.toggleFileDiscussions({ getters, dispatch });
- expect(dispatch).toHaveBeenCalledWith('expandDiscussion', { discussionId: 1 }, { root: true });
+ expect(dispatch).toHaveBeenCalledWith(
+ 'expandDiscussion',
+ { discussionId: 1 },
+ { root: true },
+ );
});
});
});
diff --git a/spec/javascripts/diffs/store/getters_spec.js b/spec/javascripts/diffs/store/getters_spec.js
index a59b26b2634..4747e437c4e 100644
--- a/spec/javascripts/diffs/store/getters_spec.js
+++ b/spec/javascripts/diffs/store/getters_spec.js
@@ -184,101 +184,73 @@ describe('Diffs Module Getters', () => {
});
});
- describe('singleDiscussionByLineCode', () => {
- it('returns found discussion per line Code', () => {
- const discussionsMock = {};
- discussionsMock.ABC = discussionMock;
-
- expect(
- getters.singleDiscussionByLineCode(localState, {}, null, {
- discussionsByLineCode: () => discussionsMock,
- })('DEF'),
- ).toEqual([]);
- });
-
- it('returns empty array when no discussions match', () => {
- expect(
- getters.singleDiscussionByLineCode(localState, {}, null, {
- discussionsByLineCode: () => {},
- })('DEF'),
- ).toEqual([]);
- });
- });
-
describe('shouldRenderParallelCommentRow', () => {
let line;
beforeEach(() => {
line = {};
+ discussionMock.expanded = true;
+
line.left = {
lineCode: 'ABC',
+ discussions: [discussionMock],
};
line.right = {
lineCode: 'DEF',
+ discussions: [discussionMock1],
};
});
it('returns true when discussion is expanded', () => {
- discussionMock.expanded = true;
-
- expect(
- getters.shouldRenderParallelCommentRow(localState, {
- singleDiscussionByLineCode: () => [discussionMock],
- })(line),
- ).toEqual(true);
+ expect(getters.shouldRenderParallelCommentRow(localState)(line)).toEqual(true);
});
it('returns false when no discussion was found', () => {
+ line.left.discussions = [];
+ line.right.discussions = [];
+
localState.diffLineCommentForms.ABC = false;
localState.diffLineCommentForms.DEF = false;
- expect(
- getters.shouldRenderParallelCommentRow(localState, {
- singleDiscussionByLineCode: () => [],
- })(line),
- ).toEqual(false);
+ expect(getters.shouldRenderParallelCommentRow(localState)(line)).toEqual(false);
});
it('returns true when discussionForm was found', () => {
localState.diffLineCommentForms.ABC = {};
- expect(
- getters.shouldRenderParallelCommentRow(localState, {
- singleDiscussionByLineCode: () => [discussionMock],
- })(line),
- ).toEqual(true);
+ expect(getters.shouldRenderParallelCommentRow(localState)(line)).toEqual(true);
});
});
describe('shouldRenderInlineCommentRow', () => {
+ let line;
+
+ beforeEach(() => {
+ discussionMock.expanded = true;
+
+ line = {
+ lineCode: 'ABC',
+ discussions: [discussionMock],
+ };
+ });
+
it('returns true when diffLineCommentForms has form', () => {
localState.diffLineCommentForms.ABC = {};
- expect(
- getters.shouldRenderInlineCommentRow(localState)({
- lineCode: 'ABC',
- }),
- ).toEqual(true);
+ expect(getters.shouldRenderInlineCommentRow(localState)(line)).toEqual(true);
});
it('returns false when no line discussions were found', () => {
- expect(
- getters.shouldRenderInlineCommentRow(localState, {
- singleDiscussionByLineCode: () => [],
- })('DEF'),
- ).toEqual(false);
+ line.discussions = [];
+ expect(getters.shouldRenderInlineCommentRow(localState)(line)).toEqual(false);
});
it('returns true if all found discussions are expanded', () => {
discussionMock.expanded = true;
- expect(
- getters.shouldRenderInlineCommentRow(localState, {
- singleDiscussionByLineCode: () => [discussionMock],
- })('ABC'),
- ).toEqual(true);
+ expect(getters.shouldRenderInlineCommentRow(localState)(line)).toEqual(true);
});
});
diff --git a/spec/javascripts/diffs/store/mutations_spec.js b/spec/javascripts/diffs/store/mutations_spec.js
index 8f89984c6e5..9a89bc57404 100644
--- a/spec/javascripts/diffs/store/mutations_spec.js
+++ b/spec/javascripts/diffs/store/mutations_spec.js
@@ -138,10 +138,9 @@ describe('DiffsStoreMutations', () => {
const fileHash = 123;
const state = { diffFiles: [{}, { fileHash, existingField: 0 }] };
- const file = { fileHash };
const data = { diff_files: [{ file_hash: fileHash, extra_field: 1, existingField: 1 }] };
- mutations[types.ADD_COLLAPSED_DIFFS](state, { file, data });
+ mutations[types.ADD_COLLAPSED_DIFFS](state, { file: state.diffFiles[1], data });
expect(spy).toHaveBeenCalledWith(data, { deep: true });
expect(state.diffFiles[1].fileHash).toEqual(fileHash);
@@ -149,4 +148,107 @@ describe('DiffsStoreMutations', () => {
expect(state.diffFiles[1].extraField).toEqual(1);
});
});
+
+ describe('SET_LINE_DISCUSSIONS_FOR_FILE', () => {
+ it('should add discussions to the given line', () => {
+ const state = {
+ diffFiles: [
+ {
+ fileHash: 'ABC',
+ parallelDiffLines: [
+ {
+ left: {
+ lineCode: 'ABC_1',
+ discussions: [],
+ },
+ right: {
+ lineCode: 'ABC_1',
+ discussions: [],
+ },
+ },
+ ],
+ highlightedDiffLines: [
+ {
+ lineCode: 'ABC_1',
+ discussions: [],
+ },
+ ],
+ },
+ ],
+ };
+ const discussions = [
+ {
+ id: 1,
+ line_code: 'ABC_1',
+ },
+ {
+ id: 2,
+ line_code: 'ABC_1',
+ },
+ ];
+
+ mutations[types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash: 'ABC', discussions });
+
+ expect(state.diffFiles[0].parallelDiffLines[0].left.discussions.length).toEqual(2);
+ expect(state.diffFiles[0].parallelDiffLines[0].left.discussions[1].id).toEqual(2);
+
+ expect(state.diffFiles[0].highlightedDiffLines[0].discussions.length).toEqual(2);
+ expect(state.diffFiles[0].highlightedDiffLines[0].discussions[1].id).toEqual(2);
+ });
+ });
+
+ describe('REMOVE_LINE_DISCUSSIONS', () => {
+ it('should remove the existing discussions on the given line', () => {
+ const state = {
+ diffFiles: [
+ {
+ fileHash: 'ABC',
+ parallelDiffLines: [
+ {
+ left: {
+ lineCode: 'ABC_1',
+ discussions: [
+ {
+ id: 1,
+ line_code: 'ABC_1',
+ },
+ {
+ id: 2,
+ line_code: 'ABC_1',
+ },
+ ],
+ },
+ right: {
+ lineCode: 'ABC_1',
+ discussions: [],
+ },
+ },
+ ],
+ highlightedDiffLines: [
+ {
+ lineCode: 'ABC_1',
+ discussions: [
+ {
+ id: 1,
+ line_code: 'ABC_1',
+ },
+ {
+ id: 2,
+ line_code: 'ABC_1',
+ },
+ ],
+ },
+ ],
+ },
+ ],
+ };
+
+ mutations[types.REMOVE_LINE_DISCUSSIONS_FOR_FILE](state, {
+ fileHash: 'ABC',
+ lineCode: 'ABC_1',
+ });
+ expect(state.diffFiles[0].parallelDiffLines[0].left.discussions.length).toEqual(0);
+ expect(state.diffFiles[0].highlightedDiffLines[0].discussions.length).toEqual(0);
+ });
+ });
});
diff --git a/spec/javascripts/diffs/store/utils_spec.js b/spec/javascripts/diffs/store/utils_spec.js
index 32136d9ebff..bd9d63769a1 100644
--- a/spec/javascripts/diffs/store/utils_spec.js
+++ b/spec/javascripts/diffs/store/utils_spec.js
@@ -179,32 +179,64 @@ describe('DiffsStoreUtils', () => {
describe('trimFirstCharOfLineContent', () => {
it('trims the line when it starts with a space', () => {
- expect(utils.trimFirstCharOfLineContent({ richText: ' diff' })).toEqual({ richText: 'diff' });
+ expect(utils.trimFirstCharOfLineContent({ richText: ' diff' })).toEqual({
+ discussions: [],
+ richText: 'diff',
+ });
});
it('trims the line when it starts with a +', () => {
- expect(utils.trimFirstCharOfLineContent({ richText: '+diff' })).toEqual({ richText: 'diff' });
+ expect(utils.trimFirstCharOfLineContent({ richText: '+diff' })).toEqual({
+ discussions: [],
+ richText: 'diff',
+ });
});
it('trims the line when it starts with a -', () => {
- expect(utils.trimFirstCharOfLineContent({ richText: '-diff' })).toEqual({ richText: 'diff' });
+ expect(utils.trimFirstCharOfLineContent({ richText: '-diff' })).toEqual({
+ discussions: [],
+ richText: 'diff',
+ });
});
it('does not trims the line when it starts with a letter', () => {
- expect(utils.trimFirstCharOfLineContent({ richText: 'diff' })).toEqual({ richText: 'diff' });
+ expect(utils.trimFirstCharOfLineContent({ richText: 'diff' })).toEqual({
+ discussions: [],
+ richText: 'diff',
+ });
});
it('does not modify the provided object', () => {
const lineObj = {
+ discussions: [],
richText: ' diff',
};
utils.trimFirstCharOfLineContent(lineObj);
- expect(lineObj).toEqual({ richText: ' diff' });
+ expect(lineObj).toEqual({ discussions: [], richText: ' diff' });
});
it('handles a undefined or null parameter', () => {
- expect(utils.trimFirstCharOfLineContent()).toEqual({});
+ expect(utils.trimFirstCharOfLineContent()).toEqual({ discussions: [] });
+ });
+ });
+
+ describe('prepareDiffData', () => {
+ it('sets the renderIt and collapsed attribute on files', () => {
+ const preparedDiff = { diffFiles: [getDiffFileMock()] };
+ utils.prepareDiffData(preparedDiff);
+
+ const firstParallelDiffLine = preparedDiff.diffFiles[0].parallelDiffLines[2];
+ expect(firstParallelDiffLine.left.discussions.length).toBe(0);
+ expect(firstParallelDiffLine.left).not.toHaveAttr('text');
+ expect(firstParallelDiffLine.right.discussions.length).toBe(0);
+ expect(firstParallelDiffLine.right).not.toHaveAttr('text');
+
+ expect(preparedDiff.diffFiles[0].highlightedDiffLines[0].discussions.length).toBe(0);
+ expect(preparedDiff.diffFiles[0].highlightedDiffLines[0]).not.toHaveAttr('text');
+
+ expect(preparedDiff.diffFiles[0].renderIt).toBeTruthy();
+ expect(preparedDiff.diffFiles[0].collapsed).toBeFalsy();
});
});
});
diff --git a/spec/javascripts/lazy_loader_spec.js b/spec/javascripts/lazy_loader_spec.js
index 1d81e4e2d1a..c177d79b9e0 100644
--- a/spec/javascripts/lazy_loader_spec.js
+++ b/spec/javascripts/lazy_loader_spec.js
@@ -2,10 +2,10 @@ import LazyLoader from '~/lazy_loader';
let lazyLoader = null;
-describe('LazyLoader', function () {
+describe('LazyLoader', function() {
preloadFixtures('issues/issue_with_comment.html.raw');
- beforeEach(function () {
+ beforeEach(function() {
loadFixtures('issues/issue_with_comment.html.raw');
lazyLoader = new LazyLoader({
observerNode: 'body',
@@ -13,8 +13,8 @@ describe('LazyLoader', function () {
// Doing everything that happens normally in onload
lazyLoader.loadCheck();
});
- describe('behavior', function () {
- it('should copy value from data-src to src for img 1', function (done) {
+ describe('behavior', function() {
+ it('should copy value from data-src to src for img 1', function(done) {
const img = document.querySelectorAll('img[data-src]')[0];
const originalDataSrc = img.getAttribute('data-src');
img.scrollIntoView();
@@ -26,7 +26,7 @@ describe('LazyLoader', function () {
}, 100);
});
- it('should lazy load dynamically added data-src images', function (done) {
+ it('should lazy load dynamically added data-src images', function(done) {
const newImg = document.createElement('img');
const testPath = '/img/testimg.png';
newImg.className = 'lazy';
@@ -41,7 +41,7 @@ describe('LazyLoader', function () {
}, 100);
});
- it('should not alter normal images', function (done) {
+ it('should not alter normal images', function(done) {
const newImg = document.createElement('img');
const testPath = '/img/testimg.png';
newImg.setAttribute('src', testPath);