diff --git a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue index d184a76f0386fb3e07d680245fb291beabcebecd..0fe0007057b38739d23fa05ec1fb400e1e38e7b8 100644 --- a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue +++ b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue @@ -71,11 +71,6 @@ export default { required: false, default: false, }, - discussions: { - type: Array, - required: false, - default: () => [], - }, }, computed: { ...mapState({ @@ -83,6 +78,7 @@ export default { diffFiles: state => state.diffs.diffFiles, }), ...mapGetters(['isLoggedIn']), + ...mapGetters('diffs', ['discussionsByLineCode']), lineHref() { return this.lineCode ? `#${this.lineCode}` : '#'; }, @@ -92,19 +88,24 @@ export default { this.showCommentButton && !this.isMatchLine && !this.isContextLine && - !this.isMetaLine && - !this.hasDiscussions + !this.hasDiscussions && + !this.isMetaLine ); }, + discussions() { + return this.discussionsByLineCode[this.lineCode] || []; + }, hasDiscussions() { return this.discussions.length > 0; }, shouldShowAvatarsOnGutter() { + let render = this.hasDiscussions && this.showCommentButton; + if (!this.lineType && this.linePosition === LINE_POSITION_RIGHT) { - return false; + render = false; } - return this.hasDiscussions && this.showCommentButton; + return render; }, }, methods: { diff --git a/app/assets/javascripts/diffs/components/diff_table_cell.vue b/app/assets/javascripts/diffs/components/diff_table_cell.vue index e8e8ddc6c5e7d9fc6809b35a86600bef17b68f34..5962f30d9bb1922c72aa2ce6d6ff38ff86154907 100644 --- a/app/assets/javascripts/diffs/components/diff_table_cell.vue +++ b/app/assets/javascripts/diffs/components/diff_table_cell.vue @@ -67,11 +67,6 @@ export default { required: false, default: false, }, - discussions: { - type: Array, - required: false, - default: () => [], - }, }, computed: { ...mapGetters(['isLoggedIn']), @@ -141,7 +136,6 @@ export default { :is-match-line="isMatchLine" :is-context-line="isContentLine" :is-meta-line="isMetaLine" - :discussions="discussions" /> diff --git a/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue b/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue index 1b5ae5e9f75b0583a80ff17cfe4e62c1eb1dd0a3..a6f011ff31e4dd3ec41ef74bec7203bca91bf081 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue @@ -1,5 +1,5 @@ @@ -57,15 +64,13 @@ export default { :line="line" :is-bottom="index + 1 === diffLinesLength" :key="line.lineCode" - :discussions="singleDiscussionByLineCode(line.lineCode)" /> diff --git a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue index bb9a65c83fad2d832abe20abf4b2998782ef0945..05e5cafc717188d9a8db12399624a0bc989bec30 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue @@ -1,5 +1,5 @@ @@ -75,17 +97,13 @@ export default { :line="line" :is-bottom="index + 1 === diffLinesLength" :key="index" - :left-discussions="singleDiscussionByLineCode(line.left.lineCode)" - :right-discussions="singleDiscussionByLineCode(line.right.lineCode)" /> diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js index c7b9b1a16e6ca43be7ce2186fddb4cbf02fab88d..d3881fa1a0a55c0db256b23329b62f2af69dcbbb 100644 --- a/app/assets/javascripts/diffs/store/getters.js +++ b/app/assets/javascripts/diffs/store/getters.js @@ -75,21 +75,19 @@ export const discussionsByLineCode = (state, getters, rootState, rootGetters) => const isDiffDiscussion = note.diff_discussion; const hasLineCode = note.line_code; const isResolvable = note.resolvable; + const diffRefs = diffRefsByLineCode[note.line_code]; - 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 (isDiffDiscussion && hasLineCode && isResolvable && 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 (_.isEqual(refs, diffRefs) || _.isEqual(originalRefs, diffRefs)) { + const lineCode = note.line_code; - if (acc[lineCode]) { - acc[lineCode].push(note); - } else { - acc[lineCode] = [note]; - } + if (acc[lineCode]) { + acc[lineCode].push(note); + } else { + acc[lineCode] = [note]; } } } @@ -98,47 +96,6 @@ export const discussionsByLineCode = (state, getters, rootState, rootGetters) => }, {}); }; -export const singleDiscussionByLineCode = (state, getters) => lineCode => { - if (!lineCode) return []; - const discussions = getters.discussionsByLineCode; - return discussions[lineCode] || []; -}; - -export const shouldRenderParallelCommentRow = (state, getters) => line => { - const leftLineCode = line.left.lineCode; - const rightLineCode = line.right.lineCode; - const leftDiscussions = getters.singleDiscussionByLineCode(leftLineCode); - const rightDiscussions = getters.singleDiscussionByLineCode(rightLineCode); - const hasDiscussion = leftDiscussions.length || rightDiscussions.length; - - const hasExpandedDiscussionOnLeft = leftDiscussions.length - ? leftDiscussions.every(discussion => discussion.expanded) - : false; - const hasExpandedDiscussionOnRight = rightDiscussions.length - ? rightDiscussions.every(discussion => discussion.expanded) - : false; - - if (hasDiscussion && (hasExpandedDiscussionOnLeft || hasExpandedDiscussionOnRight)) { - return true; - } - - const hasCommentFormOnLeft = state.diffLineCommentForms[leftLineCode]; - const hasCommentFormOnRight = state.diffLineCommentForms[rightLineCode]; - - return hasCommentFormOnLeft || hasCommentFormOnRight; -}; - -export const shouldRenderInlineCommentRow = (state, getters) => line => { - if (state.diffLineCommentForms[line.lineCode]) return true; - - const lineDiscussions = getters.singleDiscussionByLineCode(line.lineCode); - if (lineDiscussions.length === 0) { - return false; - } - - return lineDiscussions.every(discussion => discussion.expanded); -}; - // prevent babel-plugin-rewire from generating an invalid default during karma∂ tests export const getDiffFileByHash = state => fileHash => state.diffFiles.find(file => file.fileHash === fileHash); diff --git a/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js b/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js index bdc94131fc2d66689948589cc6fd05f98886c4cd..cb85d12daf253f5b2feba39cdd422b9bf89bd437 100644 --- a/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js +++ b/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js @@ -50,11 +50,7 @@ describe('DiffLineGutterContent', () => { it('should return discussions for the given lineCode', () => { const { lineCode } = getDiffFileMock().highlightedDiffLines[1]; - const component = createComponent({ - lineCode, - showCommentButton: true, - discussions: getDiscussionsMockData(), - }); + const component = createComponent({ lineCode, showCommentButton: true }); setDiscussions(component);