Commit b8b2cb36 authored by Tim Zallmann's avatar Tim Zallmann

Fixed Problems with Inline View + Specs

parent d7a5d5b6
......@@ -78,7 +78,7 @@ export default {
}),
...mapGetters(['isLoggedIn']),
lineHref() {
return this.line.code ? `#${this.line.code}` : '#';
return this.line && this.line.code ? `#${this.line.code}` : '#';
},
shouldShowCommentButton() {
return (
......@@ -92,10 +92,10 @@ export default {
);
},
hasDiscussions() {
return this.line.discussions && this.line.discussions.length > 0;
return this.line && this.line.discussions && this.line.discussions.length > 0;
},
shouldShowAvatarsOnGutter() {
if (!this.line.type && this.linePosition === LINE_POSITION_RIGHT) {
if (this.line && !this.line.type && this.linePosition === LINE_POSITION_RIGHT) {
return false;
}
......
......@@ -21,18 +21,13 @@ export default {
type: Number,
required: true,
},
discussions: {
type: Array,
required: false,
default: () => [],
},
},
computed: {
...mapState({
diffLineCommentForms: state => state.diffs.diffLineCommentForms,
}),
className() {
return this.discussions.length ? '' : 'js-temp-notes-holder';
return this.line.discussions.length ? '' : 'js-temp-notes-holder';
},
},
};
......@@ -49,8 +44,8 @@ export default {
>
<div class="content">
<diff-discussions
v-if="discussions.length"
:discussions="discussions"
v-if="line.discussions.length"
:discussions="line.discussions"
/>
<diff-line-note-form
v-if="diffLineCommentForms[line.lineCode]"
......
......@@ -33,11 +33,6 @@ export default {
required: false,
default: false,
},
discussions: {
type: Array,
required: false,
default: () => [],
},
},
data() {
return {
......@@ -94,7 +89,6 @@ export default {
:is-bottom="isBottom"
:is-hover="isHover"
:show-comment-button="true"
:discussions="discussions"
class="diff-line-num old_line"
/>
<diff-table-cell
......@@ -104,7 +98,6 @@ export default {
:line-type="newLineType"
:is-bottom="isBottom"
:is-hover="isHover"
:discussions="discussions"
class="diff-line-num new_line"
/>
<td
......
......@@ -2,7 +2,6 @@
import { mapGetters, mapState } from 'vuex';
import inlineDiffTableRow from './inline_diff_table_row.vue';
import inlineDiffCommentRow from './inline_diff_comment_row.vue';
import { trimFirstCharOfLineContent } from '../store/utils';
export default {
components: {
......@@ -20,29 +19,17 @@ export default {
},
},
computed: {
...mapGetters('diffs', [
'commitId',
'shouldRenderInlineCommentRow',
'singleDiscussionByLineCode',
]),
...mapGetters('diffs', ['commitId', 'shouldRenderInlineCommentRow']),
...mapState({
diffLineCommentForms: state => state.diffs.diffLineCommentForms,
}),
normalizedDiffLines() {
return this.diffLines.map(line => (line.richText ? trimFirstCharOfLineContent(line) : line));
},
diffLinesLength() {
return this.normalizedDiffLines.length;
return this.diffLines.length;
},
userColorScheme() {
return window.gon.user_color_scheme;
},
},
methods: {
discussionsList(line) {
return line.lineCode !== undefined ? this.singleDiscussionByLineCode(line.lineCode) : [];
},
},
};
</script>
......@@ -53,7 +40,7 @@ export default {
class="code diff-wrap-lines js-syntax-highlight text-file js-diff-inline-view">
<tbody>
<template
v-for="(line, index) in normalizedDiffLines"
v-for="(line, index) in diffLines"
>
<inline-diff-table-row
:file-hash="diffFile.fileHash"
......@@ -61,7 +48,6 @@ export default {
:line="line"
:is-bottom="index + 1 === diffLinesLength"
:key="line.lineCode"
:discussions="discussionsList(line)"
/>
<inline-diff-comment-row
v-if="shouldRenderInlineCommentRow(line)"
......@@ -69,7 +55,6 @@ export default {
:line="line"
:line-index="index"
:key="index"
:discussions="discussionsList(line)"
/>
</template>
</tbody>
......
<script>
import $ from 'jquery';
import { mapGetters } from 'vuex';
import DiffTableCell from './diff_table_cell.vue';
import {
NEW_LINE_TYPE,
......
......@@ -30,13 +30,6 @@ export default {
return window.gon.user_color_scheme;
},
},
methods: {
discussionsByLine(line, leftOrRight) {
return line[leftOrRight] && line[leftOrRight].lineCode !== undefined
? this.singleDiscussionByLineCode(line[leftOrRight].lineCode)
: [];
},
},
};
</script>
......
......@@ -32,27 +32,34 @@ export const fetchDiffFiles = ({ state, commit }) => {
export const assignDiscussionsToDiff = ({ state }, allLineDiscussions) => {
Object.values(allLineDiscussions).forEach(discussions => {
if (discussions.length > 0) {
const fileHash = discussions[0].fileHash;
const { fileHash } = discussions[0];
const selectedFile = state.diffFiles.find(file => file.fileHash === fileHash);
if (selectedFile) {
const targetLine = selectedFile.parallelDiffLines.find(line => {
return (
const targetLine = selectedFile.parallelDiffLines.find(
line =>
(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) {
Object.assign(targetLine.right, { discussions });
}
const targetInlineLine = selectedFile.highlightedDiffLines.find(
line => line.lineCode === discussions[0].line_code,
);
if (targetInlineLine) {
Object.assign(targetInlineLine, { discussions });
}
}
}
});
};
export const startRenderDiffsQueue = ({ state, commit }) => {
const checkItem = () => {
return new Promise(resolve => {
const checkItem = () =>
new Promise(resolve => {
const nextFile = state.diffFiles.find(
file => !file.renderIt && (!file.collapsed || !file.text),
);
......@@ -73,7 +80,6 @@ export const startRenderDiffsQueue = ({ state, commit }) => {
resolve();
}
});
};
return new Promise(resolve => {
checkItem()
......
......@@ -17,7 +17,10 @@ export const commitId = state => (state.commit && state.commit.id ? state.commit
export const diffHasAllExpandedDiscussions = (state, getters) => diff => {
const discussions = getters.getDiffFileDiscussions(diff);
return (discussions.length && discussions.every(discussion => discussion.expanded)) || false;
return (
(discussions && discussions.length && discussions.every(discussion => discussion.expanded)) ||
false
);
};
/**
......@@ -28,7 +31,10 @@ export const diffHasAllExpandedDiscussions = (state, getters) => diff => {
export const diffHasAllCollpasedDiscussions = (state, getters) => diff => {
const discussions = getters.getDiffFileDiscussions(diff);
return (discussions.length && discussions.every(discussion => !discussion.expanded)) || false;
return (
(discussions && discussions.length && discussions.every(discussion => !discussion.expanded)) ||
false
);
};
/**
......@@ -40,7 +46,9 @@ export const diffHasExpandedDiscussions = (state, getters) => diff => {
const discussions = getters.getDiffFileDiscussions(diff);
return (
(discussions.length && discussions.find(discussion => discussion.expanded) !== undefined) ||
(discussions &&
discussions.length &&
discussions.find(discussion => discussion.expanded) !== undefined) ||
false
);
};
......@@ -64,17 +72,6 @@ export const getDiffFileDiscussions = (state, getters, rootState, rootGetters) =
discussion.diff_discussion && _.isEqual(discussion.diff_file.file_hash, diff.fileHash),
) || [];
export const singleDiscussionByLineCodeOld = (
state,
getters,
rootState,
rootGetters,
) => lineCode => {
if (!lineCode || lineCode === undefined) return [];
const discussions = rootGetters.discussionsByLineCode;
return discussions[lineCode] || [];
};
/**
* 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
......@@ -113,16 +110,17 @@ export const discussionsByLineCode = (state, getters, rootState, rootGetters) =>
}, {});
};
export const shouldRenderParallelCommentRow = (state, getters) => line => {
export const shouldRenderParallelCommentRow = state => line => {
const hasDiscussion =
(line.left && line.left.discussions.length) || (line.right && line.right.discussions.length);
(line.left && line.left.discussions && line.left.discussions.length) ||
(line.right && line.right.discussions && line.right.discussions.length);
const hasExpandedDiscussionOnLeft =
line.left && line.left.discussions.length
line.left && line.left.discussions && line.left.discussions.length
? line.left.discussions.every(discussion => discussion.expanded)
: false;
const hasExpandedDiscussionOnRight =
line.right && line.right.discussions.length
line.right && line.right.discussions && line.right.discussions.length
? line.right.discussions.every(discussion => discussion.expanded)
: false;
......@@ -136,15 +134,14 @@ export const shouldRenderParallelCommentRow = (state, getters) => line => {
return hasCommentFormOnLeft || hasCommentFormOnRight;
};
export const shouldRenderInlineCommentRow = (state, getters) => line => {
export const shouldRenderInlineCommentRow = state => line => {
if (state.diffLineCommentForms[line.lineCode]) return true;
const lineDiscussions = getters.singleDiscussionByLineCode(line.lineCode);
if (lineDiscussions.length === 0) {
if (!line.discussions && line.discussions.length === 0) {
return false;
}
return lineDiscussions.every(discussion => discussion.expanded);
return line.discussions.every(discussion => discussion.expanded);
};
// prevent babel-plugin-rewire from generating an invalid default during karma∂ tests
......
import Vue from 'vue';
import _ from 'underscore';
import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
import { findDiffFile, addLineReferences, removeMatchLine, addContextLines } from './utils';
import { trimFirstCharOfLineContent } from '../store/utils';
import {
findDiffFile,
addLineReferences,
removeMatchLine,
addContextLines,
trimFirstCharOfLineContent,
} from './utils';
import { LINES_TO_BE_RENDERED_DIRECTLY, MAX_LINES_TO_BE_RENDERED } from '../constants';
import * as types from './mutation_types';
......
......@@ -11,6 +11,16 @@ describe('DiffLineGutterContent', () => {
const createComponent = (options = {}) => {
const cmp = Vue.extend(DiffLineGutterContent);
const props = Object.assign({}, options);
props.line = {
lineCode: 'LC_42',
type: 'new',
oldLine: null,
newLine: 1,
discussions: [],
text: '+<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n',
richText: '+<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n',
metaData: null,
};
props.fileHash = getDiffFileMock().fileHash;
props.contextLinesPath = '/context/lines/path';
......
......@@ -49,6 +49,7 @@ export default {
type: 'new',
oldLine: null,
newLine: 1,
discussions: [],
text: '+<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n',
richText: '+<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n',
metaData: null,
......@@ -58,6 +59,7 @@ export default {
type: 'new',
oldLine: null,
newLine: 2,
discussions: [],
text: '+<span id="LC2" class="line" lang="plaintext"></span>\n',
richText: '+<span id="LC2" class="line" lang="plaintext"></span>\n',
metaData: null,
......@@ -67,6 +69,7 @@ export default {
type: null,
oldLine: 1,
newLine: 3,
discussions: [],
text: ' <span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
richText: ' <span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
metaData: null,
......@@ -76,6 +79,7 @@ export default {
type: null,
oldLine: 2,
newLine: 4,
discussions: [],
text: ' <span id="LC4" class="line" lang="plaintext"></span>\n',
richText: ' <span id="LC4" class="line" lang="plaintext"></span>\n',
metaData: null,
......@@ -85,6 +89,7 @@ export default {
type: null,
oldLine: 3,
newLine: 5,
discussions: [],
text: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
richText: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
metaData: null,
......@@ -112,6 +117,7 @@ export default {
type: 'new',
oldLine: null,
newLine: 1,
discussions: [],
text: '+<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n',
richText: '<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n',
metaData: null,
......@@ -126,6 +132,7 @@ export default {
type: 'new',
oldLine: null,
newLine: 2,
discussions: [],
text: '+<span id="LC2" class="line" lang="plaintext"></span>\n',
richText: '<span id="LC2" class="line" lang="plaintext"></span>\n',
metaData: null,
......@@ -137,6 +144,7 @@ export default {
type: null,
oldLine: 1,
newLine: 3,
discussions: [],
text: ' <span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
richText: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
metaData: null,
......@@ -146,6 +154,7 @@ export default {
type: null,
oldLine: 1,
newLine: 3,
discussions: [],
text: ' <span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
richText: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
metaData: null,
......@@ -157,6 +166,7 @@ export default {
type: null,
oldLine: 2,
newLine: 4,
discussions: [],
text: ' <span id="LC4" class="line" lang="plaintext"></span>\n',
richText: '<span id="LC4" class="line" lang="plaintext"></span>\n',
metaData: null,
......@@ -166,6 +176,7 @@ export default {
type: null,
oldLine: 2,
newLine: 4,
discussions: [],
text: ' <span id="LC4" class="line" lang="plaintext"></span>\n',
richText: '<span id="LC4" class="line" lang="plaintext"></span>\n',
metaData: null,
......@@ -177,6 +188,7 @@ export default {
type: null,
oldLine: 3,
newLine: 5,
discussions: [],
text: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
richText: '<span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
metaData: null,
......@@ -186,6 +198,7 @@ export default {
type: null,
oldLine: 3,
newLine: 5,
discussions: [],
text: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
richText: '<span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
metaData: null,
......@@ -197,6 +210,7 @@ export default {
type: 'match',
oldLine: null,
newLine: null,
discussions: [],
text: '',
richText: '',
metaData: {
......@@ -209,6 +223,7 @@ export default {
type: 'match',
oldLine: null,
newLine: null,
discussions: [],
text: '',
richText: '',
metaData: {
......
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