Commit 47244ad5 authored by Filipa Lacerda's avatar Filipa Lacerda

Merge branch 'andr3-remove-mr-regressions-fixes-from-master' into 'master'

Remove fixes for MR refactor regressions from master

See merge request gitlab-org/gitlab-ce!20920
parents 5f742eb9 09c1b008
...@@ -53,8 +53,4 @@ export default class Autosave { ...@@ -53,8 +53,4 @@ export default class Autosave {
return window.localStorage.removeItem(this.key); return window.localStorage.removeItem(this.key);
} }
dispose() {
this.field.off('input');
}
} }
...@@ -30,7 +30,6 @@ export default { ...@@ -30,7 +30,6 @@ export default {
:render-header="false" :render-header="false"
:render-diff-file="false" :render-diff-file="false"
:always-expanded="true" :always-expanded="true"
:discussions-by-diff-order="true"
/> />
</ul> </ul>
</div> </div>
......
...@@ -71,18 +71,13 @@ export default { ...@@ -71,18 +71,13 @@ export default {
required: false, required: false,
default: false, default: false,
}, },
discussions: {
type: Array,
required: false,
default: () => [],
},
}, },
computed: { computed: {
...mapState({ ...mapState({
diffViewType: state => state.diffs.diffViewType, diffViewType: state => state.diffs.diffViewType,
diffFiles: state => state.diffs.diffFiles, diffFiles: state => state.diffs.diffFiles,
}), }),
...mapGetters(['isLoggedIn']), ...mapGetters(['isLoggedIn', 'discussionsByLineCode']),
lineHref() { lineHref() {
return this.lineCode ? `#${this.lineCode}` : '#'; return this.lineCode ? `#${this.lineCode}` : '#';
}, },
...@@ -92,19 +87,24 @@ export default { ...@@ -92,19 +87,24 @@ export default {
this.showCommentButton && this.showCommentButton &&
!this.isMatchLine && !this.isMatchLine &&
!this.isContextLine && !this.isContextLine &&
!this.isMetaLine && !this.hasDiscussions &&
!this.hasDiscussions !this.isMetaLine
); );
}, },
discussions() {
return this.discussionsByLineCode[this.lineCode] || [];
},
hasDiscussions() { hasDiscussions() {
return this.discussions.length > 0; return this.discussions.length > 0;
}, },
shouldShowAvatarsOnGutter() { shouldShowAvatarsOnGutter() {
let render = this.hasDiscussions && this.showCommentButton;
if (!this.lineType && this.linePosition === LINE_POSITION_RIGHT) { if (!this.lineType && this.linePosition === LINE_POSITION_RIGHT) {
return false; render = false;
} }
return this.hasDiscussions && this.showCommentButton; return render;
}, },
}, },
methods: { methods: {
...@@ -189,6 +189,7 @@ export default { ...@@ -189,6 +189,7 @@ export default {
</button> </button>
<a <a
v-if="lineNumber" v-if="lineNumber"
v-once
:data-linenumber="lineNumber" :data-linenumber="lineNumber"
:href="lineHref" :href="lineHref"
> >
......
<script> <script>
import $ from 'jquery';
import { mapState, mapGetters, mapActions } from 'vuex'; import { mapState, mapGetters, mapActions } from 'vuex';
import createFlash from '~/flash'; import createFlash from '~/flash';
import { s__ } from '~/locale'; import { s__ } from '~/locale';
import noteForm from '../../notes/components/note_form.vue'; import noteForm from '../../notes/components/note_form.vue';
import { getNoteFormData } from '../store/utils'; import { getNoteFormData } from '../store/utils';
import autosave from '../../notes/mixins/autosave'; import Autosave from '../../autosave';
import { DIFF_NOTE_TYPE } from '../constants'; import { DIFF_NOTE_TYPE, NOTE_TYPE } from '../constants';
export default { export default {
components: { components: {
noteForm, noteForm,
}, },
mixins: [autosave],
props: { props: {
diffFileHash: { diffFileHash: {
type: String, type: String,
...@@ -41,35 +41,28 @@ export default { ...@@ -41,35 +41,28 @@ export default {
}, },
mounted() { mounted() {
if (this.isLoggedIn) { if (this.isLoggedIn) {
const noteableData = this.getNoteableData;
const keys = [ const keys = [
this.noteableData.diff_head_sha, NOTE_TYPE,
this.noteableType,
noteableData.id,
noteableData.diff_head_sha,
DIFF_NOTE_TYPE, DIFF_NOTE_TYPE,
this.noteableData.source_project_id, noteableData.source_project_id,
this.line.lineCode, this.line.lineCode,
]; ];
this.initAutoSave(this.noteableData, keys); this.autosave = new Autosave($(this.$refs.noteForm.$refs.textarea), keys);
} }
}, },
methods: { methods: {
...mapActions('diffs', ['cancelCommentForm']), ...mapActions('diffs', ['cancelCommentForm']),
...mapActions(['saveNote', 'refetchDiscussionById']), ...mapActions(['saveNote', 'refetchDiscussionById']),
handleCancelCommentForm(shouldConfirm, isDirty) { handleCancelCommentForm() {
if (shouldConfirm && isDirty) { this.autosave.reset();
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({ this.cancelCommentForm({
lineCode: this.line.lineCode, lineCode: this.line.lineCode,
}); });
this.$nextTick(() => {
this.resetAutoSave();
});
}, },
handleSaveNote(note) { handleSaveNote(note) {
const selectedDiffFile = this.getDiffFileByHash(this.diffFileHash); const selectedDiffFile = this.getDiffFileByHash(this.diffFileHash);
......
...@@ -67,11 +67,6 @@ export default { ...@@ -67,11 +67,6 @@ export default {
required: false, required: false,
default: false, default: false,
}, },
discussions: {
type: Array,
required: false,
default: () => [],
},
}, },
computed: { computed: {
...mapGetters(['isLoggedIn']), ...mapGetters(['isLoggedIn']),
...@@ -141,7 +136,6 @@ export default { ...@@ -141,7 +136,6 @@ export default {
:is-match-line="isMatchLine" :is-match-line="isMatchLine"
:is-context-line="isContentLine" :is-context-line="isContentLine"
:is-meta-line="isMetaLine" :is-meta-line="isMetaLine"
:discussions="discussions"
/> />
</td> </td>
</template> </template>
<script> <script>
import { mapState } from 'vuex'; import { mapState, mapGetters } from 'vuex';
import diffDiscussions from './diff_discussions.vue'; import diffDiscussions from './diff_discussions.vue';
import diffLineNoteForm from './diff_line_note_form.vue'; import diffLineNoteForm from './diff_line_note_form.vue';
...@@ -21,22 +21,18 @@ export default { ...@@ -21,22 +21,18 @@ export default {
type: Number, type: Number,
required: true, required: true,
}, },
discussions: {
type: Array,
required: false,
default: () => [],
},
}, },
computed: { computed: {
...mapState({ ...mapState({
diffLineCommentForms: state => state.diffs.diffLineCommentForms, diffLineCommentForms: state => state.diffs.diffLineCommentForms,
}), }),
...mapGetters(['discussionsByLineCode']),
discussions() {
return this.discussionsByLineCode[this.line.lineCode] || [];
},
className() { className() {
return this.discussions.length ? '' : 'js-temp-notes-holder'; return this.discussions.length ? '' : 'js-temp-notes-holder';
}, },
hasCommentForm() {
return this.diffLineCommentForms[this.line.lineCode];
},
}, },
}; };
</script> </script>
...@@ -57,7 +53,7 @@ export default { ...@@ -57,7 +53,7 @@ export default {
:discussions="discussions" :discussions="discussions"
/> />
<diff-line-note-form <diff-line-note-form
v-if="hasCommentForm" v-if="diffLineCommentForms[line.lineCode]"
:diff-file-hash="diffFileHash" :diff-file-hash="diffFileHash"
:line="line" :line="line"
:note-target-line="line" :note-target-line="line"
......
...@@ -33,11 +33,6 @@ export default { ...@@ -33,11 +33,6 @@ export default {
required: false, required: false,
default: false, default: false,
}, },
discussions: {
type: Array,
required: false,
default: () => [],
},
}, },
data() { data() {
return { return {
...@@ -94,7 +89,6 @@ export default { ...@@ -94,7 +89,6 @@ export default {
:is-bottom="isBottom" :is-bottom="isBottom"
:is-hover="isHover" :is-hover="isHover"
:show-comment-button="true" :show-comment-button="true"
:discussions="discussions"
class="diff-line-num old_line" class="diff-line-num old_line"
/> />
<diff-table-cell <diff-table-cell
...@@ -104,10 +98,10 @@ export default { ...@@ -104,10 +98,10 @@ export default {
:line-type="newLineType" :line-type="newLineType"
:is-bottom="isBottom" :is-bottom="isBottom"
:is-hover="isHover" :is-hover="isHover"
:discussions="discussions"
class="diff-line-num new_line" class="diff-line-num new_line"
/> />
<td <td
v-once
:class="line.type" :class="line.type"
class="line_content" class="line_content"
v-html="line.richText" v-html="line.richText"
......
...@@ -20,11 +20,8 @@ export default { ...@@ -20,11 +20,8 @@ export default {
}, },
}, },
computed: { computed: {
...mapGetters('diffs', [ ...mapGetters('diffs', ['commitId']),
'commitId', ...mapGetters(['discussionsByLineCode']),
'shouldRenderInlineCommentRow',
'singleDiscussionByLineCode',
]),
...mapState({ ...mapState({
diffLineCommentForms: state => state.diffs.diffLineCommentForms, diffLineCommentForms: state => state.diffs.diffLineCommentForms,
}), }),
...@@ -38,7 +35,18 @@ export default { ...@@ -38,7 +35,18 @@ export default {
return window.gon.user_color_scheme; return window.gon.user_color_scheme;
}, },
}, },
methods: {}, 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);
},
},
}; };
</script> </script>
...@@ -57,15 +65,13 @@ export default { ...@@ -57,15 +65,13 @@ export default {
:line="line" :line="line"
:is-bottom="index + 1 === diffLinesLength" :is-bottom="index + 1 === diffLinesLength"
:key="line.lineCode" :key="line.lineCode"
:discussions="singleDiscussionByLineCode(line.lineCode)"
/> />
<inline-diff-comment-row <inline-diff-comment-row
v-if="shouldRenderInlineCommentRow(line)" v-if="shouldRenderCommentRow(line)"
:diff-file-hash="diffFile.fileHash" :diff-file-hash="diffFile.fileHash"
:line="line" :line="line"
:line-index="index" :line-index="index"
:key="index" :key="index"
:discussions="singleDiscussionByLineCode(line.lineCode)"
/> />
</template> </template>
</tbody> </tbody>
......
<script> <script>
import { mapState } from 'vuex'; import { mapState, mapGetters } from 'vuex';
import diffDiscussions from './diff_discussions.vue'; import diffDiscussions from './diff_discussions.vue';
import diffLineNoteForm from './diff_line_note_form.vue'; import diffLineNoteForm from './diff_line_note_form.vue';
...@@ -21,51 +21,48 @@ export default { ...@@ -21,51 +21,48 @@ export default {
type: Number, type: Number,
required: true, required: true,
}, },
leftDiscussions: {
type: Array,
required: false,
default: () => [],
},
rightDiscussions: {
type: Array,
required: false,
default: () => [],
},
}, },
computed: { computed: {
...mapState({ ...mapState({
diffLineCommentForms: state => state.diffs.diffLineCommentForms, diffLineCommentForms: state => state.diffs.diffLineCommentForms,
}), }),
...mapGetters(['discussionsByLineCode']),
leftLineCode() { leftLineCode() {
return this.line.left.lineCode; return this.line.left.lineCode;
}, },
rightLineCode() { rightLineCode() {
return this.line.right.lineCode; return this.line.right.lineCode;
}, },
hasDiscussion() {
const discussions = this.discussionsByLineCode;
return discussions[this.leftLineCode] || discussions[this.rightLineCode];
},
hasExpandedDiscussionOnLeft() { hasExpandedDiscussionOnLeft() {
const discussions = this.leftDiscussions; const discussions = this.discussionsByLineCode[this.leftLineCode];
return discussions ? discussions.every(discussion => discussion.expanded) : false; return discussions ? discussions.every(discussion => discussion.expanded) : false;
}, },
hasExpandedDiscussionOnRight() { hasExpandedDiscussionOnRight() {
const discussions = this.rightDiscussions; const discussions = this.discussionsByLineCode[this.rightLineCode];
return discussions ? discussions.every(discussion => discussion.expanded) : false; return discussions ? discussions.every(discussion => discussion.expanded) : false;
}, },
hasAnyExpandedDiscussion() { hasAnyExpandedDiscussion() {
return this.hasExpandedDiscussionOnLeft || this.hasExpandedDiscussionOnRight; return this.hasExpandedDiscussionOnLeft || this.hasExpandedDiscussionOnRight;
}, },
shouldRenderDiscussionsOnLeft() { shouldRenderDiscussionsOnLeft() {
return this.leftDiscussions && this.hasExpandedDiscussionOnLeft; return this.discussionsByLineCode[this.leftLineCode] && this.hasExpandedDiscussionOnLeft;
}, },
shouldRenderDiscussionsOnRight() { shouldRenderDiscussionsOnRight() {
return this.rightDiscussions && this.hasExpandedDiscussionOnRight && this.line.right.type; return (
}, this.discussionsByLineCode[this.rightLineCode] &&
showRightSideCommentForm() { this.hasExpandedDiscussionOnRight &&
return this.line.right.type && this.diffLineCommentForms[this.rightLineCode]; this.line.right.type
);
}, },
className() { className() {
return this.leftDiscussions.length > 0 || this.rightDiscussions.length > 0 return this.hasDiscussion ? '' : 'js-temp-notes-holder';
? ''
: 'js-temp-notes-holder';
}, },
}, },
}; };
...@@ -83,12 +80,13 @@ export default { ...@@ -83,12 +80,13 @@ export default {
class="content" class="content"
> >
<diff-discussions <diff-discussions
v-if="leftDiscussions.length" v-if="discussionsByLineCode[leftLineCode].length"
:discussions="leftDiscussions" :discussions="discussionsByLineCode[leftLineCode]"
/> />
</div> </div>
<diff-line-note-form <diff-line-note-form
v-if="diffLineCommentForms[leftLineCode]" v-if="diffLineCommentForms[leftLineCode] &&
diffLineCommentForms[leftLineCode]"
:diff-file-hash="diffFileHash" :diff-file-hash="diffFileHash"
:line="line.left" :line="line.left"
:note-target-line="line.left" :note-target-line="line.left"
...@@ -102,12 +100,13 @@ export default { ...@@ -102,12 +100,13 @@ export default {
class="content" class="content"
> >
<diff-discussions <diff-discussions
v-if="rightDiscussions.length" v-if="discussionsByLineCode[rightLineCode].length"
:discussions="rightDiscussions" :discussions="discussionsByLineCode[rightLineCode]"
/> />
</div> </div>
<diff-line-note-form <diff-line-note-form
v-if="showRightSideCommentForm" v-if="diffLineCommentForms[rightLineCode] &&
diffLineCommentForms[rightLineCode] && line.right.type"
:diff-file-hash="diffFileHash" :diff-file-hash="diffFileHash"
:line="line.right" :line="line.right"
:note-target-line="line.right" :note-target-line="line.right"
......
...@@ -36,16 +36,6 @@ export default { ...@@ -36,16 +36,6 @@ export default {
required: false, required: false,
default: false, default: false,
}, },
leftDiscussions: {
type: Array,
required: false,
default: () => [],
},
rightDiscussions: {
type: Array,
required: false,
default: () => [],
},
}, },
data() { data() {
return { return {
...@@ -126,10 +116,10 @@ export default { ...@@ -126,10 +116,10 @@ export default {
:is-hover="isLeftHover" :is-hover="isLeftHover"
:show-comment-button="true" :show-comment-button="true"
:diff-view-type="parallelDiffViewType" :diff-view-type="parallelDiffViewType"
:discussions="leftDiscussions"
class="diff-line-num old_line" class="diff-line-num old_line"
/> />
<td <td
v-once
:id="line.left.lineCode" :id="line.left.lineCode"
:class="parallelViewLeftLineType" :class="parallelViewLeftLineType"
class="line_content parallel left-side" class="line_content parallel left-side"
...@@ -147,10 +137,10 @@ export default { ...@@ -147,10 +137,10 @@ export default {
:is-hover="isRightHover" :is-hover="isRightHover"
:show-comment-button="true" :show-comment-button="true"
:diff-view-type="parallelDiffViewType" :diff-view-type="parallelDiffViewType"
:discussions="rightDiscussions"
class="diff-line-num new_line" class="diff-line-num new_line"
/> />
<td <td
v-once
:id="line.right.lineCode" :id="line.right.lineCode"
:class="line.right.type" :class="line.right.type"
class="line_content parallel right-side" class="line_content parallel right-side"
......
...@@ -21,11 +21,8 @@ export default { ...@@ -21,11 +21,8 @@ export default {
}, },
}, },
computed: { computed: {
...mapGetters('diffs', [ ...mapGetters('diffs', ['commitId']),
'commitId', ...mapGetters(['discussionsByLineCode']),
'singleDiscussionByLineCode',
'shouldRenderParallelCommentRow',
]),
...mapState({ ...mapState({
diffLineCommentForms: state => state.diffs.diffLineCommentForms, diffLineCommentForms: state => state.diffs.diffLineCommentForms,
}), }),
...@@ -55,6 +52,32 @@ export default { ...@@ -55,6 +52,32 @@ export default {
return window.gon.user_color_scheme; 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> </script>
...@@ -75,17 +98,13 @@ export default { ...@@ -75,17 +98,13 @@ export default {
:line="line" :line="line"
:is-bottom="index + 1 === diffLinesLength" :is-bottom="index + 1 === diffLinesLength"
:key="index" :key="index"
:left-discussions="singleDiscussionByLineCode(line.left.lineCode)"
:right-discussions="singleDiscussionByLineCode(line.right.lineCode)"
/> />
<parallel-diff-comment-row <parallel-diff-comment-row
v-if="shouldRenderParallelCommentRow(line)" v-if="shouldRenderCommentRow(line)"
:key="`dcr-${index}`" :key="`dcr-${index}`"
:line="line" :line="line"
:diff-file-hash="diffFile.fileHash" :diff-file-hash="diffFile.fileHash"
:line-index="index" :line-index="index"
:left-discussions="singleDiscussionByLineCode(line.left.lineCode)"
:right-discussions="singleDiscussionByLineCode(line.right.lineCode)"
/> />
</template> </template>
</tbody> </tbody>
......
import _ from 'underscore'; import _ from 'underscore';
import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
import { PARALLEL_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE } from '../constants'; 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; export const isParallelView = state => state.diffViewType === PARALLEL_DIFF_VIEW_TYPE;
...@@ -66,87 +64,6 @@ export const getDiffFileDiscussions = (state, getters, rootState, rootGetters) = ...@@ -66,87 +64,6 @@ export const getDiffFileDiscussions = (state, getters, rootState, rootGetters) =
discussion.diff_discussion && _.isEqual(discussion.diff_file.file_hash, diff.fileHash), 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 // prevent babel-plugin-rewire from generating an invalid default during karma∂ tests
export const getDiffFileByHash = state => fileHash => export const getDiffFileByHash = state => fileHash =>
state.diffFiles.find(file => file.fileHash === fileHash); state.diffFiles.find(file => file.fileHash === fileHash);
......
...@@ -173,24 +173,3 @@ export function trimFirstCharOfLineContent(line = {}) { ...@@ -173,24 +173,3 @@ export function trimFirstCharOfLineContent(line = {}) {
return parsedLine; 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;
}, {});
}
...@@ -63,7 +63,6 @@ export default class Poll { ...@@ -63,7 +63,6 @@ export default class Poll {
const headers = normalizeHeaders(response.headers); const headers = normalizeHeaders(response.headers);
const pollInterval = parseInt(headers[this.intervalHeader], 10); const pollInterval = parseInt(headers[this.intervalHeader], 10);
if (pollInterval > 0 && successCodes.indexOf(response.status) !== -1 && this.canPoll) { if (pollInterval > 0 && successCodes.indexOf(response.status) !== -1 && this.canPoll) {
clearTimeout(this.timeoutID);
this.timeoutID = setTimeout(() => { this.timeoutID = setTimeout(() => {
this.makeRequest(); this.makeRequest();
}, pollInterval); }, pollInterval);
......
...@@ -5,20 +5,19 @@ import resolvedSvg from 'icons/_icon_status_success_solid.svg'; ...@@ -5,20 +5,19 @@ import resolvedSvg from 'icons/_icon_status_success_solid.svg';
import mrIssueSvg from 'icons/_icon_mr_issue.svg'; import mrIssueSvg from 'icons/_icon_mr_issue.svg';
import nextDiscussionSvg from 'icons/_next_discussion.svg'; import nextDiscussionSvg from 'icons/_next_discussion.svg';
import { pluralize } from '../../lib/utils/text_utility'; import { pluralize } from '../../lib/utils/text_utility';
import discussionNavigation from '../mixins/discussion_navigation'; import { scrollToElement } from '../../lib/utils/common_utils';
import tooltip from '../../vue_shared/directives/tooltip'; import tooltip from '../../vue_shared/directives/tooltip';
export default { export default {
directives: { directives: {
tooltip, tooltip,
}, },
mixins: [discussionNavigation],
computed: { computed: {
...mapGetters([ ...mapGetters([
'getUserData', 'getUserData',
'getNoteableData', 'getNoteableData',
'discussionCount', 'discussionCount',
'firstUnresolvedDiscussionId', 'unresolvedDiscussions',
'resolvedDiscussionCount', 'resolvedDiscussionCount',
]), ]),
isLoggedIn() { isLoggedIn() {
...@@ -36,6 +35,11 @@ export default { ...@@ -36,6 +35,11 @@ export default {
resolveAllDiscussionsIssuePath() { resolveAllDiscussionsIssuePath() {
return this.getNoteableData.create_issue_to_resolve_discussions_path; return this.getNoteableData.create_issue_to_resolve_discussions_path;
}, },
firstUnresolvedDiscussionId() {
const item = this.unresolvedDiscussions[0] || {};
return item.id;
},
}, },
created() { created() {
this.resolveSvg = resolveSvg; this.resolveSvg = resolveSvg;
...@@ -46,10 +50,22 @@ export default { ...@@ -46,10 +50,22 @@ export default {
methods: { methods: {
...mapActions(['expandDiscussion']), ...mapActions(['expandDiscussion']),
jumpToFirstUnresolvedDiscussion() { jumpToFirstUnresolvedDiscussion() {
const diffTab = window.mrTabs.currentAction === 'diffs'; const discussionId = this.firstUnresolvedDiscussionId;
const discussionId = this.firstUnresolvedDiscussionId(diffTab); 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');
}
this.jumpToDiscussion(discussionId); if (el) {
this.expandDiscussion({ discussionId });
scrollToElement(el);
}
}, },
}, },
}; };
......
...@@ -7,7 +7,7 @@ import issuableStateMixin from '../mixins/issuable_state'; ...@@ -7,7 +7,7 @@ import issuableStateMixin from '../mixins/issuable_state';
import resolvable from '../mixins/resolvable'; import resolvable from '../mixins/resolvable';
export default { export default {
name: 'NoteForm', name: 'IssueNoteForm',
components: { components: {
issueWarning, issueWarning,
markdownField, markdownField,
......
<script> <script>
import _ from 'underscore';
import { mapActions, mapGetters } from 'vuex'; import { mapActions, mapGetters } from 'vuex';
import resolveDiscussionsSvg from 'icons/_icon_mr_issue.svg'; import resolveDiscussionsSvg from 'icons/_icon_mr_issue.svg';
import nextDiscussionsSvg from 'icons/_next_discussion.svg'; import nextDiscussionsSvg from 'icons/_next_discussion.svg';
import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; import { convertObjectPropsToCamelCase, scrollToElement } from '~/lib/utils/common_utils';
import { truncateSha } from '~/lib/utils/text_utility'; import { truncateSha } from '~/lib/utils/text_utility';
import systemNote from '~/vue_shared/components/notes/system_note.vue'; import systemNote from '~/vue_shared/components/notes/system_note.vue';
import { s__ } from '~/locale';
import Flash from '../../flash'; import Flash from '../../flash';
import { SYSTEM_NOTE } from '../constants'; import { SYSTEM_NOTE } from '../constants';
import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue'; import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue';
...@@ -20,7 +20,6 @@ import placeholderSystemNote from '../../vue_shared/components/notes/placeholder ...@@ -20,7 +20,6 @@ import placeholderSystemNote from '../../vue_shared/components/notes/placeholder
import autosave from '../mixins/autosave'; import autosave from '../mixins/autosave';
import noteable from '../mixins/noteable'; import noteable from '../mixins/noteable';
import resolvable from '../mixins/resolvable'; import resolvable from '../mixins/resolvable';
import discussionNavigation from '../mixins/discussion_navigation';
import tooltip from '../../vue_shared/directives/tooltip'; import tooltip from '../../vue_shared/directives/tooltip';
export default { export default {
...@@ -40,7 +39,7 @@ export default { ...@@ -40,7 +39,7 @@ export default {
directives: { directives: {
tooltip, tooltip,
}, },
mixins: [autosave, noteable, resolvable, discussionNavigation], mixins: [autosave, noteable, resolvable],
props: { props: {
discussion: { discussion: {
type: Object, type: Object,
...@@ -61,11 +60,6 @@ export default { ...@@ -61,11 +60,6 @@ export default {
required: false, required: false,
default: false, default: false,
}, },
discussionsByDiffOrder: {
type: Boolean,
required: false,
default: false,
},
}, },
data() { data() {
return { return {
...@@ -80,12 +74,7 @@ export default { ...@@ -80,12 +74,7 @@ export default {
'discussionCount', 'discussionCount',
'resolvedDiscussionCount', 'resolvedDiscussionCount',
'allDiscussions', 'allDiscussions',
'unresolvedDiscussionsIdsByDiff',
'unresolvedDiscussionsIdsByDate',
'unresolvedDiscussions', 'unresolvedDiscussions',
'unresolvedDiscussionsIdsOrdered',
'nextUnresolvedDiscussionId',
'isLastUnresolvedDiscussion',
]), ]),
transformedDiscussion() { transformedDiscussion() {
return { return {
...@@ -136,10 +125,6 @@ export default { ...@@ -136,10 +125,6 @@ export default {
hasMultipleUnresolvedDiscussions() { hasMultipleUnresolvedDiscussions() {
return this.unresolvedDiscussions.length > 1; return this.unresolvedDiscussions.length > 1;
}, },
showJumpToNextDiscussion() {
return this.hasMultipleUnresolvedDiscussions &&
!this.isLastUnresolvedDiscussion(this.discussion.id, this.discussionsByDiffOrder);
},
shouldRenderDiffs() { shouldRenderDiffs() {
const { diffDiscussion, diffFile } = this.transformedDiscussion; const { diffDiscussion, diffFile } = this.transformedDiscussion;
...@@ -159,17 +144,19 @@ export default { ...@@ -159,17 +144,19 @@ export default {
return this.isDiffDiscussion ? '' : 'card discussion-wrapper'; return this.isDiffDiscussion ? '' : 'card discussion-wrapper';
}, },
}, },
watch: { mounted() {
isReplying() {
if (this.isReplying) { if (this.isReplying) {
this.$nextTick(() => { this.initAutoSave(this.transformedDiscussion);
// Pass an extra key to separate reply and note edit forms
this.initAutoSave(this.transformedDiscussion, ['Reply']);
});
} else {
this.disposeAutoSave();
} }
}, },
updated() {
if (this.isReplying) {
if (!this.autosave) {
this.initAutoSave(this.transformedDiscussion);
} else {
this.setAutoSave();
}
}
}, },
created() { created() {
this.resolveDiscussionsSvg = resolveDiscussionsSvg; this.resolveDiscussionsSvg = resolveDiscussionsSvg;
...@@ -207,18 +194,16 @@ export default { ...@@ -207,18 +194,16 @@ export default {
showReplyForm() { showReplyForm() {
this.isReplying = true; this.isReplying = true;
}, },
cancelReplyForm(shouldConfirm, isDirty) { cancelReplyForm(shouldConfirm) {
if (shouldConfirm && isDirty) { if (shouldConfirm && this.$refs.noteForm.isDirty) {
const msg = s__('Notes|Are you sure you want to cancel creating this comment?');
// eslint-disable-next-line no-alert // eslint-disable-next-line no-alert
if (!window.confirm(msg)) { if (!window.confirm('Are you sure you want to cancel creating this comment?')) {
return; return;
} }
} }
this.isReplying = false;
this.resetAutoSave(); this.resetAutoSave();
this.isReplying = false;
}, },
saveReply(noteText, form, callback) { saveReply(noteText, form, callback) {
const postData = { const postData = {
...@@ -256,10 +241,21 @@ Please check your network connection and try again.`; ...@@ -256,10 +241,21 @@ Please check your network connection and try again.`;
}); });
}, },
jumpToNextDiscussion() { jumpToNextDiscussion() {
const nextId = const discussionIds = this.allDiscussions.map(d => d.id);
this.nextUnresolvedDiscussionId(this.discussion.id, this.discussionsByDiffOrder); 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);
this.jumpToDiscussion(nextId); if (nextIndex > -1) {
const nextId = remainingAfterCurrent[nextIndex];
const el = document.querySelector(`[data-discussion-id="${nextId}"]`);
if (el) {
this.expandDiscussion({ discussionId: nextId });
scrollToElement(el);
}
}
}, },
}, },
}; };
...@@ -401,7 +397,7 @@ Please check your network connection and try again.`; ...@@ -401,7 +397,7 @@ Please check your network connection and try again.`;
</a> </a>
</div> </div>
<div <div
v-if="showJumpToNextDiscussion" v-if="hasMultipleUnresolvedDiscussions"
class="btn-group" class="btn-group"
role="group"> role="group">
<button <button
...@@ -424,8 +420,7 @@ Please check your network connection and try again.`; ...@@ -424,8 +420,7 @@ Please check your network connection and try again.`;
:is-editing="false" :is-editing="false"
save-button-title="Comment" save-button-title="Comment"
@handleFormUpdate="saveReply" @handleFormUpdate="saveReply"
@cancelForm="cancelReplyForm" @cancelForm="cancelReplyForm" />
/>
<note-signed-out-widget v-if="!canReply" /> <note-signed-out-widget v-if="!canReply" />
</div> </div>
</div> </div>
......
...@@ -4,18 +4,12 @@ import { capitalizeFirstCharacter } from '../../lib/utils/text_utility'; ...@@ -4,18 +4,12 @@ import { capitalizeFirstCharacter } from '../../lib/utils/text_utility';
export default { export default {
methods: { methods: {
initAutoSave(noteable, extraKeys = []) { initAutoSave(noteable) {
let keys = [ this.autosave = new Autosave($(this.$refs.noteForm.$refs.textarea), [
'Note', 'Note',
capitalizeFirstCharacter(noteable.noteable_type || noteable.noteableType), capitalizeFirstCharacter(noteable.noteable_type),
noteable.id, noteable.id,
]; ]);
if (extraKeys) {
keys = keys.concat(extraKeys);
}
this.autosave = new Autosave($(this.$refs.noteForm.$refs.textarea), keys);
}, },
resetAutoSave() { resetAutoSave() {
this.autosave.reset(); this.autosave.reset();
...@@ -23,8 +17,5 @@ export default { ...@@ -23,8 +17,5 @@ export default {
setAutoSave() { setAutoSave() {
this.autosave.save(); this.autosave.save();
}, },
disposeAutoSave() {
this.autosave.dispose();
},
}, },
}; };
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;
},
},
};
...@@ -28,6 +28,18 @@ export const notesById = state => ...@@ -28,6 +28,18 @@ export const notesById = state =>
return acc; 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 => { export const noteableType = state => {
const { ISSUE_NOTEABLE_TYPE, MERGE_REQUEST_NOTEABLE_TYPE, EPIC_NOTEABLE_TYPE } = constants; const { ISSUE_NOTEABLE_TYPE, MERGE_REQUEST_NOTEABLE_TYPE, EPIC_NOTEABLE_TYPE } = constants;
...@@ -70,9 +82,6 @@ export const allDiscussions = (state, getters) => { ...@@ -70,9 +82,6 @@ export const allDiscussions = (state, getters) => {
return Object.values(resolved).concat(unresolved); return Object.values(resolved).concat(unresolved);
}; };
export const allResolvableDiscussions = (state, getters) =>
getters.allDiscussions.filter(d => !d.individual_note && d.resolvable);
export const resolvedDiscussionsById = state => { export const resolvedDiscussionsById = state => {
const map = {}; const map = {};
...@@ -89,51 +98,6 @@ export const resolvedDiscussionsById = state => { ...@@ -89,51 +98,6 @@ export const resolvedDiscussionsById = state => {
return map; 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) => { export const resolvedDiscussionCount = (state, getters) => {
const resolvedMap = getters.resolvedDiscussionsById; const resolvedMap = getters.resolvedDiscussionsById;
...@@ -150,42 +114,5 @@ export const discussionTabCounter = state => { ...@@ -150,42 +114,5 @@ export const discussionTabCounter = state => {
return all.length; 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 // prevent babel-plugin-rewire from generating an invalid default during karma tests
export default () => {}; export default () => {};
...@@ -174,19 +174,27 @@ export default { ...@@ -174,19 +174,27 @@ export default {
[types.UPDATE_NOTE](state, note) { [types.UPDATE_NOTE](state, note) {
const noteObj = utils.findNoteObjectById(state.discussions, note.discussion_id); const noteObj = utils.findNoteObjectById(state.discussions, note.discussion_id);
if (noteObj.individual_note) { if (noteObj.individual_note) {
noteObj.notes.splice(0, 1, note); noteObj.notes.splice(0, 1, note);
} else { } else {
const comment = utils.findNoteObjectById(noteObj.notes, note.id); const comment = utils.findNoteObjectById(noteObj.notes, note.id);
Object.assign(comment, note); noteObj.notes.splice(noteObj.notes.indexOf(comment), 1, note);
} }
}, },
[types.UPDATE_DISCUSSION](state, noteData) { [types.UPDATE_DISCUSSION](state, noteData) {
const note = noteData; const note = noteData;
const selectedDiscussion = state.discussions.find(n => n.id === note.id); let index = 0;
state.discussions.forEach((n, i) => {
if (n.id === note.id) {
index = i;
}
});
note.expanded = true; // override expand flag to prevent collapse note.expanded = true; // override expand flag to prevent collapse
Object.assign(selectedDiscussion, note); state.discussions.splice(index, 1, note);
}, },
[types.CLOSE_ISSUE](state) { [types.CLOSE_ISSUE](state) {
...@@ -207,9 +215,12 @@ export default { ...@@ -207,9 +215,12 @@ export default {
[types.SET_DISCUSSION_DIFF_LINES](state, { discussionId, diffLines }) { [types.SET_DISCUSSION_DIFF_LINES](state, { discussionId, diffLines }) {
const discussion = utils.findNoteObjectById(state.discussions, discussionId); const discussion = utils.findNoteObjectById(state.discussions, discussionId);
const index = state.discussions.indexOf(discussion);
Object.assign(discussion, { const discussionWithDiffLines = Object.assign({}, discussion, {
truncated_diff_lines: diffLines, truncated_diff_lines: diffLines,
}); });
state.discussions.splice(index, 1, discussionWithDiffLines);
}, },
}; };
...@@ -2,11 +2,13 @@ import AjaxCache from '~/lib/utils/ajax_cache'; ...@@ -2,11 +2,13 @@ import AjaxCache from '~/lib/utils/ajax_cache';
const REGEX_QUICK_ACTIONS = /^\/\w+.*$/gm; const REGEX_QUICK_ACTIONS = /^\/\w+.*$/gm;
export const findNoteObjectById = (notes, id) => notes.find(n => n.id === id); export const findNoteObjectById = (notes, id) =>
notes.filter(n => n.id === id)[0];
export const getQuickActionText = note => { export const getQuickActionText = note => {
let text = 'Applying command'; 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 executedCommands = quickActions.filter(command => {
const commandRegex = new RegExp(`/${command.name}`); const commandRegex = new RegExp(`/${command.name}`);
...@@ -27,4 +29,5 @@ export const getQuickActionText = note => { ...@@ -27,4 +29,5 @@ export const getQuickActionText = note => {
export const hasQuickActions = note => REGEX_QUICK_ACTIONS.test(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();
...@@ -6,7 +6,6 @@ class DiscussionEntity < Grape::Entity ...@@ -6,7 +6,6 @@ class DiscussionEntity < Grape::Entity
expose :id, :reply_id expose :id, :reply_id
expose :position, if: -> (d, _) { d.diff_discussion? && !d.legacy_diff_discussion? } 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 :line_code, if: -> (d, _) { d.diff_discussion? }
expose :expanded?, as: :expanded expose :expanded?, as: :expanded
expose :active?, as: :active, if: -> (d, _) { d.diff_discussion? } expose :active?, as: :active, if: -> (d, _) { d.diff_discussion? }
......
...@@ -3622,9 +3622,6 @@ msgstr "" ...@@ -3622,9 +3622,6 @@ 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." 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 "" msgstr ""
msgid "Notes|Are you sure you want to cancel creating this comment?"
msgstr ""
msgid "Notification events" msgid "Notification events"
msgstr "" msgstr ""
......
...@@ -342,9 +342,8 @@ describe 'Merge request > User resolves diff notes and discussions', :js do ...@@ -342,9 +342,8 @@ describe 'Merge request > User resolves diff notes and discussions', :js do
end end
end end
it 'shows jump to next discussion button, apart from the last one' do it 'shows jump to next discussion button' do
expect(page).to have_selector('.discussion-reply-holder', count: 2) expect(page.all('.discussion-reply-holder', count: 2)).to all(have_selector('.discussion-next-btn'))
expect(page).to have_selector('.discussion-reply-holder .discussion-next-btn', count: 1)
end end
it 'displays next discussion even if hidden' do it 'displays next discussion even if hidden' do
......
...@@ -59,10 +59,12 @@ describe('Autosave', () => { ...@@ -59,10 +59,12 @@ describe('Autosave', () => {
Autosave.prototype.restore.call(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', () => { autosave.field.get(0).addEventListener('change', () => {
done(); done();
}); });
...@@ -79,7 +81,9 @@ describe('Autosave', () => { ...@@ -79,7 +81,9 @@ describe('Autosave', () => {
it('does not trigger event', () => { it('does not trigger event', () => {
spyOn(field, 'trigger').and.callThrough(); spyOn(field, 'trigger').and.callThrough();
expect(field.trigger).not.toHaveBeenCalled(); expect(
field.trigger,
).not.toHaveBeenCalled();
}); });
}); });
}); });
......
...@@ -18,12 +18,10 @@ describe('DiffLineGutterContent', () => { ...@@ -18,12 +18,10 @@ describe('DiffLineGutterContent', () => {
}; };
const setDiscussions = component => { const setDiscussions = component => {
component.$store.dispatch('setInitialNotes', getDiscussionsMockData()); component.$store.dispatch('setInitialNotes', getDiscussionsMockData());
component.$store.commit('diffs/SET_DIFF_DATA', { diffFiles: [getDiffFileMock()] });
}; };
const resetDiscussions = component => { const resetDiscussions = component => {
component.$store.dispatch('setInitialNotes', []); component.$store.dispatch('setInitialNotes', []);
component.$store.commit('diffs/SET_DIFF_DATA', {});
}; };
describe('computed', () => { describe('computed', () => {
...@@ -50,11 +48,7 @@ describe('DiffLineGutterContent', () => { ...@@ -50,11 +48,7 @@ describe('DiffLineGutterContent', () => {
it('should return discussions for the given lineCode', () => { it('should return discussions for the given lineCode', () => {
const { lineCode } = getDiffFileMock().highlightedDiffLines[1]; const { lineCode } = getDiffFileMock().highlightedDiffLines[1];
const component = createComponent({ const component = createComponent({ lineCode, showCommentButton: true });
lineCode,
showCommentButton: true,
discussions: getDiscussionsMockData(),
});
setDiscussions(component); setDiscussions(component);
......
...@@ -3,7 +3,6 @@ import DiffLineNoteForm from '~/diffs/components/diff_line_note_form.vue'; ...@@ -3,7 +3,6 @@ import DiffLineNoteForm from '~/diffs/components/diff_line_note_form.vue';
import store from '~/mr_notes/stores'; import store from '~/mr_notes/stores';
import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
import diffFileMockData from '../mock_data/diff_file'; import diffFileMockData from '../mock_data/diff_file';
import { noteableDataMock } from '../../notes/mock_data';
describe('DiffLineNoteForm', () => { describe('DiffLineNoteForm', () => {
let component; let component;
...@@ -22,9 +21,10 @@ describe('DiffLineNoteForm', () => { ...@@ -22,9 +21,10 @@ describe('DiffLineNoteForm', () => {
noteTargetLine: diffLines[0], noteTargetLine: diffLines[0],
}); });
Object.defineProperties(component, { Object.defineProperty(component, 'isLoggedIn', {
noteableData: { value: noteableDataMock }, get() {
isLoggedIn: { value: true }, return true;
},
}); });
component.$mount(); component.$mount();
...@@ -32,38 +32,13 @@ describe('DiffLineNoteForm', () => { ...@@ -32,38 +32,13 @@ describe('DiffLineNoteForm', () => {
describe('methods', () => { describe('methods', () => {
describe('handleCancelCommentForm', () => { describe('handleCancelCommentForm', () => {
it('should ask for confirmation when shouldConfirm and isDirty passed as truthy', () => { it('should call cancelCommentForm with lineCode', () => {
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, 'cancelCommentForm');
spyOn(component, 'resetAutoSave');
component.handleCancelCommentForm(); component.handleCancelCommentForm();
expect(window.confirm).not.toHaveBeenCalled();
component.$nextTick(() => {
expect(component.cancelCommentForm).toHaveBeenCalledWith({ expect(component.cancelCommentForm).toHaveBeenCalledWith({
lineCode: diffLines[0].lineCode, lineCode: diffLines[0].lineCode,
}); });
expect(component.resetAutoSave).toHaveBeenCalled();
done();
});
}); });
}); });
...@@ -91,7 +66,7 @@ describe('DiffLineNoteForm', () => { ...@@ -91,7 +66,7 @@ describe('DiffLineNoteForm', () => {
describe('mounted', () => { describe('mounted', () => {
it('should init autosave', () => { it('should init autosave', () => {
const key = 'autosave/Note/Issue/98//DiffNote//1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_1'; const key = 'autosave/Note/issue///DiffNote//1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_1';
expect(component.autosave).toBeDefined(); expect(component.autosave).toBeDefined();
expect(component.autosave.key).toEqual(key); expect(component.autosave.key).toEqual(key);
......
...@@ -33,7 +33,6 @@ describe('InlineDiffView', () => { ...@@ -33,7 +33,6 @@ describe('InlineDiffView', () => {
it('should render discussions', done => { it('should render discussions', done => {
const el = component.$el; const el = component.$el;
component.$store.dispatch('setInitialNotes', getDiscussionsMockData()); component.$store.dispatch('setInitialNotes', getDiscussionsMockData());
component.$store.commit('diffs/SET_DIFF_DATA', { diffFiles: [getDiffFileMock()] });
Vue.nextTick(() => { Vue.nextTick(() => {
expect(el.querySelectorAll('.notes_holder').length).toEqual(1); expect(el.querySelectorAll('.notes_holder').length).toEqual(1);
......
...@@ -12,17 +12,6 @@ export default { ...@@ -12,17 +12,6 @@ export default {
head_sha: 'c48ee0d1bf3b30453f5b32250ce03134beaa6d13', 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', line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_2',
expanded: true, expanded: true,
notes: [ notes: [
......
...@@ -2,7 +2,6 @@ import * as getters from '~/diffs/store/getters'; ...@@ -2,7 +2,6 @@ import * as getters from '~/diffs/store/getters';
import state from '~/diffs/store/modules/diff_state'; import state from '~/diffs/store/modules/diff_state';
import { PARALLEL_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE } from '~/diffs/constants'; import { PARALLEL_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE } from '~/diffs/constants';
import discussion from '../mock_data/diff_discussions'; import discussion from '../mock_data/diff_discussions';
import diffFile from '../mock_data/diff_file';
describe('Diffs Module Getters', () => { describe('Diffs Module Getters', () => {
let localState; let localState;
...@@ -222,38 +221,4 @@ describe('Diffs Module Getters', () => { ...@@ -222,38 +221,4 @@ describe('Diffs Module Getters', () => {
expect(getters.getDiffFileByHash(localState)('123')).toBeUndefined(); 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);
});
});
}); });
...@@ -207,24 +207,4 @@ describe('DiffsStoreUtils', () => { ...@@ -207,24 +207,4 @@ describe('DiffsStoreUtils', () => {
expect(utils.trimFirstCharOfLineContent()).toEqual({}); 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);
});
});
}); });
...@@ -46,7 +46,7 @@ describe('DiscussionCounter component', () => { ...@@ -46,7 +46,7 @@ describe('DiscussionCounter component', () => {
discussions, discussions,
}); });
setFixtures(` setFixtures(`
<div class="discussion" data-discussion-id="${firstDiscussionId}"></div> <div data-discussion-id="${firstDiscussionId}"></div>
`); `);
vm.jumpToFirstUnresolvedDiscussion(); vm.jumpToFirstUnresolvedDiscussion();
......
...@@ -14,7 +14,6 @@ describe('noteable_discussion component', () => { ...@@ -14,7 +14,6 @@ describe('noteable_discussion component', () => {
preloadFixtures(discussionWithTwoUnresolvedNotes); preloadFixtures(discussionWithTwoUnresolvedNotes);
beforeEach(() => { beforeEach(() => {
window.mrTabs = {};
store = createStore(); store = createStore();
store.dispatch('setNoteableData', noteableDataMock); store.dispatch('setNoteableData', noteableDataMock);
store.dispatch('setNotesData', notesDataMock); store.dispatch('setNotesData', notesDataMock);
...@@ -47,17 +46,12 @@ describe('noteable_discussion component', () => { ...@@ -47,17 +46,12 @@ describe('noteable_discussion component', () => {
it('should toggle reply form', done => { it('should toggle reply form', done => {
vm.$el.querySelector('.js-vue-discussion-reply').click(); vm.$el.querySelector('.js-vue-discussion-reply').click();
Vue.nextTick(() => {
expect(vm.isReplying).toEqual(true);
// There is a watcher for `isReplying` which will init autosave in the next tick
Vue.nextTick(() => { Vue.nextTick(() => {
expect(vm.$refs.noteForm).not.toBeNull(); expect(vm.$refs.noteForm).not.toBeNull();
expect(vm.isReplying).toEqual(true);
done(); done();
}); });
}); });
});
it('does not render jump to discussion button', () => { it('does not render jump to discussion button', () => {
expect( expect(
...@@ -107,29 +101,33 @@ describe('noteable_discussion component', () => { ...@@ -107,29 +101,33 @@ describe('noteable_discussion component', () => {
describe('methods', () => { describe('methods', () => {
describe('jumpToNextDiscussion', () => { describe('jumpToNextDiscussion', () => {
it('expands next unresolved discussion', done => { it('expands next unresolved discussion', () => {
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(); spyOn(vm, 'expandDiscussion').and.stub();
const discussions = [
const nextDiscussionId = discussion2.id; 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(` setFixtures(`
<div class="discussion" data-discussion-id="${nextDiscussionId}"></div> <div 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);
}); });
}); });
}); });
......
...@@ -1168,87 +1168,3 @@ export const collapsedSystemNotes = [ ...@@ -1168,87 +1168,3 @@ export const collapsedSystemNotes = [
diff_discussion: false, 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,
};
...@@ -5,11 +5,6 @@ import { ...@@ -5,11 +5,6 @@ import {
noteableDataMock, noteableDataMock,
individualNote, individualNote,
collapseNotesMock, collapseNotesMock,
discussion1,
discussion2,
discussion3,
resolvedDiscussion1,
unresolvableDiscussion,
} from '../mock_data'; } from '../mock_data';
const discussionWithTwoUnresolvedNotes = 'merge_requests/resolved_diff_discussion.json'; const discussionWithTwoUnresolvedNotes = 'merge_requests/resolved_diff_discussion.json';
...@@ -114,154 +109,4 @@ describe('Getters Notes Store', () => { ...@@ -114,154 +109,4 @@ describe('Getters Notes Store', () => {
expect(getters.isNotesFetched(state)).toBeFalsy(); 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();
});
});
}); });
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment