diff options
Diffstat (limited to 'app/assets/javascripts/diffs')
13 files changed, 104 insertions, 56 deletions
diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue index d3ffbe0415a..517fbf400e8 100644 --- a/app/assets/javascripts/diffs/components/diff_file_header.vue +++ b/app/assets/javascripts/diffs/components/diff_file_header.vue @@ -181,8 +181,8 @@ export default { </span> <strong - v-tooltip v-else + v-tooltip :title="filePath" class="file-title-name" data-container="body" @@ -255,8 +255,8 @@ export default { </a> <a - v-tooltip v-if="diffFile.externalUrl" + v-tooltip :href="diffFile.externalUrl" :title="`View on ${diffFile.formattedExternalUrl}`" target="_blank" diff --git a/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue b/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue index a02c41f39ab..1b59777f901 100644 --- a/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue +++ b/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue @@ -87,8 +87,8 @@ export default { @click.native="toggleDiscussions" /> <span - v-gl-tooltip v-if="moreText" + v-gl-tooltip :title="moreText" class="diff-comments-more-count js-diff-comment-avatar js-diff-comment-plus" data-container="body" 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 a0dc381ccc7..bb9bb821de3 100644 --- a/app/assets/javascripts/diffs/components/diff_line_note_form.vue +++ b/app/assets/javascripts/diffs/components/diff_line_note_form.vue @@ -1,12 +1,9 @@ <script> import { mapState, mapGetters, mapActions } from 'vuex'; -import createFlash from '~/flash'; import { s__ } from '~/locale'; import noteForm from '../../notes/components/note_form.vue'; -import { getNoteFormData } from '../store/utils'; import autosave from '../../notes/mixins/autosave'; import { DIFF_NOTE_TYPE } from '../constants'; -import { reduceDiscussionsToLineCodes } from '../../notes/stores/utils'; export default { components: { @@ -22,7 +19,7 @@ export default { type: Object, required: true, }, - position: { + linePosition: { type: String, required: false, default: '', @@ -39,6 +36,16 @@ export default { }), ...mapGetters('diffs', ['getDiffFileByHash']), ...mapGetters(['isLoggedIn', 'noteableType', 'getNoteableData', 'getNotesDataByProp']), + formData() { + return { + noteableData: this.noteableData, + noteableType: this.noteableType, + noteTargetLine: this.noteTargetLine, + diffViewType: this.diffViewType, + diffFile: this.getDiffFileByHash(this.diffFileHash), + linePosition: this.linePosition, + }; + }, }, mounted() { if (this.isLoggedIn) { @@ -53,8 +60,7 @@ export default { } }, methods: { - ...mapActions('diffs', ['cancelCommentForm', 'assignDiscussionsToDiff']), - ...mapActions(['saveNote', 'refetchDiscussionById']), + ...mapActions('diffs', ['cancelCommentForm', 'assignDiscussionsToDiff', 'saveDiffDiscussion']), handleCancelCommentForm(shouldConfirm, isDirty) { if (shouldConfirm && isDirty) { const msg = s__('Notes|Are you sure you want to cancel creating this comment?'); @@ -73,35 +79,9 @@ export default { }); }, handleSaveNote(note) { - const selectedDiffFile = this.getDiffFileByHash(this.diffFileHash); - const postData = getNoteFormData({ - note, - noteableData: this.noteableData, - noteableType: this.noteableType, - noteTargetLine: this.noteTargetLine, - diffViewType: this.diffViewType, - diffFile: selectedDiffFile, - linePosition: this.position, - }); - - this.saveNote(postData) - .then(result => { - const endpoint = this.getNotesDataByProp('discussionsPath'); - - this.refetchDiscussionById({ path: endpoint, discussionId: result.discussion_id }) - .then(selectedDiscussion => { - const lineCodeDiscussions = reduceDiscussionsToLineCodes([selectedDiscussion]); - this.assignDiscussionsToDiff(lineCodeDiscussions); - - this.handleCancelCommentForm(); - }) - .catch(() => { - createFlash(s__('MergeRequests|Updating discussions failed')); - }); - }) - .catch(() => { - createFlash(s__('MergeRequests|Saving the comment failed')); - }); + return this.saveDiffDiscussion({ note, formData: this.formData }).then(() => + this.handleCancelCommentForm(), + ); }, }, }; diff --git a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue index 0e306f39a9f..62fa34e835a 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue @@ -1,5 +1,5 @@ <script> -import { mapGetters } from 'vuex'; +import { mapGetters, mapActions } from 'vuex'; import DiffTableCell from './diff_table_cell.vue'; import { NEW_LINE_TYPE, @@ -63,7 +63,11 @@ export default { this.linePositionLeft = LINE_POSITION_LEFT; this.linePositionRight = LINE_POSITION_RIGHT; }, + mounted() { + this.scrollToLineIfNeededInline(this.line); + }, methods: { + ...mapActions('diffs', ['scrollToLineIfNeededInline']), handleMouseMove(e) { // To show the comment icon on the gutter we need to know if we hover the line. // Current table structure doesn't allow us to do this with CSS in both of the diff view types diff --git a/app/assets/javascripts/diffs/components/inline_diff_view.vue b/app/assets/javascripts/diffs/components/inline_diff_view.vue index 947e7c98fae..fbf9e77ac07 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_view.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_view.vue @@ -43,18 +43,18 @@ export default { v-for="(line, index) in diffLines" > <inline-diff-table-row + :key="line.lineCode" :file-hash="diffFile.fileHash" :context-lines-path="diffFile.contextLinesPath" :line="line" :is-bottom="index + 1 === diffLinesLength" - :key="line.lineCode" /> <inline-diff-comment-row v-if="shouldRenderInlineCommentRow(line)" + :key="index" :diff-file-hash="diffFile.fileHash" :line="line" :line-index="index" - :key="index" /> </template> </tbody> diff --git a/app/assets/javascripts/diffs/components/no_changes.vue b/app/assets/javascripts/diffs/components/no_changes.vue index d817157fbcd..6905630ad8c 100644 --- a/app/assets/javascripts/diffs/components/no_changes.vue +++ b/app/assets/javascripts/diffs/components/no_changes.vue @@ -38,7 +38,7 @@ export default { <div class="text-center"> <a :href="newBlobPath" - class="btn btn-save" + class="btn btn-success" > {{ __('Create commit') }} </a> 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 26417c350cb..3339c56cbb6 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue @@ -92,7 +92,7 @@ export default { :diff-file-hash="diffFileHash" :line="line.left" :note-target-line="line.left" - position="left" + line-position="left" /> </td> <td class="notes_line new"></td> @@ -111,7 +111,7 @@ export default { :diff-file-hash="diffFileHash" :line="line.right" :note-target-line="line.right" - position="right" + line-position="right" /> </td> </tr> diff --git a/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue index fb68d191091..fcc3b3e9117 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue @@ -1,4 +1,5 @@ <script> +import { mapActions } from 'vuex'; import $ from 'jquery'; import DiffTableCell from './diff_table_cell.vue'; import { @@ -64,7 +65,11 @@ export default { this.oldLineType = OLD_LINE_TYPE; this.parallelDiffViewType = PARALLEL_DIFF_VIEW_TYPE; }, + mounted() { + this.scrollToLineIfNeededParallel(this.line); + }, methods: { + ...mapActions('diffs', ['scrollToLineIfNeededParallel']), handleMouseMove(e) { const isHover = e.type === 'mouseover'; const hoveringCell = e.target.closest('td'); diff --git a/app/assets/javascripts/diffs/components/parallel_diff_view.vue b/app/assets/javascripts/diffs/components/parallel_diff_view.vue index 501bd4450d8..3452f0d2b00 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_view.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_view.vue @@ -45,11 +45,11 @@ export default { v-for="(line, index) in diffLines" > <parallel-diff-table-row + :key="index" :file-hash="diffFile.fileHash" :context-lines-path="diffFile.contextLinesPath" :line="line" :is-bottom="index + 1 === diffLinesLength" - :key="index" /> <parallel-diff-comment-row v-if="shouldRenderParallelCommentRow(line)" diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js index f68afa44837..2795dddfc48 100644 --- a/app/assets/javascripts/diffs/constants.js +++ b/app/assets/javascripts/diffs/constants.js @@ -7,6 +7,7 @@ export const CONTEXT_LINE_TYPE = 'context'; export const EMPTY_CELL_TYPE = 'empty-cell'; export const COMMENT_FORM_TYPE = 'commentForm'; export const DIFF_NOTE_TYPE = 'DiffNote'; +export const LEGACY_DIFF_NOTE_TYPE = 'LegacyDiffNote'; export const NOTE_TYPE = 'Note'; export const NEW_LINE_TYPE = 'new'; export const OLD_LINE_TYPE = 'old'; diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 027df2ec841..98d8d5943f9 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -1,9 +1,12 @@ import Vue from 'vue'; import axios from '~/lib/utils/axios_utils'; import Cookies from 'js-cookie'; +import createFlash from '~/flash'; +import { s__ } from '~/locale'; import { handleLocationHash, historyPushState } from '~/lib/utils/common_utils'; -import { mergeUrlParams } from '~/lib/utils/url_utility'; -import { getDiffPositionByLineCode } from './utils'; +import { mergeUrlParams, getLocationHash } from '~/lib/utils/url_utility'; +import { reduceDiscussionsToLineCodes } from '../../notes/stores/utils'; +import { getDiffPositionByLineCode, getNoteFormData } from './utils'; import * as types from './mutation_types'; import { PARALLEL_DIFF_VIEW_TYPE, @@ -120,6 +123,25 @@ export const loadMoreLines = ({ commit }, options) => { }); }; +export const scrollToLineIfNeededInline = (_, line) => { + const hash = getLocationHash(); + + if (hash && line.lineCode === hash) { + handleLocationHash(); + } +}; + +export const scrollToLineIfNeededParallel = (_, line) => { + const hash = getLocationHash(); + + if ( + hash && + ((line.left && line.left.lineCode === hash) || (line.right && line.right.lineCode === hash)) + ) { + handleLocationHash(); + } +}; + export const loadCollapsedDiff = ({ commit }, file) => axios.get(file.loadCollapsedDiffUrl).then(res => { commit(types.ADD_COLLAPSED_DIFFS, { @@ -159,5 +181,19 @@ export const toggleFileDiscussions = ({ getters, dispatch }, diff) => { }); }; +export const saveDiffDiscussion = ({ dispatch }, { note, formData }) => { + const postData = getNoteFormData({ + note, + ...formData, + }); + + return dispatch('saveNote', postData, { root: true }) + .then(result => dispatch('updateDiscussion', result.discussion, { root: true })) + .then(discussion => + dispatch('assignDiscussionsToDiff', reduceDiscussionsToLineCodes([discussion])), + ) + .catch(() => createFlash(s__('MergeRequests|Saving the comment failed'))); +}; + // prevent babel-plugin-rewire from generating an invalid default during karma tests export default () => {}; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 6dc5bf16c65..59a2c09e54f 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -90,16 +90,18 @@ export default { const firstDiscussion = discussions[0]; const isDiffDiscussion = firstDiscussion.diff_discussion; const hasLineCode = firstDiscussion.line_code; - const isResolvable = firstDiscussion.resolvable; const diffPosition = diffPositionByLineCode[firstDiscussion.line_code]; if ( selectedFile && isDiffDiscussion && hasLineCode && - isResolvable && diffPosition && - isDiscussionApplicableToLine(firstDiscussion, diffPosition) + isDiscussionApplicableToLine({ + discussion: firstDiscussion, + diffPosition, + latestDiff: state.latestDiff, + }) ) { const targetLine = selectedFile.parallelDiffLines.find( line => diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index b7e52a8f37f..631e3de311e 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -4,6 +4,7 @@ import { LINE_POSITION_LEFT, LINE_POSITION_RIGHT, TEXT_DIFF_POSITION_TYPE, + LEGACY_DIFF_NOTE_TYPE, DIFF_NOTE_TYPE, NEW_LINE_TYPE, OLD_LINE_TYPE, @@ -54,13 +55,17 @@ export function getNoteFormData(params) { note_project_id: '', target_type: noteableData.targetType, target_id: noteableData.id, + return_discussion: true, note: { note, position, noteable_type: noteableType, noteable_id: noteableData.id, commit_id: '', - type: DIFF_NOTE_TYPE, + type: + diffFile.diffRefs.startSha && diffFile.diffRefs.headSha + ? DIFF_NOTE_TYPE + : LEGACY_DIFF_NOTE_TYPE, line_code: noteTargetLine.lineCode, }, }; @@ -230,7 +235,16 @@ export function getDiffPositionByLineCode(diffFiles) { const { lineCode, oldLine, newLine } = line; if (lineCode) { - acc[lineCode] = { baseSha, headSha, startSha, newPath, oldPath, oldLine, newLine }; + acc[lineCode] = { + baseSha, + headSha, + startSha, + newPath, + oldPath, + oldLine, + newLine, + lineCode, + }; } }); } @@ -241,9 +255,15 @@ export function getDiffPositionByLineCode(diffFiles) { // This method will check whether the discussion is still applicable // to the diff line in question regarding different versions of the MR -export function isDiscussionApplicableToLine(discussion, diffPosition) { - const originalRefs = convertObjectPropsToCamelCase(discussion.original_position.formatter); - const refs = convertObjectPropsToCamelCase(discussion.position.formatter); +export function isDiscussionApplicableToLine({ discussion, diffPosition, latestDiff }) { + const { lineCode, ...diffPositionCopy } = diffPosition; - return _.isEqual(refs, diffPosition) || _.isEqual(originalRefs, diffPosition); + if (discussion.original_position && discussion.position) { + const originalRefs = convertObjectPropsToCamelCase(discussion.original_position.formatter); + const refs = convertObjectPropsToCamelCase(discussion.position.formatter); + + return _.isEqual(refs, diffPositionCopy) || _.isEqual(originalRefs, diffPositionCopy); + } + + return latestDiff && discussion.active && lineCode === discussion.line_code; } |