diff --git a/app/assets/javascripts/autosave.js b/app/assets/javascripts/autosave.js index e8c59fab609..fa00a3cf386 100644 --- a/app/assets/javascripts/autosave.js +++ b/app/assets/javascripts/autosave.js @@ -53,8 +53,4 @@ export default class Autosave { return window.localStorage.removeItem(this.key); } - - dispose() { - this.field.off('input'); - } } diff --git a/app/assets/javascripts/diffs/components/diff_discussions.vue b/app/assets/javascripts/diffs/components/diff_discussions.vue index e64d5511d78..20483161033 100644 --- a/app/assets/javascripts/diffs/components/diff_discussions.vue +++ b/app/assets/javascripts/diffs/components/diff_discussions.vue @@ -30,7 +30,6 @@ export default { :render-header="false" :render-diff-file="false" :always-expanded="true" - :discussions-by-diff-order="true" /> 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 d184a76f038..ad838a32518 100644 --- a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue +++ b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue @@ -71,18 +71,13 @@ export default { required: false, default: false, }, - discussions: { - type: Array, - required: false, - default: () => [], - }, }, computed: { ...mapState({ diffViewType: state => state.diffs.diffViewType, diffFiles: state => state.diffs.diffFiles, }), - ...mapGetters(['isLoggedIn']), + ...mapGetters(['isLoggedIn', 'discussionsByLineCode']), lineHref() { return this.lineCode ? `#${this.lineCode}` : '#'; }, @@ -92,19 +87,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: { @@ -189,6 +189,7 @@ export default { diff --git a/app/assets/javascripts/diffs/components/diff_line_note_form.vue b/app/assets/javascripts/diffs/components/diff_line_note_form.vue index cbe4551d06b..32f9516d332 100644 --- a/app/assets/javascripts/diffs/components/diff_line_note_form.vue +++ b/app/assets/javascripts/diffs/components/diff_line_note_form.vue @@ -1,17 +1,17 @@ @@ -57,7 +53,7 @@ export default { :discussions="discussions" /> [], - }, }, 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" /> state.diffs.diffLineCommentForms, }), @@ -38,7 +35,18 @@ export default { 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); + }, + }, }; @@ -57,15 +65,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 bb9a65c83fa..cc5248c25d9 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 +98,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 fc1d15b8d54..9aec117c236 100644 --- a/app/assets/javascripts/diffs/store/getters.js +++ b/app/assets/javascripts/diffs/store/getters.js @@ -1,7 +1,5 @@ import _ from 'underscore'; -import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; import { PARALLEL_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE } from '../constants'; -import { getDiffRefsByLineCode } from './utils'; export const isParallelView = state => state.diffViewType === PARALLEL_DIFF_VIEW_TYPE; @@ -66,87 +64,6 @@ export const getDiffFileDiscussions = (state, getters, rootState, rootGetters) = discussion.diff_discussion && _.isEqual(discussion.diff_file.file_hash, diff.fileHash), ) || []; -/** - * Returns an Object with discussions by their diff line code - * To avoid rendering outdated discussions on the Changes tab we should do a bunch of SHA - * comparisions. `note.position.formatter` have the current version diff refs but - * `note.original_position.formatter` will have the first version's diff refs. - * If line diff refs matches with one of them, we should render it as a discussion on Changes tab. - * - * @param {Object} diff - * @returns {Array} - */ -export const discussionsByLineCode = (state, getters, rootState, rootGetters) => { - const diffRefsByLineCode = getDiffRefsByLineCode(state.diffFiles); - - return rootGetters.discussions.reduce((acc, note) => { - const isDiffDiscussion = note.diff_discussion; - const hasLineCode = note.line_code; - const isResolvable = note.resolvable; - - if (isDiffDiscussion && hasLineCode && isResolvable) { - const diffRefs = diffRefsByLineCode[note.line_code]; - if (diffRefs) { - const refs = convertObjectPropsToCamelCase(note.position.formatter); - const originalRefs = convertObjectPropsToCamelCase(note.original_position.formatter); - - if (_.isEqual(refs, diffRefs) || _.isEqual(originalRefs, diffRefs)) { - const lineCode = note.line_code; - - if (acc[lineCode]) { - acc[lineCode].push(note); - } else { - acc[lineCode] = [note]; - } - } - } - } - - return acc; - }, {}); -}; - -export const singleDiscussionByLineCode = (state, getters) => lineCode => { - if (!lineCode) return []; - const discussions = getters.discussionsByLineCode; - return discussions[lineCode] || []; -}; - -export const shouldRenderParallelCommentRow = (state, getters) => line => { - const leftLineCode = line.left.lineCode; - const rightLineCode = line.right.lineCode; - const leftDiscussions = getters.singleDiscussionByLineCode(leftLineCode); - const rightDiscussions = getters.singleDiscussionByLineCode(rightLineCode); - const hasDiscussion = leftDiscussions.length || rightDiscussions.length; - - const hasExpandedDiscussionOnLeft = leftDiscussions.length - ? leftDiscussions.every(discussion => discussion.expanded) - : false; - const hasExpandedDiscussionOnRight = rightDiscussions.length - ? rightDiscussions.every(discussion => discussion.expanded) - : false; - - if (hasDiscussion && (hasExpandedDiscussionOnLeft || hasExpandedDiscussionOnRight)) { - return true; - } - - const hasCommentFormOnLeft = state.diffLineCommentForms[leftLineCode]; - const hasCommentFormOnRight = state.diffLineCommentForms[rightLineCode]; - - return hasCommentFormOnLeft || hasCommentFormOnRight; -}; - -export const shouldRenderInlineCommentRow = (state, getters) => line => { - if (state.diffLineCommentForms[line.lineCode]) return true; - - const lineDiscussions = getters.singleDiscussionByLineCode(line.lineCode); - if (lineDiscussions.length === 0) { - return false; - } - - return lineDiscussions.every(discussion => discussion.expanded); -}; - // prevent babel-plugin-rewire from generating an invalid default during karma∂ tests export const getDiffFileByHash = state => fileHash => state.diffFiles.find(file => file.fileHash === fileHash); diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index 82082ac508a..d9589baa76e 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -173,24 +173,3 @@ export function trimFirstCharOfLineContent(line = {}) { return parsedLine; } - -export function getDiffRefsByLineCode(diffFiles) { - return diffFiles.reduce((acc, diffFile) => { - const { baseSha, headSha, startSha } = diffFile.diffRefs; - const { newPath, oldPath } = diffFile; - - // We can only use highlightedDiffLines to create the map of diff lines because - // highlightedDiffLines will also include every parallel diff line in it. - if (diffFile.highlightedDiffLines) { - diffFile.highlightedDiffLines.forEach(line => { - const { lineCode, oldLine, newLine } = line; - - if (lineCode) { - acc[lineCode] = { baseSha, headSha, startSha, newPath, oldPath, oldLine, newLine }; - } - }); - } - - return acc; - }, {}); -} diff --git a/app/assets/javascripts/lib/utils/poll.js b/app/assets/javascripts/lib/utils/poll.js index 04a6948f1f1..198711cf427 100644 --- a/app/assets/javascripts/lib/utils/poll.js +++ b/app/assets/javascripts/lib/utils/poll.js @@ -63,7 +63,6 @@ export default class Poll { const headers = normalizeHeaders(response.headers); const pollInterval = parseInt(headers[this.intervalHeader], 10); if (pollInterval > 0 && successCodes.indexOf(response.status) !== -1 && this.canPoll) { - clearTimeout(this.timeoutID); this.timeoutID = setTimeout(() => { this.makeRequest(); }, pollInterval); diff --git a/app/assets/javascripts/notes/components/discussion_counter.vue b/app/assets/javascripts/notes/components/discussion_counter.vue index ad6e7cf501d..6385b75e557 100644 --- a/app/assets/javascripts/notes/components/discussion_counter.vue +++ b/app/assets/javascripts/notes/components/discussion_counter.vue @@ -5,20 +5,19 @@ import resolvedSvg from 'icons/_icon_status_success_solid.svg'; import mrIssueSvg from 'icons/_icon_mr_issue.svg'; import nextDiscussionSvg from 'icons/_next_discussion.svg'; import { pluralize } from '../../lib/utils/text_utility'; -import discussionNavigation from '../mixins/discussion_navigation'; +import { scrollToElement } from '../../lib/utils/common_utils'; import tooltip from '../../vue_shared/directives/tooltip'; export default { directives: { tooltip, }, - mixins: [discussionNavigation], computed: { ...mapGetters([ 'getUserData', 'getNoteableData', 'discussionCount', - 'firstUnresolvedDiscussionId', + 'unresolvedDiscussions', 'resolvedDiscussionCount', ]), isLoggedIn() { @@ -36,6 +35,11 @@ export default { resolveAllDiscussionsIssuePath() { return this.getNoteableData.create_issue_to_resolve_discussions_path; }, + firstUnresolvedDiscussionId() { + const item = this.unresolvedDiscussions[0] || {}; + + return item.id; + }, }, created() { this.resolveSvg = resolveSvg; @@ -46,10 +50,22 @@ export default { methods: { ...mapActions(['expandDiscussion']), jumpToFirstUnresolvedDiscussion() { - const diffTab = window.mrTabs.currentAction === 'diffs'; - const discussionId = this.firstUnresolvedDiscussionId(diffTab); + const discussionId = this.firstUnresolvedDiscussionId; + if (!discussionId) { + return; + } - this.jumpToDiscussion(discussionId); + const el = document.querySelector(`[data-discussion-id="${discussionId}"]`); + const activeTab = window.mrTabs.currentAction; + + if (activeTab === 'commits' || activeTab === 'pipelines') { + window.mrTabs.activateTab('show'); + } + + if (el) { + this.expandDiscussion({ discussionId }); + scrollToElement(el); + } }, }, }; diff --git a/app/assets/javascripts/notes/components/note_form.vue b/app/assets/javascripts/notes/components/note_form.vue index abcd4422d7c..26482a02e00 100644 --- a/app/assets/javascripts/notes/components/note_form.vue +++ b/app/assets/javascripts/notes/components/note_form.vue @@ -7,7 +7,7 @@ import issuableStateMixin from '../mixins/issuable_state'; import resolvable from '../mixins/resolvable'; export default { - name: 'NoteForm', + name: 'IssueNoteForm', components: { issueWarning, markdownField, diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue index 0fe1c16854a..bee635398b3 100644 --- a/app/assets/javascripts/notes/components/noteable_discussion.vue +++ b/app/assets/javascripts/notes/components/noteable_discussion.vue @@ -1,11 +1,11 @@