summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFelipe Artur <fcardozo@gitlab.com>2018-07-27 18:25:53 +0000
committerFelipe Artur <fcardozo@gitlab.com>2018-07-27 18:25:53 +0000
commit0cf823558887aaf2ad041473ed9961755d5d5f3b (patch)
treecafd8d35271ddad4890ce05c88075816f8733e01
parent923b26133b8d04e8c4da4183b8f0382eefcc8147 (diff)
parentd1f09ec7cdab3b6892b1006b8c382c08be4cd9e0 (diff)
downloadgitlab-ce-11-1-stable-patch-3.tar.gz
Merge branch '11-1-3-stable-fixes-mr-refactor-regressions' into '11-1-stable-patch-3'11-1-stable-patch-3
MR refactor fixes for 11.1.3 See merge request gitlab-org/gitlab-ce!20878
-rw-r--r--app/assets/javascripts/autosave.js4
-rw-r--r--app/assets/javascripts/diffs/components/diff_discussions.vue1
-rw-r--r--app/assets/javascripts/diffs/components/diff_line_gutter_content.vue21
-rw-r--r--app/assets/javascripts/diffs/components/diff_line_note_form.vue31
-rw-r--r--app/assets/javascripts/diffs/components/diff_table_cell.vue6
-rw-r--r--app/assets/javascripts/diffs/components/inline_diff_comment_row.vue16
-rw-r--r--app/assets/javascripts/diffs/components/inline_diff_table_row.vue8
-rw-r--r--app/assets/javascripts/diffs/components/inline_diff_view.vue24
-rw-r--r--app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue53
-rw-r--r--app/assets/javascripts/diffs/components/parallel_diff_table_row.vue14
-rw-r--r--app/assets/javascripts/diffs/components/parallel_diff_view.vue39
-rw-r--r--app/assets/javascripts/diffs/store/getters.js83
-rw-r--r--app/assets/javascripts/diffs/store/utils.js21
-rw-r--r--app/assets/javascripts/lib/utils/poll.js11
-rw-r--r--app/assets/javascripts/notes/components/discussion_counter.vue28
-rw-r--r--app/assets/javascripts/notes/components/note_form.vue2
-rw-r--r--app/assets/javascripts/notes/components/noteable_discussion.vue73
-rw-r--r--app/assets/javascripts/notes/mixins/autosave.js17
-rw-r--r--app/assets/javascripts/notes/mixins/discussion_navigation.js29
-rw-r--r--app/assets/javascripts/notes/stores/getters.js97
-rw-r--r--app/assets/javascripts/notes/stores/mutations.js19
-rw-r--r--app/assets/javascripts/notes/stores/utils.js9
-rw-r--r--app/serializers/discussion_entity.rb1
-rw-r--r--changelogs/unreleased/48817-fix-mr-changes-discussion-navigation.yml5
-rw-r--r--changelogs/unreleased/_acet-fix-expanding-context-lines.yml5
-rw-r--r--changelogs/unreleased/_acet-fix-mr-autosave.yml5
-rw-r--r--changelogs/unreleased/_acet-fix-outdated-discussions.yml5
-rw-r--r--changelogs/unreleased/tz-mr-refactor-memory-reduction.yml5
-rw-r--r--locale/gitlab.pot3
-rw-r--r--spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb5
-rw-r--r--spec/javascripts/autosave_spec.js10
-rw-r--r--spec/javascripts/diffs/components/diff_line_gutter_content_spec.js8
-rw-r--r--spec/javascripts/diffs/components/diff_line_note_form_spec.js41
-rw-r--r--spec/javascripts/diffs/components/inline_diff_view_spec.js1
-rw-r--r--spec/javascripts/diffs/mock_data/diff_discussions.js11
-rw-r--r--spec/javascripts/diffs/store/getters_spec.js35
-rw-r--r--spec/javascripts/diffs/store/utils_spec.js20
-rw-r--r--spec/javascripts/notes/components/discussion_counter_spec.js2
-rw-r--r--spec/javascripts/notes/components/noteable_discussion_spec.js56
-rw-r--r--spec/javascripts/notes/mock_data.js84
-rw-r--r--spec/javascripts/notes/stores/getters_spec.js155
41 files changed, 816 insertions, 247 deletions
diff --git a/app/assets/javascripts/autosave.js b/app/assets/javascripts/autosave.js
index fa00a3cf386..e8c59fab609 100644
--- a/app/assets/javascripts/autosave.js
+++ b/app/assets/javascripts/autosave.js
@@ -53,4 +53,8 @@ export default class Autosave {
return window.localStorage.removeItem(this.key);
}
+
+ dispose() {
+ this.field.off('input');
+ }
}
diff --git a/app/assets/javascripts/diffs/components/diff_discussions.vue b/app/assets/javascripts/diffs/components/diff_discussions.vue
index 20483161033..e64d5511d78 100644
--- a/app/assets/javascripts/diffs/components/diff_discussions.vue
+++ b/app/assets/javascripts/diffs/components/diff_discussions.vue
@@ -30,6 +30,7 @@ export default {
:render-header="false"
:render-diff-file="false"
:always-expanded="true"
+ :discussions-by-diff-order="true"
/>
</ul>
</div>
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 ad838a32518..d184a76f038 100644
--- a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue
+++ b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue
@@ -71,13 +71,18 @@ export default {
required: false,
default: false,
},
+ discussions: {
+ type: Array,
+ required: false,
+ default: () => [],
+ },
},
computed: {
...mapState({
diffViewType: state => state.diffs.diffViewType,
diffFiles: state => state.diffs.diffFiles,
}),
- ...mapGetters(['isLoggedIn', 'discussionsByLineCode']),
+ ...mapGetters(['isLoggedIn']),
lineHref() {
return this.lineCode ? `#${this.lineCode}` : '#';
},
@@ -87,24 +92,19 @@ export default {
this.showCommentButton &&
!this.isMatchLine &&
!this.isContextLine &&
- !this.hasDiscussions &&
- !this.isMetaLine
+ !this.isMetaLine &&
+ !this.hasDiscussions
);
},
- discussions() {
- return this.discussionsByLineCode[this.lineCode] || [];
- },
hasDiscussions() {
return this.discussions.length > 0;
},
shouldShowAvatarsOnGutter() {
- let render = this.hasDiscussions && this.showCommentButton;
-
if (!this.lineType && this.linePosition === LINE_POSITION_RIGHT) {
- render = false;
+ return false;
}
- return render;
+ return this.hasDiscussions && this.showCommentButton;
},
},
methods: {
@@ -189,7 +189,6 @@ export default {
</button>
<a
v-if="lineNumber"
- v-once
:data-linenumber="lineNumber"
:href="lineHref"
>
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 32f9516d332..cbe4551d06b 100644
--- a/app/assets/javascripts/diffs/components/diff_line_note_form.vue
+++ b/app/assets/javascripts/diffs/components/diff_line_note_form.vue
@@ -1,17 +1,17 @@
<script>
-import $ from 'jquery';
import { mapState, mapGetters, mapActions } from 'vuex';
import createFlash from '~/flash';
import { s__ } from '~/locale';
import noteForm from '../../notes/components/note_form.vue';
import { getNoteFormData } from '../store/utils';
-import Autosave from '../../autosave';
-import { DIFF_NOTE_TYPE, NOTE_TYPE } from '../constants';
+import autosave from '../../notes/mixins/autosave';
+import { DIFF_NOTE_TYPE } from '../constants';
export default {
components: {
noteForm,
},
+ mixins: [autosave],
props: {
diffFileHash: {
type: String,
@@ -41,28 +41,35 @@ export default {
},
mounted() {
if (this.isLoggedIn) {
- const noteableData = this.getNoteableData;
const keys = [
- NOTE_TYPE,
- this.noteableType,
- noteableData.id,
- noteableData.diff_head_sha,
+ this.noteableData.diff_head_sha,
DIFF_NOTE_TYPE,
- noteableData.source_project_id,
+ this.noteableData.source_project_id,
this.line.lineCode,
];
- this.autosave = new Autosave($(this.$refs.noteForm.$refs.textarea), keys);
+ this.initAutoSave(this.noteableData, keys);
}
},
methods: {
...mapActions('diffs', ['cancelCommentForm']),
...mapActions(['saveNote', 'refetchDiscussionById']),
- handleCancelCommentForm() {
- this.autosave.reset();
+ handleCancelCommentForm(shouldConfirm, isDirty) {
+ if (shouldConfirm && isDirty) {
+ const msg = s__('Notes|Are you sure you want to cancel creating this comment?');
+
+ // eslint-disable-next-line no-alert
+ if (!window.confirm(msg)) {
+ return;
+ }
+ }
+
this.cancelCommentForm({
lineCode: this.line.lineCode,
});
+ this.$nextTick(() => {
+ this.resetAutoSave();
+ });
},
handleSaveNote(note) {
const selectedDiffFile = this.getDiffFileByHash(this.diffFileHash);
diff --git a/app/assets/javascripts/diffs/components/diff_table_cell.vue b/app/assets/javascripts/diffs/components/diff_table_cell.vue
index 5962f30d9bb..e8e8ddc6c5e 100644
--- a/app/assets/javascripts/diffs/components/diff_table_cell.vue
+++ b/app/assets/javascripts/diffs/components/diff_table_cell.vue
@@ -67,6 +67,11 @@ export default {
required: false,
default: false,
},
+ discussions: {
+ type: Array,
+ required: false,
+ default: () => [],
+ },
},
computed: {
...mapGetters(['isLoggedIn']),
@@ -136,6 +141,7 @@ export default {
: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 ca265dd892c..1b5ae5e9f75 100644
--- a/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue
+++ b/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue
@@ -1,5 +1,5 @@
<script>
-import { mapState, mapGetters } from 'vuex';
+import { mapState } from 'vuex';
import diffDiscussions from './diff_discussions.vue';
import diffLineNoteForm from './diff_line_note_form.vue';
@@ -21,18 +21,22 @@ export default {
type: Number,
required: true,
},
+ discussions: {
+ type: Array,
+ required: false,
+ default: () => [],
+ },
},
computed: {
...mapState({
diffLineCommentForms: state => state.diffs.diffLineCommentForms,
}),
- ...mapGetters(['discussionsByLineCode']),
- discussions() {
- return this.discussionsByLineCode[this.line.lineCode] || [];
- },
className() {
return this.discussions.length ? '' : 'js-temp-notes-holder';
},
+ hasCommentForm() {
+ return this.diffLineCommentForms[this.line.lineCode];
+ },
},
};
</script>
@@ -53,7 +57,7 @@ export default {
:discussions="discussions"
/>
<diff-line-note-form
- v-if="diffLineCommentForms[line.lineCode]"
+ v-if="hasCommentForm"
:diff-file-hash="diffFileHash"
:line="line"
:note-target-line="line"
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 0197a510ef1..32d65ff994f 100644
--- a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue
+++ b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue
@@ -33,6 +33,11 @@ export default {
required: false,
default: false,
},
+ discussions: {
+ type: Array,
+ required: false,
+ default: () => [],
+ },
},
data() {
return {
@@ -89,6 +94,7 @@ export default {
:is-bottom="isBottom"
:is-hover="isHover"
:show-comment-button="true"
+ :discussions="discussions"
class="diff-line-num old_line"
/>
<diff-table-cell
@@ -98,10 +104,10 @@ export default {
:line-type="newLineType"
:is-bottom="isBottom"
:is-hover="isHover"
+ :discussions="discussions"
class="diff-line-num new_line"
/>
<td
- v-once
:class="line.type"
class="line_content"
v-html="line.richText"
diff --git a/app/assets/javascripts/diffs/components/inline_diff_view.vue b/app/assets/javascripts/diffs/components/inline_diff_view.vue
index 9fd19b74cd7..5f30cc57a59 100644
--- a/app/assets/javascripts/diffs/components/inline_diff_view.vue
+++ b/app/assets/javascripts/diffs/components/inline_diff_view.vue
@@ -20,8 +20,11 @@ export default {
},
},
computed: {
- ...mapGetters('diffs', ['commitId']),
- ...mapGetters(['discussionsByLineCode']),
+ ...mapGetters('diffs', [
+ 'commitId',
+ 'shouldRenderInlineCommentRow',
+ 'singleDiscussionByLineCode',
+ ]),
...mapState({
diffLineCommentForms: state => state.diffs.diffLineCommentForms,
}),
@@ -35,18 +38,7 @@ export default {
return window.gon.user_color_scheme;
},
},
- methods: {
- shouldRenderCommentRow(line) {
- if (this.diffLineCommentForms[line.lineCode]) return true;
-
- const lineDiscussions = this.discussionsByLineCode[line.lineCode];
- if (lineDiscussions === undefined) {
- return false;
- }
-
- return lineDiscussions.every(discussion => discussion.expanded);
- },
- },
+ methods: {},
};
</script>
@@ -65,13 +57,15 @@ export default {
:line="line"
:is-bottom="index + 1 === diffLinesLength"
:key="line.lineCode"
+ :discussions="singleDiscussionByLineCode(line.lineCode)"
/>
<inline-diff-comment-row
- v-if="shouldRenderCommentRow(line)"
+ v-if="shouldRenderInlineCommentRow(line)"
:diff-file-hash="diffFile.fileHash"
:line="line"
:line-index="index"
:key="index"
+ :discussions="singleDiscussionByLineCode(line.lineCode)"
/>
</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 cc5248c25d9..bb9a65c83fa 100644
--- a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue
+++ b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue
@@ -1,5 +1,5 @@
<script>
-import { mapState, mapGetters } from 'vuex';
+import { mapState } from 'vuex';
import diffDiscussions from './diff_discussions.vue';
import diffLineNoteForm from './diff_line_note_form.vue';
@@ -21,48 +21,51 @@ 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,
}),
- ...mapGetters(['discussionsByLineCode']),
leftLineCode() {
return this.line.left.lineCode;
},
rightLineCode() {
return this.line.right.lineCode;
},
- hasDiscussion() {
- const discussions = this.discussionsByLineCode;
-
- return discussions[this.leftLineCode] || discussions[this.rightLineCode];
- },
hasExpandedDiscussionOnLeft() {
- const discussions = this.discussionsByLineCode[this.leftLineCode];
-
+ const discussions = this.leftDiscussions;
return discussions ? discussions.every(discussion => discussion.expanded) : false;
},
hasExpandedDiscussionOnRight() {
- const discussions = this.discussionsByLineCode[this.rightLineCode];
-
+ const discussions = this.rightDiscussions;
return discussions ? discussions.every(discussion => discussion.expanded) : false;
},
hasAnyExpandedDiscussion() {
return this.hasExpandedDiscussionOnLeft || this.hasExpandedDiscussionOnRight;
},
shouldRenderDiscussionsOnLeft() {
- return this.discussionsByLineCode[this.leftLineCode] && this.hasExpandedDiscussionOnLeft;
+ return this.leftDiscussions && this.hasExpandedDiscussionOnLeft;
},
shouldRenderDiscussionsOnRight() {
- return (
- this.discussionsByLineCode[this.rightLineCode] &&
- this.hasExpandedDiscussionOnRight &&
- this.line.right.type
- );
+ return this.rightDiscussions && this.hasExpandedDiscussionOnRight && this.line.right.type;
+ },
+ showRightSideCommentForm() {
+ return this.line.right.type && this.diffLineCommentForms[this.rightLineCode];
},
className() {
- return this.hasDiscussion ? '' : 'js-temp-notes-holder';
+ return this.leftDiscussions.length > 0 || this.rightDiscussions.length > 0
+ ? ''
+ : 'js-temp-notes-holder';
},
},
};
@@ -80,13 +83,12 @@ export default {
class="content"
>
<diff-discussions
- v-if="discussionsByLineCode[leftLineCode].length"
- :discussions="discussionsByLineCode[leftLineCode]"
+ v-if="leftDiscussions.length"
+ :discussions="leftDiscussions"
/>
</div>
<diff-line-note-form
- v-if="diffLineCommentForms[leftLineCode] &&
- diffLineCommentForms[leftLineCode]"
+ v-if="diffLineCommentForms[leftLineCode]"
:diff-file-hash="diffFileHash"
:line="line.left"
:note-target-line="line.left"
@@ -100,13 +102,12 @@ export default {
class="content"
>
<diff-discussions
- v-if="discussionsByLineCode[rightLineCode].length"
- :discussions="discussionsByLineCode[rightLineCode]"
+ v-if="rightDiscussions.length"
+ :discussions="rightDiscussions"
/>
</div>
<diff-line-note-form
- v-if="diffLineCommentForms[rightLineCode] &&
- diffLineCommentForms[rightLineCode] && line.right.type"
+ v-if="showRightSideCommentForm"
:diff-file-hash="diffFileHash"
:line="line.right"
:note-target-line="line.right"
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 ee5bb4d8d05..d4e54c2bd00 100644
--- a/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue
+++ b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue
@@ -36,6 +36,16 @@ export default {
required: false,
default: false,
},
+ leftDiscussions: {
+ type: Array,
+ required: false,
+ default: () => [],
+ },
+ rightDiscussions: {
+ type: Array,
+ required: false,
+ default: () => [],
+ },
},
data() {
return {
@@ -116,10 +126,10 @@ export default {
:is-hover="isLeftHover"
:show-comment-button="true"
:diff-view-type="parallelDiffViewType"
+ :discussions="leftDiscussions"
class="diff-line-num old_line"
/>
<td
- v-once
:id="line.left.lineCode"
:class="parallelViewLeftLineType"
class="line_content parallel left-side"
@@ -137,10 +147,10 @@ export default {
:is-hover="isRightHover"
:show-comment-button="true"
:diff-view-type="parallelDiffViewType"
+ :discussions="rightDiscussions"
class="diff-line-num new_line"
/>
<td
- v-once
:id="line.right.lineCode"
:class="line.right.type"
class="line_content parallel right-side"
diff --git a/app/assets/javascripts/diffs/components/parallel_diff_view.vue b/app/assets/javascripts/diffs/components/parallel_diff_view.vue
index 32528c9e7ab..4d97cb6d15d 100644
--- a/app/assets/javascripts/diffs/components/parallel_diff_view.vue
+++ b/app/assets/javascripts/diffs/components/parallel_diff_view.vue
@@ -21,8 +21,11 @@ export default {
},
},
computed: {
- ...mapGetters('diffs', ['commitId']),
- ...mapGetters(['discussionsByLineCode']),
+ ...mapGetters('diffs', [
+ 'commitId',
+ 'singleDiscussionByLineCode',
+ 'shouldRenderParallelCommentRow',
+ ]),
...mapState({
diffLineCommentForms: state => state.diffs.diffLineCommentForms,
}),
@@ -52,32 +55,6 @@ export default {
return window.gon.user_color_scheme;
},
},
- methods: {
- shouldRenderCommentRow(line) {
- const leftLineCode = line.left.lineCode;
- const rightLineCode = line.right.lineCode;
- const discussions = this.discussionsByLineCode;
- const leftDiscussions = discussions[leftLineCode];
- const rightDiscussions = discussions[rightLineCode];
- const hasDiscussion = leftDiscussions || rightDiscussions;
-
- const hasExpandedDiscussionOnLeft = leftDiscussions
- ? leftDiscussions.every(discussion => discussion.expanded)
- : false;
- const hasExpandedDiscussionOnRight = rightDiscussions
- ? rightDiscussions.every(discussion => discussion.expanded)
- : false;
-
- if (hasDiscussion && (hasExpandedDiscussionOnLeft || hasExpandedDiscussionOnRight)) {
- return true;
- }
-
- const hasCommentFormOnLeft = this.diffLineCommentForms[leftLineCode];
- const hasCommentFormOnRight = this.diffLineCommentForms[rightLineCode];
-
- return hasCommentFormOnLeft || hasCommentFormOnRight;
- },
- },
};
</script>
@@ -98,13 +75,17 @@ export default {
:line="line"
:is-bottom="index + 1 === diffLinesLength"
:key="index"
+ :left-discussions="singleDiscussionByLineCode(line.left.lineCode)"
+ :right-discussions="singleDiscussionByLineCode(line.right.lineCode)"
/>
<parallel-diff-comment-row
- v-if="shouldRenderCommentRow(line)"
+ v-if="shouldRenderParallelCommentRow(line)"
:key="`dcr-${index}`"
:line="line"
:diff-file-hash="diffFile.fileHash"
:line-index="index"
+ :left-discussions="singleDiscussionByLineCode(line.left.lineCode)"
+ :right-discussions="singleDiscussionByLineCode(line.right.lineCode)"
/>
</template>
</tbody>
diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js
index 855de79adf8..c7b9b1a16e6 100644
--- a/app/assets/javascripts/diffs/store/getters.js
+++ b/app/assets/javascripts/diffs/store/getters.js
@@ -1,5 +1,7 @@
import _ from 'underscore';
+import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
import { PARALLEL_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE } from '../constants';
+import { getDiffRefsByLineCode } from './utils';
export const isParallelView = state => state.diffViewType === PARALLEL_DIFF_VIEW_TYPE;
@@ -56,6 +58,87 @@ export const getDiffFileDiscussions = (state, getters, rootState, rootGetters) =
discussion.diff_discussion && _.isEqual(discussion.diff_file.file_hash, diff.fileHash),
) || [];
+/**
+ * Returns an Object with discussions by their diff line code
+ * To avoid rendering outdated discussions on the Changes tab we should do a bunch of SHA
+ * comparisions. `note.position.formatter` have the current version diff refs but
+ * `note.original_position.formatter` will have the first version's diff refs.
+ * If line diff refs matches with one of them, we should render it as a discussion on Changes tab.
+ *
+ * @param {Object} diff
+ * @returns {Array}
+ */
+export const discussionsByLineCode = (state, getters, rootState, rootGetters) => {
+ const diffRefsByLineCode = getDiffRefsByLineCode(state.diffFiles);
+
+ return rootGetters.discussions.reduce((acc, note) => {
+ const isDiffDiscussion = note.diff_discussion;
+ const hasLineCode = note.line_code;
+ const isResolvable = note.resolvable;
+
+ if (isDiffDiscussion && hasLineCode && isResolvable) {
+ const diffRefs = diffRefsByLineCode[note.line_code];
+ if (diffRefs) {
+ const refs = convertObjectPropsToCamelCase(note.position.formatter);
+ const originalRefs = convertObjectPropsToCamelCase(note.original_position.formatter);
+
+ if (_.isEqual(refs, diffRefs) || _.isEqual(originalRefs, diffRefs)) {
+ const lineCode = note.line_code;
+
+ if (acc[lineCode]) {
+ acc[lineCode].push(note);
+ } else {
+ acc[lineCode] = [note];
+ }
+ }
+ }
+ }
+
+ return acc;
+ }, {});
+};
+
+export const singleDiscussionByLineCode = (state, getters) => lineCode => {
+ if (!lineCode) return [];
+ const discussions = getters.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;
+
+ const hasExpandedDiscussionOnLeft = leftDiscussions.length
+ ? leftDiscussions.every(discussion => discussion.expanded)
+ : false;
+ const hasExpandedDiscussionOnRight = rightDiscussions.length
+ ? rightDiscussions.every(discussion => discussion.expanded)
+ : false;
+
+ if (hasDiscussion && (hasExpandedDiscussionOnLeft || hasExpandedDiscussionOnRight)) {
+ return true;
+ }
+
+ const hasCommentFormOnLeft = state.diffLineCommentForms[leftLineCode];
+ const hasCommentFormOnRight = state.diffLineCommentForms[rightLineCode];
+
+ return hasCommentFormOnLeft || hasCommentFormOnRight;
+};
+
+export const shouldRenderInlineCommentRow = (state, getters) => line => {
+ if (state.diffLineCommentForms[line.lineCode]) return true;
+
+ const lineDiscussions = getters.singleDiscussionByLineCode(line.lineCode);
+ if (lineDiscussions.length === 0) {
+ return false;
+ }
+
+ return lineDiscussions.every(discussion => discussion.expanded);
+};
+
// prevent babel-plugin-rewire from generating an invalid default during karma∂ tests
export const getDiffFileByHash = state => fileHash =>
state.diffFiles.find(file => file.fileHash === fileHash);
diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js
index d9589baa76e..82082ac508a 100644
--- a/app/assets/javascripts/diffs/store/utils.js
+++ b/app/assets/javascripts/diffs/store/utils.js
@@ -173,3 +173,24 @@ export function trimFirstCharOfLineContent(line = {}) {
return parsedLine;
}
+
+export function getDiffRefsByLineCode(diffFiles) {
+ return diffFiles.reduce((acc, diffFile) => {
+ const { baseSha, headSha, startSha } = diffFile.diffRefs;
+ const { newPath, oldPath } = diffFile;
+
+ // We can only use highlightedDiffLines to create the map of diff lines because
+ // highlightedDiffLines will also include every parallel diff line in it.
+ if (diffFile.highlightedDiffLines) {
+ diffFile.highlightedDiffLines.forEach(line => {
+ const { lineCode, oldLine, newLine } = line;
+
+ if (lineCode) {
+ acc[lineCode] = { baseSha, headSha, startSha, newPath, oldPath, oldLine, newLine };
+ }
+ });
+ }
+
+ return acc;
+ }, {});
+}
diff --git a/app/assets/javascripts/lib/utils/poll.js b/app/assets/javascripts/lib/utils/poll.js
index 7fca80c2fdb..91d8c30744f 100644
--- a/app/assets/javascripts/lib/utils/poll.js
+++ b/app/assets/javascripts/lib/utils/poll.js
@@ -38,7 +38,7 @@ import { normalizeHeaders } from './common_utils';
* } else {
* poll.stop();
* }
-* });
+ * });
*
* 1. Checks for response and headers before start polling
* 2. Interval is provided by `Poll-Interval` header.
@@ -51,8 +51,8 @@ export default class Poll {
constructor(options = {}) {
this.options = options;
this.options.data = options.data || {};
- this.options.notificationCallback = options.notificationCallback ||
- function notificationCallback() {};
+ this.options.notificationCallback =
+ options.notificationCallback || function notificationCallback() {};
this.intervalHeader = 'POLL-INTERVAL';
this.timeoutID = null;
@@ -63,6 +63,7 @@ export default class Poll {
const headers = normalizeHeaders(response.headers);
const pollInterval = parseInt(headers[this.intervalHeader], 10);
if (pollInterval > 0 && response.status === httpStatusCodes.OK && this.canPoll) {
+ clearTimeout(this.timeoutID);
this.timeoutID = setTimeout(() => {
this.makeRequest();
}, pollInterval);
@@ -77,11 +78,11 @@ export default class Poll {
notificationCallback(true);
return resource[method](data)
- .then((response) => {
+ .then(response => {
this.checkConditions(response);
notificationCallback(false);
})
- .catch((error) => {
+ .catch(error => {
notificationCallback(false);
if (error.status === httpStatusCodes.ABORTED) {
return;
diff --git a/app/assets/javascripts/notes/components/discussion_counter.vue b/app/assets/javascripts/notes/components/discussion_counter.vue
index 6385b75e557..ad6e7cf501d 100644
--- a/app/assets/javascripts/notes/components/discussion_counter.vue
+++ b/app/assets/javascripts/notes/components/discussion_counter.vue
@@ -5,19 +5,20 @@ import resolvedSvg from 'icons/_icon_status_success_solid.svg';
import mrIssueSvg from 'icons/_icon_mr_issue.svg';
import nextDiscussionSvg from 'icons/_next_discussion.svg';
import { pluralize } from '../../lib/utils/text_utility';
-import { scrollToElement } from '../../lib/utils/common_utils';
+import discussionNavigation from '../mixins/discussion_navigation';
import tooltip from '../../vue_shared/directives/tooltip';
export default {
directives: {
tooltip,
},
+ mixins: [discussionNavigation],
computed: {
...mapGetters([
'getUserData',
'getNoteableData',
'discussionCount',
- 'unresolvedDiscussions',
+ 'firstUnresolvedDiscussionId',
'resolvedDiscussionCount',
]),
isLoggedIn() {
@@ -35,11 +36,6 @@ export default {
resolveAllDiscussionsIssuePath() {
return this.getNoteableData.create_issue_to_resolve_discussions_path;
},
- firstUnresolvedDiscussionId() {
- const item = this.unresolvedDiscussions[0] || {};
-
- return item.id;
- },
},
created() {
this.resolveSvg = resolveSvg;
@@ -50,22 +46,10 @@ export default {
methods: {
...mapActions(['expandDiscussion']),
jumpToFirstUnresolvedDiscussion() {
- const discussionId = this.firstUnresolvedDiscussionId;
- if (!discussionId) {
- return;
- }
-
- const el = document.querySelector(`[data-discussion-id="${discussionId}"]`);
- const activeTab = window.mrTabs.currentAction;
-
- if (activeTab === 'commits' || activeTab === 'pipelines') {
- window.mrTabs.activateTab('show');
- }
+ const diffTab = window.mrTabs.currentAction === 'diffs';
+ const discussionId = this.firstUnresolvedDiscussionId(diffTab);
- if (el) {
- this.expandDiscussion({ discussionId });
- scrollToElement(el);
- }
+ this.jumpToDiscussion(discussionId);
},
},
};
diff --git a/app/assets/javascripts/notes/components/note_form.vue b/app/assets/javascripts/notes/components/note_form.vue
index 26482a02e00..abcd4422d7c 100644
--- a/app/assets/javascripts/notes/components/note_form.vue
+++ b/app/assets/javascripts/notes/components/note_form.vue
@@ -7,7 +7,7 @@ import issuableStateMixin from '../mixins/issuable_state';
import resolvable from '../mixins/resolvable';
export default {
- name: 'IssueNoteForm',
+ name: 'NoteForm',
components: {
issueWarning,
markdownField,
diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue
index bee635398b3..0fe1c16854a 100644
--- a/app/assets/javascripts/notes/components/noteable_discussion.vue
+++ b/app/assets/javascripts/notes/components/noteable_discussion.vue
@@ -1,11 +1,11 @@
<script>
-import _ from 'underscore';
import { mapActions, mapGetters } from 'vuex';
import resolveDiscussionsSvg from 'icons/_icon_mr_issue.svg';
import nextDiscussionsSvg from 'icons/_next_discussion.svg';
-import { convertObjectPropsToCamelCase, scrollToElement } from '~/lib/utils/common_utils';
+import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
import { truncateSha } from '~/lib/utils/text_utility';
import systemNote from '~/vue_shared/components/notes/system_note.vue';
+import { s__ } from '~/locale';
import Flash from '../../flash';
import { SYSTEM_NOTE } from '../constants';
import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue';
@@ -20,6 +20,7 @@ import placeholderSystemNote from '../../vue_shared/components/notes/placeholder
import autosave from '../mixins/autosave';
import noteable from '../mixins/noteable';
import resolvable from '../mixins/resolvable';
+import discussionNavigation from '../mixins/discussion_navigation';
import tooltip from '../../vue_shared/directives/tooltip';
export default {
@@ -39,7 +40,7 @@ export default {
directives: {
tooltip,
},
- mixins: [autosave, noteable, resolvable],
+ mixins: [autosave, noteable, resolvable, discussionNavigation],
props: {
discussion: {
type: Object,
@@ -60,6 +61,11 @@ export default {
required: false,
default: false,
},
+ discussionsByDiffOrder: {
+ type: Boolean,
+ required: false,
+ default: false,
+ },
},
data() {
return {
@@ -74,7 +80,12 @@ export default {
'discussionCount',
'resolvedDiscussionCount',
'allDiscussions',
+ 'unresolvedDiscussionsIdsByDiff',
+ 'unresolvedDiscussionsIdsByDate',
'unresolvedDiscussions',
+ 'unresolvedDiscussionsIdsOrdered',
+ 'nextUnresolvedDiscussionId',
+ 'isLastUnresolvedDiscussion',
]),
transformedDiscussion() {
return {
@@ -125,6 +136,10 @@ export default {
hasMultipleUnresolvedDiscussions() {
return this.unresolvedDiscussions.length > 1;
},
+ showJumpToNextDiscussion() {
+ return this.hasMultipleUnresolvedDiscussions &&
+ !this.isLastUnresolvedDiscussion(this.discussion.id, this.discussionsByDiffOrder);
+ },
shouldRenderDiffs() {
const { diffDiscussion, diffFile } = this.transformedDiscussion;
@@ -144,19 +159,17 @@ export default {
return this.isDiffDiscussion ? '' : 'card discussion-wrapper';
},
},
- mounted() {
- if (this.isReplying) {
- this.initAutoSave(this.transformedDiscussion);
- }
- },
- updated() {
- if (this.isReplying) {
- if (!this.autosave) {
- this.initAutoSave(this.transformedDiscussion);
+ watch: {
+ isReplying() {
+ if (this.isReplying) {
+ this.$nextTick(() => {
+ // Pass an extra key to separate reply and note edit forms
+ this.initAutoSave(this.transformedDiscussion, ['Reply']);
+ });
} else {
- this.setAutoSave();
+ this.disposeAutoSave();
}
- }
+ },
},
created() {
this.resolveDiscussionsSvg = resolveDiscussionsSvg;
@@ -194,16 +207,18 @@ export default {
showReplyForm() {
this.isReplying = true;
},
- cancelReplyForm(shouldConfirm) {
- if (shouldConfirm && this.$refs.noteForm.isDirty) {
+ cancelReplyForm(shouldConfirm, isDirty) {
+ if (shouldConfirm && isDirty) {
+ const msg = s__('Notes|Are you sure you want to cancel creating this comment?');
+
// eslint-disable-next-line no-alert
- if (!window.confirm('Are you sure you want to cancel creating this comment?')) {
+ if (!window.confirm(msg)) {
return;
}
}
- this.resetAutoSave();
this.isReplying = false;
+ this.resetAutoSave();
},
saveReply(noteText, form, callback) {
const postData = {
@@ -241,21 +256,10 @@ Please check your network connection and try again.`;
});
},
jumpToNextDiscussion() {
- const discussionIds = this.allDiscussions.map(d => d.id);
- const unresolvedIds = this.unresolvedDiscussions.map(d => d.id);
- const currentIndex = discussionIds.indexOf(this.discussion.id);
- const remainingAfterCurrent = discussionIds.slice(currentIndex + 1);
- const nextIndex = _.findIndex(remainingAfterCurrent, id => unresolvedIds.indexOf(id) > -1);
-
- if (nextIndex > -1) {
- const nextId = remainingAfterCurrent[nextIndex];
- const el = document.querySelector(`[data-discussion-id="${nextId}"]`);
+ const nextId =
+ this.nextUnresolvedDiscussionId(this.discussion.id, this.discussionsByDiffOrder);
- if (el) {
- this.expandDiscussion({ discussionId: nextId });
- scrollToElement(el);
- }
- }
+ this.jumpToDiscussion(nextId);
},
},
};
@@ -397,7 +401,7 @@ Please check your network connection and try again.`;
</a>
</div>
<div
- v-if="hasMultipleUnresolvedDiscussions"
+ v-if="showJumpToNextDiscussion"
class="btn-group"
role="group">
<button
@@ -420,7 +424,8 @@ Please check your network connection and try again.`;
:is-editing="false"
save-button-title="Comment"
@handleFormUpdate="saveReply"
- @cancelForm="cancelReplyForm" />
+ @cancelForm="cancelReplyForm"
+ />
<note-signed-out-widget v-if="!canReply" />
</div>
</div>
diff --git a/app/assets/javascripts/notes/mixins/autosave.js b/app/assets/javascripts/notes/mixins/autosave.js
index 36cc8d5d056..4f45f912479 100644
--- a/app/assets/javascripts/notes/mixins/autosave.js
+++ b/app/assets/javascripts/notes/mixins/autosave.js
@@ -4,12 +4,18 @@ import { capitalizeFirstCharacter } from '../../lib/utils/text_utility';
export default {
methods: {
- initAutoSave(noteable) {
- this.autosave = new Autosave($(this.$refs.noteForm.$refs.textarea), [
+ initAutoSave(noteable, extraKeys = []) {
+ let keys = [
'Note',
- capitalizeFirstCharacter(noteable.noteable_type),
+ capitalizeFirstCharacter(noteable.noteable_type || noteable.noteableType),
noteable.id,
- ]);
+ ];
+
+ if (extraKeys) {
+ keys = keys.concat(extraKeys);
+ }
+
+ this.autosave = new Autosave($(this.$refs.noteForm.$refs.textarea), keys);
},
resetAutoSave() {
this.autosave.reset();
@@ -17,5 +23,8 @@ export default {
setAutoSave() {
this.autosave.save();
},
+ disposeAutoSave() {
+ this.autosave.dispose();
+ },
},
};
diff --git a/app/assets/javascripts/notes/mixins/discussion_navigation.js b/app/assets/javascripts/notes/mixins/discussion_navigation.js
new file mode 100644
index 00000000000..f7c4deee1f8
--- /dev/null
+++ b/app/assets/javascripts/notes/mixins/discussion_navigation.js
@@ -0,0 +1,29 @@
+import { scrollToElement } from '~/lib/utils/common_utils';
+
+export default {
+ methods: {
+ jumpToDiscussion(id) {
+ if (id) {
+ const activeTab = window.mrTabs.currentAction;
+ const selector =
+ activeTab === 'diffs'
+ ? `ul.notes[data-discussion-id="${id}"]`
+ : `div.discussion[data-discussion-id="${id}"]`;
+ const el = document.querySelector(selector);
+
+ if (activeTab === 'commits' || activeTab === 'pipelines') {
+ window.mrTabs.activateTab('show');
+ }
+
+ if (el) {
+ this.expandDiscussion({ discussionId: id });
+
+ scrollToElement(el);
+ return true;
+ }
+ }
+
+ return false;
+ },
+ },
+};
diff --git a/app/assets/javascripts/notes/stores/getters.js b/app/assets/javascripts/notes/stores/getters.js
index 5c65e1c3bb5..0d8d197bf71 100644
--- a/app/assets/javascripts/notes/stores/getters.js
+++ b/app/assets/javascripts/notes/stores/getters.js
@@ -28,18 +28,6 @@ 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 noteableType = state => {
const { ISSUE_NOTEABLE_TYPE, MERGE_REQUEST_NOTEABLE_TYPE, EPIC_NOTEABLE_TYPE } = constants;
@@ -82,6 +70,9 @@ export const allDiscussions = (state, getters) => {
return Object.values(resolved).concat(unresolved);
};
+export const allResolvableDiscussions = (state, getters) =>
+ getters.allDiscussions.filter(d => !d.individual_note && d.resolvable);
+
export const resolvedDiscussionsById = state => {
const map = {};
@@ -98,6 +89,51 @@ export const resolvedDiscussionsById = state => {
return map;
};
+// Gets Discussions IDs ordered by the date of their initial note
+export const unresolvedDiscussionsIdsByDate = (state, getters) =>
+ getters.allResolvableDiscussions
+ .filter(d => !d.resolved)
+ .sort((a, b) => {
+ const aDate = new Date(a.notes[0].created_at);
+ const bDate = new Date(b.notes[0].created_at);
+
+ if (aDate < bDate) {
+ return -1;
+ }
+
+ return aDate === bDate ? 0 : 1;
+ })
+ .map(d => d.id);
+
+// Gets Discussions IDs ordered by their position in the diff
+//
+// Sorts the array of resolvable yet unresolved discussions by
+// comparing file names first. If file names are the same, compares
+// line numbers.
+export const unresolvedDiscussionsIdsByDiff = (state, getters) =>
+ getters.allResolvableDiscussions
+ .filter(d => !d.resolved)
+ .sort((a, b) => {
+ if (!a.diff_file || !b.diff_file) {
+ return 0;
+ }
+
+ // Get file names comparison result
+ const filenameComparison = a.diff_file.file_path.localeCompare(b.diff_file.file_path);
+
+ // Get the line numbers, to compare within the same file
+ const aLines = [a.position.formatter.new_line, a.position.formatter.old_line];
+ const bLines = [b.position.formatter.new_line, b.position.formatter.old_line];
+
+ return filenameComparison < 0 ||
+ (filenameComparison === 0 &&
+ // .max() because one of them might be zero (if removed/added)
+ Math.max(aLines[0], aLines[1]) < Math.max(bLines[0], bLines[1]))
+ ? -1
+ : 1;
+ })
+ .map(d => d.id);
+
export const resolvedDiscussionCount = (state, getters) => {
const resolvedMap = getters.resolvedDiscussionsById;
@@ -114,5 +150,42 @@ export const discussionTabCounter = state => {
return all.length;
};
+// Returns the list of discussion IDs ordered according to given parameter
+// @param {Boolean} diffOrder - is ordered by diff?
+export const unresolvedDiscussionsIdsOrdered = (state, getters) => diffOrder => {
+ if (diffOrder) {
+ return getters.unresolvedDiscussionsIdsByDiff;
+ }
+ return getters.unresolvedDiscussionsIdsByDate;
+};
+
+// Checks if a given discussion is the last in the current order (diff or date)
+// @param {Boolean} discussionId - id of the discussion
+// @param {Boolean} diffOrder - is ordered by diff?
+export const isLastUnresolvedDiscussion = (state, getters) => (discussionId, diffOrder) => {
+ const idsOrdered = getters.unresolvedDiscussionsIdsOrdered(diffOrder);
+ const lastDiscussionId = idsOrdered[idsOrdered.length - 1];
+
+ return lastDiscussionId === discussionId;
+};
+
+// Gets the ID of the discussion following the one provided, respecting order (diff or date)
+// @param {Boolean} discussionId - id of the current discussion
+// @param {Boolean} diffOrder - is ordered by diff?
+export const nextUnresolvedDiscussionId = (state, getters) => (discussionId, diffOrder) => {
+ const idsOrdered = getters.unresolvedDiscussionsIdsOrdered(diffOrder);
+ const currentIndex = idsOrdered.indexOf(discussionId);
+
+ return idsOrdered.slice(currentIndex + 1, currentIndex + 2)[0];
+};
+
+// @param {Boolean} diffOrder - is ordered by diff?
+export const firstUnresolvedDiscussionId = (state, getters) => diffOrder => {
+ if (diffOrder) {
+ return getters.unresolvedDiscussionsIdsByDiff[0];
+ }
+ return getters.unresolvedDiscussionsIdsByDate[0];
+};
+
// prevent babel-plugin-rewire from generating an invalid default during karma tests
export default () => {};
diff --git a/app/assets/javascripts/notes/stores/mutations.js b/app/assets/javascripts/notes/stores/mutations.js
index ab6a95e2601..e1b159142c9 100644
--- a/app/assets/javascripts/notes/stores/mutations.js
+++ b/app/assets/javascripts/notes/stores/mutations.js
@@ -174,27 +174,19 @@ export default {
[types.UPDATE_NOTE](state, note) {
const noteObj = utils.findNoteObjectById(state.discussions, note.discussion_id);
-
if (noteObj.individual_note) {
noteObj.notes.splice(0, 1, note);
} else {
const comment = utils.findNoteObjectById(noteObj.notes, note.id);
- noteObj.notes.splice(noteObj.notes.indexOf(comment), 1, note);
+ Object.assign(comment, note);
}
},
[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(n => n.id === note.id);
note.expanded = true; // override expand flag to prevent collapse
- state.discussions.splice(index, 1, note);
+ Object.assign(selectedDiscussion, note);
},
[types.CLOSE_ISSUE](state) {
@@ -215,12 +207,9 @@ export default {
[types.SET_DISCUSSION_DIFF_LINES](state, { discussionId, diffLines }) {
const discussion = utils.findNoteObjectById(state.discussions, discussionId);
- const index = state.discussions.indexOf(discussion);
- const discussionWithDiffLines = Object.assign({}, discussion, {
+ Object.assign(discussion, {
truncated_diff_lines: diffLines,
});
-
- state.discussions.splice(index, 1, discussionWithDiffLines);
},
};
diff --git a/app/assets/javascripts/notes/stores/utils.js b/app/assets/javascripts/notes/stores/utils.js
index a0e096ebfaf..c4a812c5af4 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.find(n => n.id === id);
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}`);
@@ -29,5 +27,4 @@ export const getQuickActionText = note => {
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/app/serializers/discussion_entity.rb b/app/serializers/discussion_entity.rb
index 8a39a4950f5..7505bbdeb3d 100644
--- a/app/serializers/discussion_entity.rb
+++ b/app/serializers/discussion_entity.rb
@@ -4,6 +4,7 @@ class DiscussionEntity < Grape::Entity
expose :id, :reply_id
expose :position, if: -> (d, _) { d.diff_discussion? && !d.legacy_diff_discussion? }
+ expose :original_position, if: -> (d, _) { d.diff_discussion? && !d.legacy_diff_discussion? }
expose :line_code, if: -> (d, _) { d.diff_discussion? }
expose :expanded?, as: :expanded
expose :active?, as: :active, if: -> (d, _) { d.diff_discussion? }
diff --git a/changelogs/unreleased/48817-fix-mr-changes-discussion-navigation.yml b/changelogs/unreleased/48817-fix-mr-changes-discussion-navigation.yml
new file mode 100644
index 00000000000..ec4b843b863
--- /dev/null
+++ b/changelogs/unreleased/48817-fix-mr-changes-discussion-navigation.yml
@@ -0,0 +1,5 @@
+---
+title: Fix navigation to First and Next discussion on MR Changes tab
+merge_request: 20434
+author:
+type: fixed
diff --git a/changelogs/unreleased/_acet-fix-expanding-context-lines.yml b/changelogs/unreleased/_acet-fix-expanding-context-lines.yml
new file mode 100644
index 00000000000..41b4dbca5d6
--- /dev/null
+++ b/changelogs/unreleased/_acet-fix-expanding-context-lines.yml
@@ -0,0 +1,5 @@
+---
+title: Fix rendering of the context lines in MR diffs page
+merge_request: 20642
+author:
+type: fixed
diff --git a/changelogs/unreleased/_acet-fix-mr-autosave.yml b/changelogs/unreleased/_acet-fix-mr-autosave.yml
new file mode 100644
index 00000000000..f87b32f68e2
--- /dev/null
+++ b/changelogs/unreleased/_acet-fix-mr-autosave.yml
@@ -0,0 +1,5 @@
+---
+title: Fix autosave and ESC confirmation issues for MR discussions
+merge_request: 20569
+author:
+type: fixed
diff --git a/changelogs/unreleased/_acet-fix-outdated-discussions.yml b/changelogs/unreleased/_acet-fix-outdated-discussions.yml
new file mode 100644
index 00000000000..d31483b4765
--- /dev/null
+++ b/changelogs/unreleased/_acet-fix-outdated-discussions.yml
@@ -0,0 +1,5 @@
+---
+title: Fix showing outdated discussions on Changes tab
+merge_request: 20445
+author:
+type: fixed
diff --git a/changelogs/unreleased/tz-mr-refactor-memory-reduction.yml b/changelogs/unreleased/tz-mr-refactor-memory-reduction.yml
new file mode 100644
index 00000000000..16003fa9cad
--- /dev/null
+++ b/changelogs/unreleased/tz-mr-refactor-memory-reduction.yml
@@ -0,0 +1,5 @@
+---
+title: Reduces the client side memory footprint on merge requests
+merge_request: 20744
+author:
+type: performance
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index f2e4eda576f..59dfa04a19d 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -3190,6 +3190,9 @@ msgstr ""
msgid "Note: Consider asking your GitLab administrator to configure %{github_integration_link}, which will allow login via GitHub and allow importing repositories without generating a Personal Access Token."
msgstr ""
+msgid "Notes|Are you sure you want to cancel creating this comment?"
+msgstr ""
+
msgid "Notification events"
msgstr ""
diff --git a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb
index a0b9d6cb059..1c7aa674955 100644
--- a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb
+++ b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb
@@ -342,8 +342,9 @@ describe 'Merge request > User resolves diff notes and discussions', :js do
end
end
- it 'shows jump to next discussion button' do
- expect(page.all('.discussion-reply-holder', count: 2)).to all(have_selector('.discussion-next-btn'))
+ it 'shows jump to next discussion button, apart from the last one' do
+ expect(page).to have_selector('.discussion-reply-holder', count: 2)
+ expect(page).to have_selector('.discussion-reply-holder .discussion-next-btn', count: 1)
end
it 'displays next discussion even if hidden' do
diff --git a/spec/javascripts/autosave_spec.js b/spec/javascripts/autosave_spec.js
index 38ae5b7e00c..dcb1c781591 100644
--- a/spec/javascripts/autosave_spec.js
+++ b/spec/javascripts/autosave_spec.js
@@ -59,12 +59,10 @@ describe('Autosave', () => {
Autosave.prototype.restore.call(autosave);
- expect(
- field.trigger,
- ).toHaveBeenCalled();
+ expect(field.trigger).toHaveBeenCalled();
});
- it('triggers native event', (done) => {
+ it('triggers native event', done => {
autosave.field.get(0).addEventListener('change', () => {
done();
});
@@ -81,9 +79,7 @@ describe('Autosave', () => {
it('does not trigger event', () => {
spyOn(field, 'trigger').and.callThrough();
- expect(
- field.trigger,
- ).not.toHaveBeenCalled();
+ expect(field.trigger).not.toHaveBeenCalled();
});
});
});
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 2d136a63c52..bdc94131fc2 100644
--- a/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js
+++ b/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js
@@ -18,10 +18,12 @@ describe('DiffLineGutterContent', () => {
};
const setDiscussions = component => {
component.$store.dispatch('setInitialNotes', getDiscussionsMockData());
+ component.$store.commit('diffs/SET_DIFF_DATA', { diffFiles: [getDiffFileMock()] });
};
const resetDiscussions = component => {
component.$store.dispatch('setInitialNotes', []);
+ component.$store.commit('diffs/SET_DIFF_DATA', {});
};
describe('computed', () => {
@@ -48,7 +50,11 @@ describe('DiffLineGutterContent', () => {
it('should return discussions for the given lineCode', () => {
const { lineCode } = getDiffFileMock().highlightedDiffLines[1];
- const component = createComponent({ lineCode, showCommentButton: true });
+ const component = createComponent({
+ lineCode,
+ showCommentButton: true,
+ discussions: getDiscussionsMockData(),
+ });
setDiscussions(component);
diff --git a/spec/javascripts/diffs/components/diff_line_note_form_spec.js b/spec/javascripts/diffs/components/diff_line_note_form_spec.js
index 4600aaea70b..6fe5fdaf7f9 100644
--- a/spec/javascripts/diffs/components/diff_line_note_form_spec.js
+++ b/spec/javascripts/diffs/components/diff_line_note_form_spec.js
@@ -3,6 +3,7 @@ import DiffLineNoteForm from '~/diffs/components/diff_line_note_form.vue';
import store from '~/mr_notes/stores';
import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
import diffFileMockData from '../mock_data/diff_file';
+import { noteableDataMock } from '../../notes/mock_data';
describe('DiffLineNoteForm', () => {
let component;
@@ -21,10 +22,9 @@ describe('DiffLineNoteForm', () => {
noteTargetLine: diffLines[0],
});
- Object.defineProperty(component, 'isLoggedIn', {
- get() {
- return true;
- },
+ Object.defineProperties(component, {
+ noteableData: { value: noteableDataMock },
+ isLoggedIn: { value: true },
});
component.$mount();
@@ -32,12 +32,37 @@ describe('DiffLineNoteForm', () => {
describe('methods', () => {
describe('handleCancelCommentForm', () => {
- it('should call cancelCommentForm with lineCode', () => {
+ it('should ask for confirmation when shouldConfirm and isDirty passed as truthy', () => {
+ spyOn(window, 'confirm').and.returnValue(false);
+
+ component.handleCancelCommentForm(true, true);
+ expect(window.confirm).toHaveBeenCalled();
+ });
+
+ it('should ask for confirmation when one of the params false', () => {
+ spyOn(window, 'confirm').and.returnValue(false);
+
+ component.handleCancelCommentForm(true, false);
+ expect(window.confirm).not.toHaveBeenCalled();
+
+ component.handleCancelCommentForm(false, true);
+ expect(window.confirm).not.toHaveBeenCalled();
+ });
+
+ it('should call cancelCommentForm with lineCode', done => {
+ spyOn(window, 'confirm');
spyOn(component, 'cancelCommentForm');
+ spyOn(component, 'resetAutoSave');
component.handleCancelCommentForm();
- expect(component.cancelCommentForm).toHaveBeenCalledWith({
- lineCode: diffLines[0].lineCode,
+ expect(window.confirm).not.toHaveBeenCalled();
+ component.$nextTick(() => {
+ expect(component.cancelCommentForm).toHaveBeenCalledWith({
+ lineCode: diffLines[0].lineCode,
+ });
+ expect(component.resetAutoSave).toHaveBeenCalled();
+
+ done();
});
});
});
@@ -66,7 +91,7 @@ describe('DiffLineNoteForm', () => {
describe('mounted', () => {
it('should init autosave', () => {
- const key = 'autosave/Note/issue///DiffNote//1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_1';
+ const key = 'autosave/Note/Issue/98//DiffNote//1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_1';
expect(component.autosave).toBeDefined();
expect(component.autosave.key).toEqual(key);
diff --git a/spec/javascripts/diffs/components/inline_diff_view_spec.js b/spec/javascripts/diffs/components/inline_diff_view_spec.js
index b02328dd359..90dfa5c5a58 100644
--- a/spec/javascripts/diffs/components/inline_diff_view_spec.js
+++ b/spec/javascripts/diffs/components/inline_diff_view_spec.js
@@ -33,6 +33,7 @@ describe('InlineDiffView', () => {
it('should render discussions', done => {
const el = component.$el;
component.$store.dispatch('setInitialNotes', getDiscussionsMockData());
+ component.$store.commit('diffs/SET_DIFF_DATA', { diffFiles: [getDiffFileMock()] });
Vue.nextTick(() => {
expect(el.querySelectorAll('.notes_holder').length).toEqual(1);
diff --git a/spec/javascripts/diffs/mock_data/diff_discussions.js b/spec/javascripts/diffs/mock_data/diff_discussions.js
index 41d0dfd8939..8cd57d2248b 100644
--- a/spec/javascripts/diffs/mock_data/diff_discussions.js
+++ b/spec/javascripts/diffs/mock_data/diff_discussions.js
@@ -12,6 +12,17 @@ export default {
head_sha: 'c48ee0d1bf3b30453f5b32250ce03134beaa6d13',
},
},
+ original_position: {
+ formatter: {
+ old_line: null,
+ new_line: 2,
+ old_path: 'CHANGELOG',
+ new_path: 'CHANGELOG',
+ base_sha: 'e63f41fe459e62e1228fcef60d7189127aeba95a',
+ start_sha: 'd9eaefe5a676b820c57ff18cf5b68316025f7962',
+ head_sha: 'c48ee0d1bf3b30453f5b32250ce03134beaa6d13',
+ },
+ },
line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_2',
expanded: true,
notes: [
diff --git a/spec/javascripts/diffs/store/getters_spec.js b/spec/javascripts/diffs/store/getters_spec.js
index 6210d4a7124..f5628a01a55 100644
--- a/spec/javascripts/diffs/store/getters_spec.js
+++ b/spec/javascripts/diffs/store/getters_spec.js
@@ -2,6 +2,7 @@ import * as getters from '~/diffs/store/getters';
import state from '~/diffs/store/modules/diff_state';
import { PARALLEL_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE } from '~/diffs/constants';
import discussion from '../mock_data/diff_discussions';
+import diffFile from '../mock_data/diff_file';
describe('Diffs Module Getters', () => {
let localState;
@@ -203,4 +204,38 @@ describe('Diffs Module Getters', () => {
expect(getters.getDiffFileByHash(localState)('123')).toBeUndefined();
});
});
+
+ describe('discussionsByLineCode', () => {
+ let mockState;
+
+ beforeEach(() => {
+ mockState = { diffFiles: [diffFile] };
+ });
+
+ it('should return a map of diff lines with their line codes', () => {
+ const mockGetters = { discussions: [discussionMock] };
+
+ const map = getters.discussionsByLineCode(mockState, {}, {}, mockGetters);
+ expect(map['1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_2']).toBeDefined();
+ expect(Object.keys(map).length).toEqual(1);
+ });
+
+ it('should have the diff discussion on the map if the original position matches', () => {
+ discussionMock.position.formatter.base_sha = 'ff9200';
+ const mockGetters = { discussions: [discussionMock] };
+
+ const map = getters.discussionsByLineCode(mockState, {}, {}, mockGetters);
+ expect(map['1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_2']).toBeDefined();
+ expect(Object.keys(map).length).toEqual(1);
+ });
+
+ it('should not add an outdated diff discussion to the returned map', () => {
+ discussionMock.position.formatter.base_sha = 'ff9200';
+ discussionMock.original_position.formatter.base_sha = 'ff9200';
+ const mockGetters = { discussions: [discussionMock] };
+
+ const map = getters.discussionsByLineCode(mockState, {}, {}, mockGetters);
+ expect(Object.keys(map).length).toEqual(0);
+ });
+ });
});
diff --git a/spec/javascripts/diffs/store/utils_spec.js b/spec/javascripts/diffs/store/utils_spec.js
index 32136d9ebff..8e7bd8afca4 100644
--- a/spec/javascripts/diffs/store/utils_spec.js
+++ b/spec/javascripts/diffs/store/utils_spec.js
@@ -207,4 +207,24 @@ describe('DiffsStoreUtils', () => {
expect(utils.trimFirstCharOfLineContent()).toEqual({});
});
});
+
+ describe('getDiffRefsByLineCode', () => {
+ it('should return diffRefs for all highlightedDiffLines', () => {
+ const diffFile = getDiffFileMock();
+ const map = utils.getDiffRefsByLineCode([diffFile]);
+ const { highlightedDiffLines } = diffFile;
+ const lineCodeCount = highlightedDiffLines.reduce(
+ (acc, line) => (line.lineCode ? acc + 1 : acc),
+ 0,
+ );
+
+ const { baseSha, headSha, startSha } = diffFile.diffRefs;
+ const targetLine = map[highlightedDiffLines[4].lineCode];
+
+ expect(Object.keys(map).length).toEqual(lineCodeCount);
+ expect(targetLine.baseSha).toEqual(baseSha);
+ expect(targetLine.headSha).toEqual(headSha);
+ expect(targetLine.startSha).toEqual(startSha);
+ });
+ });
});
diff --git a/spec/javascripts/notes/components/discussion_counter_spec.js b/spec/javascripts/notes/components/discussion_counter_spec.js
index a3869cc6498..d09bc5037ef 100644
--- a/spec/javascripts/notes/components/discussion_counter_spec.js
+++ b/spec/javascripts/notes/components/discussion_counter_spec.js
@@ -46,7 +46,7 @@ describe('DiscussionCounter component', () => {
discussions,
});
setFixtures(`
- <div data-discussion-id="${firstDiscussionId}"></div>
+ <div class="discussion" data-discussion-id="${firstDiscussionId}"></div>
`);
vm.jumpToFirstUnresolvedDiscussion();
diff --git a/spec/javascripts/notes/components/noteable_discussion_spec.js b/spec/javascripts/notes/components/noteable_discussion_spec.js
index 7da931fd9cb..2a01bd85520 100644
--- a/spec/javascripts/notes/components/noteable_discussion_spec.js
+++ b/spec/javascripts/notes/components/noteable_discussion_spec.js
@@ -14,6 +14,7 @@ describe('noteable_discussion component', () => {
preloadFixtures(discussionWithTwoUnresolvedNotes);
beforeEach(() => {
+ window.mrTabs = {};
store = createStore();
store.dispatch('setNoteableData', noteableDataMock);
store.dispatch('setNotesData', notesDataMock);
@@ -46,10 +47,15 @@ describe('noteable_discussion component', () => {
it('should toggle reply form', done => {
vm.$el.querySelector('.js-vue-discussion-reply').click();
+
Vue.nextTick(() => {
- expect(vm.$refs.noteForm).not.toBeNull();
expect(vm.isReplying).toEqual(true);
- done();
+
+ // There is a watcher for `isReplying` which will init autosave in the next tick
+ Vue.nextTick(() => {
+ expect(vm.$refs.noteForm).not.toBeNull();
+ done();
+ });
});
});
@@ -101,33 +107,29 @@ describe('noteable_discussion component', () => {
describe('methods', () => {
describe('jumpToNextDiscussion', () => {
- it('expands next unresolved discussion', () => {
- spyOn(vm, 'expandDiscussion').and.stub();
- const discussions = [
- discussionMock,
- {
- ...discussionMock,
- id: discussionMock.id + 1,
- notes: [{ ...discussionMock.notes[0], resolvable: true, resolved: true }],
- },
- {
- ...discussionMock,
- id: discussionMock.id + 2,
- notes: [{ ...discussionMock.notes[0], resolvable: true, resolved: false }],
- },
- ];
- const nextDiscussionId = discussionMock.id + 2;
- store.replaceState({
- ...store.state,
- discussions,
- });
- setFixtures(`
- <div data-discussion-id="${nextDiscussionId}"></div>
- `);
+ it('expands next unresolved discussion', done => {
+ const discussion2 = getJSONFixture(discussionWithTwoUnresolvedNotes)[0];
+ discussion2.resolved = false;
+ discussion2.id = 'next'; // prepare this for being identified as next one (to be jumped to)
+ vm.$store.dispatch('setInitialNotes', [discussionMock, discussion2]);
+ window.mrTabs.currentAction = 'show';
+
+ Vue.nextTick()
+ .then(() => {
+ spyOn(vm, 'expandDiscussion').and.stub();
+
+ const nextDiscussionId = discussion2.id;
+
+ setFixtures(`
+ <div class="discussion" data-discussion-id="${nextDiscussionId}"></div>
+ `);
- vm.jumpToNextDiscussion();
+ vm.jumpToNextDiscussion();
- expect(vm.expandDiscussion).toHaveBeenCalledWith({ discussionId: nextDiscussionId });
+ expect(vm.expandDiscussion).toHaveBeenCalledWith({ discussionId: nextDiscussionId });
+ })
+ .then(done)
+ .catch(done.fail);
});
});
});
diff --git a/spec/javascripts/notes/mock_data.js b/spec/javascripts/notes/mock_data.js
index be2a8ba67fe..67f6a9629d9 100644
--- a/spec/javascripts/notes/mock_data.js
+++ b/spec/javascripts/notes/mock_data.js
@@ -1168,3 +1168,87 @@ export const collapsedSystemNotes = [
diff_discussion: false,
},
];
+
+export const discussion1 = {
+ id: 'abc1',
+ resolvable: true,
+ resolved: false,
+ diff_file: {
+ file_path: 'about.md',
+ },
+ position: {
+ formatter: {
+ new_line: 50,
+ old_line: null,
+ },
+ },
+ notes: [
+ {
+ created_at: '2018-07-04T16:25:41.749Z',
+ },
+ ],
+};
+
+export const resolvedDiscussion1 = {
+ id: 'abc1',
+ resolvable: true,
+ resolved: true,
+ diff_file: {
+ file_path: 'about.md',
+ },
+ position: {
+ formatter: {
+ new_line: 50,
+ old_line: null,
+ },
+ },
+ notes: [
+ {
+ created_at: '2018-07-04T16:25:41.749Z',
+ },
+ ],
+};
+
+export const discussion2 = {
+ id: 'abc2',
+ resolvable: true,
+ resolved: false,
+ diff_file: {
+ file_path: 'README.md',
+ },
+ position: {
+ formatter: {
+ new_line: null,
+ old_line: 20,
+ },
+ },
+ notes: [
+ {
+ created_at: '2018-07-04T12:05:41.749Z',
+ },
+ ],
+};
+
+export const discussion3 = {
+ id: 'abc3',
+ resolvable: true,
+ resolved: false,
+ diff_file: {
+ file_path: 'README.md',
+ },
+ position: {
+ formatter: {
+ new_line: 21,
+ old_line: null,
+ },
+ },
+ notes: [
+ {
+ created_at: '2018-07-05T17:25:41.749Z',
+ },
+ ],
+};
+
+export const unresolvableDiscussion = {
+ resolvable: false,
+};
diff --git a/spec/javascripts/notes/stores/getters_spec.js b/spec/javascripts/notes/stores/getters_spec.js
index 41599e00122..7f8ede51508 100644
--- a/spec/javascripts/notes/stores/getters_spec.js
+++ b/spec/javascripts/notes/stores/getters_spec.js
@@ -5,6 +5,11 @@ import {
noteableDataMock,
individualNote,
collapseNotesMock,
+ discussion1,
+ discussion2,
+ discussion3,
+ resolvedDiscussion1,
+ unresolvableDiscussion,
} from '../mock_data';
const discussionWithTwoUnresolvedNotes = 'merge_requests/resolved_diff_discussion.json';
@@ -109,4 +114,154 @@ describe('Getters Notes Store', () => {
expect(getters.isNotesFetched(state)).toBeFalsy();
});
});
+
+ describe('allResolvableDiscussions', () => {
+ it('should return only resolvable discussions in same order', () => {
+ const localGetters = {
+ allDiscussions: [
+ discussion3,
+ unresolvableDiscussion,
+ discussion1,
+ unresolvableDiscussion,
+ discussion2,
+ ],
+ };
+
+ expect(getters.allResolvableDiscussions(state, localGetters)).toEqual([
+ discussion3,
+ discussion1,
+ discussion2,
+ ]);
+ });
+
+ it('should return empty array if there are no resolvable discussions', () => {
+ const localGetters = {
+ allDiscussions: [unresolvableDiscussion, unresolvableDiscussion],
+ };
+
+ expect(getters.allResolvableDiscussions(state, localGetters)).toEqual([]);
+ });
+ });
+
+ describe('unresolvedDiscussionsIdsByDiff', () => {
+ it('should return all discussions IDs in diff order', () => {
+ const localGetters = {
+ allResolvableDiscussions: [discussion3, discussion1, discussion2],
+ };
+
+ expect(getters.unresolvedDiscussionsIdsByDiff(state, localGetters)).toEqual([
+ 'abc1',
+ 'abc2',
+ 'abc3',
+ ]);
+ });
+
+ it('should return empty array if all discussions have been resolved', () => {
+ const localGetters = {
+ allResolvableDiscussions: [resolvedDiscussion1],
+ };
+
+ expect(getters.unresolvedDiscussionsIdsByDiff(state, localGetters)).toEqual([]);
+ });
+ });
+
+ describe('unresolvedDiscussionsIdsByDate', () => {
+ it('should return all discussions in date ascending order', () => {
+ const localGetters = {
+ allResolvableDiscussions: [discussion3, discussion1, discussion2],
+ };
+
+ expect(getters.unresolvedDiscussionsIdsByDate(state, localGetters)).toEqual([
+ 'abc2',
+ 'abc1',
+ 'abc3',
+ ]);
+ });
+
+ it('should return empty array if all discussions have been resolved', () => {
+ const localGetters = {
+ allResolvableDiscussions: [resolvedDiscussion1],
+ };
+
+ expect(getters.unresolvedDiscussionsIdsByDate(state, localGetters)).toEqual([]);
+ });
+ });
+
+ describe('unresolvedDiscussionsIdsOrdered', () => {
+ const localGetters = {
+ unresolvedDiscussionsIdsByDate: ['123', '456'],
+ unresolvedDiscussionsIdsByDiff: ['abc', 'def'],
+ };
+
+ it('should return IDs ordered by diff when diffOrder param is true', () => {
+ expect(getters.unresolvedDiscussionsIdsOrdered(state, localGetters)(true)).toEqual([
+ 'abc',
+ 'def',
+ ]);
+ });
+
+ it('should return IDs ordered by date when diffOrder param is not true', () => {
+ expect(getters.unresolvedDiscussionsIdsOrdered(state, localGetters)(false)).toEqual([
+ '123',
+ '456',
+ ]);
+ expect(getters.unresolvedDiscussionsIdsOrdered(state, localGetters)(undefined)).toEqual([
+ '123',
+ '456',
+ ]);
+ });
+ });
+
+ describe('isLastUnresolvedDiscussion', () => {
+ const localGetters = {
+ unresolvedDiscussionsIdsOrdered: () => ['123', '456', '789'],
+ };
+
+ it('should return true if the discussion id provided is the last', () => {
+ expect(getters.isLastUnresolvedDiscussion(state, localGetters)('789')).toBe(true);
+ });
+
+ it('should return false if the discussion id provided is not the last', () => {
+ expect(getters.isLastUnresolvedDiscussion(state, localGetters)('123')).toBe(false);
+ expect(getters.isLastUnresolvedDiscussion(state, localGetters)('456')).toBe(false);
+ });
+ });
+
+ describe('nextUnresolvedDiscussionId', () => {
+ const localGetters = {
+ unresolvedDiscussionsIdsOrdered: () => ['123', '456', '789'],
+ };
+
+ it('should return the ID of the discussion after the ID provided', () => {
+ expect(getters.nextUnresolvedDiscussionId(state, localGetters)('123')).toBe('456');
+ expect(getters.nextUnresolvedDiscussionId(state, localGetters)('456')).toBe('789');
+ expect(getters.nextUnresolvedDiscussionId(state, localGetters)('789')).toBe(undefined);
+ });
+ });
+
+ describe('firstUnresolvedDiscussionId', () => {
+ const localGetters = {
+ unresolvedDiscussionsIdsByDate: ['123', '456'],
+ unresolvedDiscussionsIdsByDiff: ['abc', 'def'],
+ };
+
+ it('should return the first discussion id by diff when diffOrder param is true', () => {
+ expect(getters.firstUnresolvedDiscussionId(state, localGetters)(true)).toBe('abc');
+ });
+
+ it('should return the first discussion id by date when diffOrder param is not true', () => {
+ expect(getters.firstUnresolvedDiscussionId(state, localGetters)(false)).toBe('123');
+ expect(getters.firstUnresolvedDiscussionId(state, localGetters)(undefined)).toBe('123');
+ });
+
+ it('should be falsy if all discussions are resolved', () => {
+ const localGettersFalsy = {
+ unresolvedDiscussionsIdsByDiff: [],
+ unresolvedDiscussionsIdsByDate: [],
+ };
+
+ expect(getters.firstUnresolvedDiscussionId(state, localGettersFalsy)(true)).toBeFalsy();
+ expect(getters.firstUnresolvedDiscussionId(state, localGettersFalsy)(false)).toBeFalsy();
+ });
+ });
});