Commit c9bacfd6 authored by Tim Zallmann's avatar Tim Zallmann

Fixed Resolving, Loading more and Line Bugs

parent cee271ee
...@@ -25,44 +25,44 @@ const ResolveDiscussionBtn = Vue.extend({ ...@@ -25,44 +25,44 @@ const ResolveDiscussionBtn = Vue.extend({
}; };
}, },
computed: { computed: {
showButton: function () { showButton: function() {
if (this.discussion) { if (this.discussion) {
return this.discussion.isResolvable(); return this.discussion.isResolvable();
} else { } else {
return false; return false;
} }
}, },
isDiscussionResolved: function () { isDiscussionResolved: function() {
if (this.discussion) { if (this.discussion) {
return this.discussion.isResolved(); return this.discussion.isResolved();
} else { } else {
return false; return false;
} }
}, },
buttonText: function () { buttonText: function() {
if (this.isDiscussionResolved) { if (this.isDiscussionResolved) {
return "Unresolve discussion"; return 'Unresolve discussion';
} else { } else {
return "Resolve discussion"; return 'Resolve discussion';
} }
}, },
loading: function () { loading: function() {
if (this.discussion) { if (this.discussion) {
return this.discussion.loading; return this.discussion.loading;
} else { } else {
return false; return false;
} }
}
}, },
created: function () { },
created: function() {
CommentsStore.createDiscussion(this.discussionId, this.canResolve); CommentsStore.createDiscussion(this.discussionId, this.canResolve);
this.discussion = CommentsStore.state[this.discussionId]; this.discussion = CommentsStore.state[this.discussionId];
}, },
methods: { methods: {
resolve: function () { resolve: function() {
ResolveService.toggleResolveForDiscussion(this.mergeRequestId, this.discussionId); ResolveService.toggleResolveForDiscussion(this.mergeRequestId, this.discussionId);
} },
}, },
}); });
......
...@@ -8,9 +8,7 @@ window.gl = window.gl || {}; ...@@ -8,9 +8,7 @@ window.gl = window.gl || {};
class ResolveServiceClass { class ResolveServiceClass {
constructor(root) { constructor(root) {
this.noteResource = Vue.resource( this.noteResource = Vue.resource(`${root}/notes{/noteId}/resolve?html=true`);
`${root}/notes{/noteId}/resolve?html=true`,
);
this.discussionResource = Vue.resource( this.discussionResource = Vue.resource(
`${root}/merge_requests{/mergeRequestId}/discussions{/discussionId}/resolve?html=true`, `${root}/merge_requests{/mergeRequestId}/discussions{/discussionId}/resolve?html=true`,
); );
...@@ -51,10 +49,7 @@ class ResolveServiceClass { ...@@ -51,10 +49,7 @@ class ResolveServiceClass {
discussion.updateHeadline(data); discussion.updateHeadline(data);
}) })
.catch( .catch(
() => () => new Flash('An error occurred when trying to resolve a discussion. Please try again.'),
new Flash(
'An error occurred when trying to resolve a discussion. Please try again.',
),
); );
} }
......
...@@ -112,6 +112,7 @@ export default { ...@@ -112,6 +112,7 @@ export default {
}, },
created() { created() {
this.adjustView(); this.adjustView();
eventHub.$once('fetchedNotesData', this.setDiscussions);
}, },
methods: { methods: {
...mapActions('diffs', [ ...mapActions('diffs', [
...@@ -128,12 +129,7 @@ export default { ...@@ -128,12 +129,7 @@ export default {
() => { () => {
this.startRenderDiffsQueue() this.startRenderDiffsQueue()
.then(() => { .then(() => {
requestIdleCallback( this.setDiscussions();
() => {
this.assignDiscussionsToDiff(this.discussionsStructuredByLineCode);
},
{ timeout: 1000 },
);
}) })
.catch(() => { .catch(() => {
createFlash(__('Something went wrong on our end. Please try again!')); createFlash(__('Something went wrong on our end. Please try again!'));
...@@ -150,6 +146,16 @@ export default { ...@@ -150,6 +146,16 @@ export default {
eventHub.$emit('fetchNotesData'); eventHub.$emit('fetchNotesData');
} }
}, },
setDiscussions() {
if (this.isNotesFetched) {
requestIdleCallback(
() => {
this.assignDiscussionsToDiff(this.discussionsStructuredByLineCode);
},
{ timeout: 1000 },
);
}
},
adjustView() { adjustView() {
if (this.shouldShow && this.isParallelView) { if (this.shouldShow && this.isParallelView) {
window.mrTabs.expandViewContainer(); window.mrTabs.expandViewContainer();
......
<script> <script>
import { mapActions } from 'vuex'; import { mapActions, mapGetters } from 'vuex';
import _ from 'underscore'; import _ from 'underscore';
import { __, sprintf } from '~/locale'; import { __, sprintf } from '~/locale';
import createFlash from '~/flash'; import createFlash from '~/flash';
...@@ -30,6 +30,7 @@ export default { ...@@ -30,6 +30,7 @@ export default {
}; };
}, },
computed: { computed: {
...mapGetters(['isNotesFetched', 'discussionsStructuredByLineCode']),
isCollapsed() { isCollapsed() {
return this.file.collapsed || false; return this.file.collapsed || false;
}, },
...@@ -44,23 +45,22 @@ export default { ...@@ -44,23 +45,22 @@ export default {
); );
}, },
showExpandMessage() { showExpandMessage() {
return this.isCollapsed && !this.isLoadingCollapsedDiff && !this.file.tooLarge; return (
!this.isCollapsed &&
!this.file.highlightedDiffLines &&
!this.isLoadingCollapsedDiff &&
!this.file.tooLarge
);
}, },
showLoadingIcon() { showLoadingIcon() {
return this.isLoadingCollapsedDiff || (!this.file.renderIt && !this.isCollapsed); return this.isLoadingCollapsedDiff || (!this.file.renderIt && !this.isCollapsed);
}, },
}, },
methods: { methods: {
...mapActions('diffs', ['loadCollapsedDiff']), ...mapActions('diffs', ['loadCollapsedDiff', 'assignDiscussionsToDiff']),
handleToggle() { handleToggle() {
const { collapsed, highlightedDiffLines, parallelDiffLines } = this.file; const { highlightedDiffLines, parallelDiffLines } = this.file;
if (!highlightedDiffLines && parallelDiffLines !== undefined && !parallelDiffLines.length) {
if (
collapsed &&
!highlightedDiffLines &&
parallelDiffLines !== undefined &&
!parallelDiffLines.length
) {
this.handleLoadCollapsedDiff(); this.handleLoadCollapsedDiff();
} else { } else {
this.file.collapsed = !this.file.collapsed; this.file.collapsed = !this.file.collapsed;
...@@ -76,6 +76,15 @@ export default { ...@@ -76,6 +76,15 @@ export default {
this.file.collapsed = false; this.file.collapsed = false;
this.file.renderIt = true; this.file.renderIt = true;
}) })
.then(() => this.$nextTick())
.then(() => {
requestIdleCallback(
() => {
this.assignDiscussionsToDiff(this.discussionsStructuredByLineCode);
},
{ timeout: 1000 },
);
})
.catch(() => { .catch(() => {
this.isLoadingCollapsedDiff = false; this.isLoadingCollapsedDiff = false;
createFlash(__('Something went wrong on our end. Please try again!')); createFlash(__('Something went wrong on our end. Please try again!'));
...@@ -136,7 +145,7 @@ export default { ...@@ -136,7 +145,7 @@ export default {
:diff-file="file" :diff-file="file"
/> />
<loading-icon <loading-icon
v-else-if="showLoadingIcon" v-if="showLoadingIcon"
class="diff-content loading" class="diff-content loading"
/> />
<div <div
......
...@@ -73,7 +73,7 @@ export default { ...@@ -73,7 +73,7 @@ export default {
}), }),
...mapGetters(['isLoggedIn']), ...mapGetters(['isLoggedIn']),
lineHref() { lineHref() {
return this.line && this.line.code ? `#${this.line.code}` : '#'; return this.line && this.line.lineCode ? `#${this.line.lineCode}` : '#';
}, },
shouldShowCommentButton() { shouldShowCommentButton() {
return ( return (
...@@ -93,7 +93,6 @@ export default { ...@@ -93,7 +93,6 @@ export default {
if (this.line && !this.line.type && this.linePosition === LINE_POSITION_RIGHT) { if (this.line && !this.line.type && this.linePosition === LINE_POSITION_RIGHT) {
return false; return false;
} }
return this.showCommentButton && this.hasDiscussions; return this.showCommentButton && this.hasDiscussions;
}, },
}, },
......
...@@ -29,7 +29,7 @@ export const fetchDiffFiles = ({ state, commit }) => { ...@@ -29,7 +29,7 @@ export const fetchDiffFiles = ({ state, commit }) => {
.then(handleLocationHash); .then(handleLocationHash);
}; };
export const assignDiscussionsToDiff = ({ state }, allLineDiscussions) => { export const assignDiscussionsToDiff = ({ state, commit }, allLineDiscussions) => {
Object.values(allLineDiscussions).forEach(discussions => { Object.values(allLineDiscussions).forEach(discussions => {
if (discussions.length > 0) { if (discussions.length > 0) {
const { fileHash } = discussions[0]; const { fileHash } = discussions[0];
...@@ -40,12 +40,11 @@ export const assignDiscussionsToDiff = ({ state }, allLineDiscussions) => { ...@@ -40,12 +40,11 @@ export const assignDiscussionsToDiff = ({ state }, allLineDiscussions) => {
(line.left && line.left.lineCode === discussions[0].line_code) || (line.left && line.left.lineCode === discussions[0].line_code) ||
(line.right && line.right.lineCode === discussions[0].line_code), (line.right && line.right.lineCode === discussions[0].line_code),
); );
if (targetLine) { if (targetLine) {
if (targetLine.left && targetLine.left.lineCode === discussions[0].line_code) { if (targetLine.left && targetLine.left.lineCode === discussions[0].line_code) {
Object.assign(targetLine.left, { discussions }); commit(types.SET_LINE_DISCUSSIONS, { line: targetLine.left, discussions });
} else { } else {
Object.assign(targetLine.right, { discussions }); commit(types.SET_LINE_DISCUSSIONS, { line: targetLine.right, discussions });
} }
} }
...@@ -55,7 +54,7 @@ export const assignDiscussionsToDiff = ({ state }, allLineDiscussions) => { ...@@ -55,7 +54,7 @@ export const assignDiscussionsToDiff = ({ state }, allLineDiscussions) => {
); );
if (targetInlineLine) { if (targetInlineLine) {
Object.assign(targetInlineLine, { discussions }); commit(types.SET_LINE_DISCUSSIONS, { line: targetInlineLine, discussions });
} }
} }
} }
...@@ -63,7 +62,7 @@ export const assignDiscussionsToDiff = ({ state }, allLineDiscussions) => { ...@@ -63,7 +62,7 @@ export const assignDiscussionsToDiff = ({ state }, allLineDiscussions) => {
}); });
}; };
export const removeDiscussionsFromDiff = ({ state }, removeDiscussion) => { export const removeDiscussionsFromDiff = ({ state, commit }, removeDiscussion) => {
const { fileHash } = removeDiscussion; const { fileHash } = removeDiscussion;
const selectedFile = state.diffFiles.find(file => file.fileHash === fileHash); const selectedFile = state.diffFiles.find(file => file.fileHash === fileHash);
if (selectedFile) { if (selectedFile) {
...@@ -74,7 +73,13 @@ export const removeDiscussionsFromDiff = ({ state }, removeDiscussion) => { ...@@ -74,7 +73,13 @@ export const removeDiscussionsFromDiff = ({ state }, removeDiscussion) => {
); );
if (targetLine) { if (targetLine) {
Object.assign(targetLine.right, { discussions: [] }); if (targetLine) {
if (targetLine.left && targetLine.left.lineCode === removeDiscussion.line_code) {
commit(types.REMOVE_LINE_DISCUSSIONS, targetLine.left);
} else {
commit(types.REMOVE_LINE_DISCUSSIONS, targetLine.right);
}
}
} }
const targetInlineLine = selectedFile.highlightedDiffLines.find( const targetInlineLine = selectedFile.highlightedDiffLines.find(
...@@ -82,7 +87,7 @@ export const removeDiscussionsFromDiff = ({ state }, removeDiscussion) => { ...@@ -82,7 +87,7 @@ export const removeDiscussionsFromDiff = ({ state }, removeDiscussion) => {
); );
if (targetInlineLine) { if (targetInlineLine) {
Object.assign(targetInlineLine, { discussions: [] }); commit(types.REMOVE_LINE_DISCUSSIONS, targetInlineLine);
} }
} }
}; };
......
...@@ -9,3 +9,5 @@ export const ADD_CONTEXT_LINES = 'ADD_CONTEXT_LINES'; ...@@ -9,3 +9,5 @@ export const ADD_CONTEXT_LINES = 'ADD_CONTEXT_LINES';
export const ADD_COLLAPSED_DIFFS = 'ADD_COLLAPSED_DIFFS'; export const ADD_COLLAPSED_DIFFS = 'ADD_COLLAPSED_DIFFS';
export const EXPAND_ALL_FILES = 'EXPAND_ALL_FILES'; export const EXPAND_ALL_FILES = 'EXPAND_ALL_FILES';
export const RENDER_FILE = 'RENDER_FILE'; export const RENDER_FILE = 'RENDER_FILE';
export const SET_LINE_DISCUSSIONS = 'SET_LINE_DISCUSSIONS';
export const REMOVE_LINE_DISCUSSIONS = 'REMOVE_LINE_DISCUSSIONS';
...@@ -6,9 +6,8 @@ import { ...@@ -6,9 +6,8 @@ import {
addLineReferences, addLineReferences,
removeMatchLine, removeMatchLine,
addContextLines, addContextLines,
trimFirstCharOfLineContent, prepareDiffData,
} from './utils'; } from './utils';
import { LINES_TO_BE_RENDERED_DIRECTLY, MAX_LINES_TO_BE_RENDERED } from '../constants';
import * as types from './mutation_types'; import * as types from './mutation_types';
export default { export default {
...@@ -23,40 +22,7 @@ export default { ...@@ -23,40 +22,7 @@ export default {
[types.SET_DIFF_DATA](state, data) { [types.SET_DIFF_DATA](state, data) {
const diffData = convertObjectPropsToCamelCase(data, { deep: true }); const diffData = convertObjectPropsToCamelCase(data, { deep: true });
let showingLines = 0; prepareDiffData(diffData);
const filesLength = diffData.diffFiles.length;
let i;
for (i = 0; i < filesLength; i += 1) {
const file = diffData.diffFiles[i];
if (file.parallelDiffLines) {
const linesLength = file.parallelDiffLines.length;
let u = 0;
for (u = 0; u < linesLength; u += 1) {
const line = file.parallelDiffLines[u];
if (line.left) {
line.left = trimFirstCharOfLineContent(line.left);
}
if (line.right) {
line.right = trimFirstCharOfLineContent(line.right);
}
}
}
if (file.highlightedDiffLines) {
const linesLength = file.highlightedDiffLines.length;
let u;
for (u = 0; u < linesLength; u += 1) {
trimFirstCharOfLineContent(file.highlightedDiffLines[u]);
}
showingLines += file.parallelDiffLines.length;
}
Object.assign(file, {
renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY,
collapsed: file.text && showingLines > MAX_LINES_TO_BE_RENDERED,
});
}
Object.assign(state, { Object.assign(state, {
...diffData, ...diffData,
...@@ -106,12 +72,9 @@ export default { ...@@ -106,12 +72,9 @@ export default {
[types.ADD_COLLAPSED_DIFFS](state, { file, data }) { [types.ADD_COLLAPSED_DIFFS](state, { file, data }) {
const normalizedData = convertObjectPropsToCamelCase(data, { deep: true }); const normalizedData = convertObjectPropsToCamelCase(data, { deep: true });
prepareDiffData(normalizedData);
const [newFileData] = normalizedData.diffFiles.filter(f => f.fileHash === file.fileHash); const [newFileData] = normalizedData.diffFiles.filter(f => f.fileHash === file.fileHash);
Object.assign(file, { ...newFileData });
if (newFileData) {
const index = _.findIndex(state.diffFiles, f => f.fileHash === file.fileHash);
state.diffFiles.splice(index, 1, newFileData);
}
}, },
[types.EXPAND_ALL_FILES](state) { [types.EXPAND_ALL_FILES](state) {
...@@ -120,4 +83,16 @@ export default { ...@@ -120,4 +83,16 @@ export default {
collapsed: false, collapsed: false,
})); }));
}, },
[types.SET_LINE_DISCUSSIONS](state, { line, discussions }) {
Object.assign(line, {
discussions,
});
},
[types.REMOVE_LINE_DISCUSSIONS](state, line) {
Object.assign(line, {
discussions: [],
});
},
}; };
...@@ -8,6 +8,8 @@ import { ...@@ -8,6 +8,8 @@ import {
NEW_LINE_TYPE, NEW_LINE_TYPE,
OLD_LINE_TYPE, OLD_LINE_TYPE,
MATCH_LINE_TYPE, MATCH_LINE_TYPE,
LINES_TO_BE_RENDERED_DIRECTLY,
MAX_LINES_TO_BE_RENDERED,
} from '../constants'; } from '../constants';
export function findDiffFile(files, hash) { export function findDiffFile(files, hash) {
...@@ -179,6 +181,43 @@ export function trimFirstCharOfLineContent(line = {}) { ...@@ -179,6 +181,43 @@ export function trimFirstCharOfLineContent(line = {}) {
return parsedLine; return parsedLine;
} }
export function prepareDiffData(diffData) {
let showingLines = 0;
const filesLength = diffData.diffFiles.length;
let i;
for (i = 0; i < filesLength; i += 1) {
const file = diffData.diffFiles[i];
if (file.parallelDiffLines) {
const linesLength = file.parallelDiffLines.length;
let u = 0;
for (u = 0; u < linesLength; u += 1) {
const line = file.parallelDiffLines[u];
if (line.left) {
line.left = trimFirstCharOfLineContent(line.left);
}
if (line.right) {
line.right = trimFirstCharOfLineContent(line.right);
}
}
}
if (file.highlightedDiffLines) {
const linesLength = file.highlightedDiffLines.length;
let u;
for (u = 0; u < linesLength; u += 1) {
trimFirstCharOfLineContent(file.highlightedDiffLines[u]);
}
showingLines += file.parallelDiffLines.length;
}
Object.assign(file, {
renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY,
collapsed: file.text && showingLines > MAX_LINES_TO_BE_RENDERED,
});
}
}
export function getDiffRefsByLineCode(diffFiles) { export function getDiffRefsByLineCode(diffFiles) {
return diffFiles.reduce((acc, diffFile) => { return diffFiles.reduce((acc, diffFile) => {
const { baseSha, headSha, startSha } = diffFile.diffRefs; const { baseSha, headSha, startSha } = diffFile.diffRefs;
......
...@@ -138,6 +138,7 @@ export default { ...@@ -138,6 +138,7 @@ export default {
.then(() => { .then(() => {
this.isLoading = false; this.isLoading = false;
this.setNotesFetchedState(true); this.setNotesFetchedState(true);
eventHub.$emit('fetchedNotesData');
}) })
.then(() => this.$nextTick()) .then(() => this.$nextTick())
.then(() => this.checkLocationHash()) .then(() => this.checkLocationHash())
......
...@@ -185,16 +185,9 @@ export default { ...@@ -185,16 +185,9 @@ export default {
[types.UPDATE_DISCUSSION](state, noteData) { [types.UPDATE_DISCUSSION](state, noteData) {
const note = noteData; const note = noteData;
let index = 0; const selectedDiscussion = state.discussions.find(disc => disc.id === note.id);
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
state.discussions.splice(index, 1, note); Object.assign(selectedDiscussion, { ...note });
}, },
[types.CLOSE_ISSUE](state) { [types.CLOSE_ISSUE](state) {
......
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