diff --git a/app/assets/javascripts/diffs/components/diff_content.vue b/app/assets/javascripts/diffs/components/diff_content.vue index 741462a849c3f07231be8ed66f59ba36f7b14b1a..087a558efdc5d4e859b08d793ef1033a035f73df 100644 --- a/app/assets/javascripts/diffs/components/diff_content.vue +++ b/app/assets/javascripts/diffs/components/diff_content.vue @@ -147,7 +147,7 @@ export default { slot="image-overlay" :discussions="imageDiscussions" :file-hash="diffFileHash" - :can-comment="getNoteableData.current_user.can_create_note" + :can-comment="getNoteableData.current_user.can_create_note && !diffFile.brokenSymlink" /> <div v-if="showNotesContainer" class="note-container"> <user-avatar-link diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index 58a4a3981bf5d3a773b16fc8720669537ab91786..623a4ae1f8ff0d68c4a7bdf1231576dfd7fce06c 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -167,6 +167,7 @@ export default { :id="file.file_hash" :class="{ 'is-active': currentDiffFileId === file.file_hash, + 'comments-disabled': Boolean(file.brokenSymlink), }" :data-path="file.new_path" class="diff-file file-holder" diff --git a/app/assets/javascripts/diffs/components/diff_table_cell.vue b/app/assets/javascripts/diffs/components/diff_table_cell.vue index 198113e330a2fa8df49d90083e7b1e069e363bee..49982a8137283e6fc31511afdd6ccaf9745940d6 100644 --- a/app/assets/javascripts/diffs/components/diff_table_cell.vue +++ b/app/assets/javascripts/diffs/components/diff_table_cell.vue @@ -1,8 +1,9 @@ <script> import { mapGetters, mapActions } from 'vuex'; -import { GlIcon } from '@gitlab/ui'; +import { GlIcon, GlTooltipDirective } from '@gitlab/ui'; import { getParameterByName, parseBoolean } from '~/lib/utils/common_utils'; import DiffGutterAvatars from './diff_gutter_avatars.vue'; +import { __ } from '~/locale'; import { CONTEXT_LINE_TYPE, LINE_POSITION_RIGHT, @@ -18,6 +19,9 @@ export default { DiffGutterAvatars, GlIcon, }, + directives: { + GlTooltip: GlTooltipDirective, + }, props: { line: { type: Object, @@ -123,6 +127,24 @@ export default { lineNumber() { return this.lineType === OLD_LINE_TYPE ? this.line.old_line : this.line.new_line; }, + addCommentTooltip() { + const brokenSymlinks = this.line.commentsDisabled; + let tooltip = __('Add a comment to this line'); + + if (brokenSymlinks) { + if (brokenSymlinks.wasSymbolic || brokenSymlinks.isSymbolic) { + tooltip = __( + 'Commenting on symbolic links that replace or are replaced by files is currently not supported.', + ); + } else if (brokenSymlinks.wasReal || brokenSymlinks.isReal) { + tooltip = __( + 'Commenting on files that replace or are replaced by symbolic links is currently not supported.', + ); + } + } + + return tooltip; + }, }, mounted() { this.unwatchShouldShowCommentButton = this.$watch('shouldShowCommentButton', newVal => { @@ -146,17 +168,24 @@ export default { <template> <td ref="td" :class="classNameMap"> - <button - v-if="shouldRenderCommentButton" - v-show="shouldShowCommentButton" - ref="addDiffNoteButton" - type="button" - class="add-diff-note js-add-diff-note-button qa-diff-comment" - title="Add a comment to this line" - @click="handleCommentButton" + <span + ref="addNoteTooltip" + v-gl-tooltip + class="add-diff-note tooltip-wrapper" + :title="addCommentTooltip" > - <gl-icon :size="12" name="comment" /> - </button> + <button + v-if="shouldRenderCommentButton" + v-show="shouldShowCommentButton" + ref="addDiffNoteButton" + type="button" + class="add-diff-note note-button js-add-diff-note-button qa-diff-comment" + :disabled="line.commentsDisabled" + @click="handleCommentButton" + > + <gl-icon :size="12" name="comment" /> + </button> + </span> <a v-if="lineNumber" ref="lineNumberRef" diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index 2d87008d3b1a1e2e29b5d741735a4a8e777faf88..f014cddda32f73c13fa863f829529619e281c8de 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -480,6 +480,10 @@ export function getDiffPositionByLineCode(diffFiles, useSingleDiffStyle) { // 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, latestDiff }) { + if (!diffPosition) { + return false; + } + const { line_code, ...dp } = diffPosition; // Removing `line_range` from diffPosition because the backend does not // yet consistently return this property. This check can be removed, diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 46ea3c37f2d15817386891a03874146a6f19a62e..e4e5450162731e3ecf4a1a1b17283a812155fd6f 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -820,9 +820,7 @@ $note-form-margin-left: 72px; } } -.add-diff-note { - @include btn-comment-icon; - opacity: 0; +.tooltip-wrapper.add-diff-note { margin-left: -52px; position: absolute; top: 50%; @@ -830,6 +828,18 @@ $note-form-margin-left: 72px; z-index: 10; } +.note-button.add-diff-note { + @include btn-comment-icon; + opacity: 0; + + &[disabled] { + background: $white; + border-color: $gray-200; + color: $gl-gray-400; + cursor: not-allowed; + } +} + .disabled-comment { background-color: $gray-light; border-radius: $border-radius-base; diff --git a/changelogs/unreleased/symlink-comments-disable-symlink-comments.yml b/changelogs/unreleased/symlink-comments-disable-symlink-comments.yml new file mode 100644 index 0000000000000000000000000000000000000000..2e1d19ae2a95e87665771f06ce0ec444aa66b713 --- /dev/null +++ b/changelogs/unreleased/symlink-comments-disable-symlink-comments.yml @@ -0,0 +1,6 @@ +--- +title: Disable commenting on lines in files that were or are symlinks or replace or + are replaced by symlinks +merge_request: 35371 +author: +type: fixed diff --git a/locale/gitlab.pot b/locale/gitlab.pot index ff2627e69a5f5b1d36c2ec2b41f0d77f5b7cc7e5..eba71cacad11a3529b4a360893c426d138eb13e7 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -6161,6 +6161,12 @@ msgstr "" msgid "Comment/Reply (quoting selected text)" msgstr "" +msgid "Commenting on files that replace or are replaced by symbolic links is currently not supported." +msgstr "" + +msgid "Commenting on symbolic links that replace or are replaced by files is currently not supported." +msgstr "" + msgid "Comments" msgstr "" diff --git a/spec/frontend/diffs/components/diff_table_cell_spec.js b/spec/frontend/diffs/components/diff_table_cell_spec.js index 24daef62ee2a83b538fef85dd9c593c2323b2e35..02f5c27eecb25de1babeaf633f7bf8d308e6dfe3 100644 --- a/spec/frontend/diffs/components/diff_table_cell_spec.js +++ b/spec/frontend/diffs/components/diff_table_cell_spec.js @@ -18,6 +18,12 @@ const TEST_LINE_CODE = 'LC_42'; const TEST_FILE_HASH = diffFileMockData.file_hash; describe('DiffTableCell', () => { + const symlinkishFileTooltip = + 'Commenting on symbolic links that replace or are replaced by files is currently not supported.'; + const realishFileTooltip = + 'Commenting on files that replace or are replaced by symbolic links is currently not supported.'; + const otherFileTooltip = 'Add a comment to this line'; + let wrapper; let line; let store; @@ -67,6 +73,7 @@ describe('DiffTableCell', () => { const findTd = () => wrapper.find({ ref: 'td' }); const findNoteButton = () => wrapper.find({ ref: 'addDiffNoteButton' }); const findLineNumber = () => wrapper.find({ ref: 'lineNumberRef' }); + const findTooltip = () => wrapper.find({ ref: 'addNoteTooltip' }); const findAvatars = () => wrapper.find(DiffGutterAvatars); describe('td', () => { @@ -134,6 +141,53 @@ describe('DiffTableCell', () => { }); }, ); + + it.each` + disabled | commentsDisabled + ${'disabled'} | ${true} + ${undefined} | ${false} + `( + 'has attribute disabled=$disabled when the outer component has prop commentsDisabled=$commentsDisabled', + ({ disabled, commentsDisabled }) => { + line.commentsDisabled = commentsDisabled; + + createComponent({ + showCommentButton: true, + isHover: true, + }); + + wrapper.setData({ isCommentButtonRendered: true }); + + return wrapper.vm.$nextTick().then(() => { + expect(findNoteButton().attributes('disabled')).toBe(disabled); + }); + }, + ); + + it.each` + tooltip | commentsDisabled + ${symlinkishFileTooltip} | ${{ wasSymbolic: true }} + ${symlinkishFileTooltip} | ${{ isSymbolic: true }} + ${realishFileTooltip} | ${{ wasReal: true }} + ${realishFileTooltip} | ${{ isReal: true }} + ${otherFileTooltip} | ${false} + `( + 'has the correct tooltip when commentsDisabled=$commentsDisabled', + ({ tooltip, commentsDisabled }) => { + line.commentsDisabled = commentsDisabled; + + createComponent({ + showCommentButton: true, + isHover: true, + }); + + wrapper.setData({ isCommentButtonRendered: true }); + + return wrapper.vm.$nextTick().then(() => { + expect(findTooltip().attributes('title')).toBe(tooltip); + }); + }, + ); }); describe('line number', () => {