BigW Consortium Gitlab

Skip to content

G
gitlab-ce
Commit 20bd76da by Eric Eastwood
Options

Fix up some notes tech debt

Fix up tech debt comments from @jschatz1 - https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10837#note_28757436 - https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10837#note_28757905 - https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10837#note_28757952 - https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10837#note_28758013
parent 8e2fefc6
Showing with 115 additions and 109 deletions
app/assets/javascripts/notes.js
/* eslint-disable no-restricted-properties, func-names, space-before-function-paren, no-var, prefer-rest-params, wrap-iife, no-use-before-define, camelcase, no-unused-expressions, quotes, max-len, one-var, one-var-declaration-per-line, default-case, prefer-template, consistent-return, no-alert, no-return-assign, no-param-reassign, prefer-arrow-callback, no-else-return, comma-dangle, no-new, brace-style, no-lonely-if, vars-on-top, no-unused-vars, no-sequences, no-shadow, newline-per-chained-call, no-useless-escape */
/* eslint-disable no-restricted-properties, func-names, space-before-function-paren,
no-var, prefer-rest-params, wrap-iife, no-use-before-define, camelcase,
no-unused-expressions, quotes, max-len, one-var, one-var-declaration-per-line,
default-case, prefer-template, consistent-return, no-alert, no-return-assign,
no-param-reassign, prefer-arrow-callback, no-else-return, comma-dangle, no-new,
brace-style, no-lonely-if, vars-on-top, no-unused-vars, no-sequences, no-shadow,
newline-per-chained-call, no-useless-escape */
/* global Flash */
/* global Autosave */
/* global ResolveService */
......@@ -57,7 +63,7 @@ const normalizeNewlines = function(str) {
this.updatedNotesTrackingMap = {};
this.last_fetched_at = last_fetched_at;
this.noteable_url = document.URL;
this.notesCountBadge || (this.notesCountBadge = $(".issuable-details").find(".notes-tab .badge"));
this.notesCountBadge || (this.notesCountBadge = $('.issuable-details').find('.notes-tab .badge'));
this.basePollingInterval = 15000;
this.maxPollingSteps = 4;
this.flashErrors = [];
......@@ -87,61 +93,61 @@ const normalizeNewlines = function(str) {
Notes.prototype.addBinding = function() {
// Edit note link
$(document).on("click", ".js-note-edit", this.showEditForm.bind(this));
$(document).on("click", ".note-edit-cancel", this.cancelEdit);
$(document).on('click', '.js-note-edit', this.showEditForm.bind(this));
$(document).on('click', '.note-edit-cancel', this.cancelEdit);
// Reopen and close actions for Issue/MR combined with note form submit
$(document).on("click", ".js-comment-submit-button", this.postComment);
$(document).on("click", ".js-comment-save-button", this.updateComment);
$(document).on("keyup input", ".js-note-text", this.updateTargetButtons);
$(document).on('click', '.js-comment-submit-button', this.postComment);
$(document).on('click', '.js-comment-save-button', this.updateComment);
$(document).on('keyup input', '.js-note-text', this.updateTargetButtons);
// resolve a discussion
$(document).on('click', '.js-comment-resolve-button', this.postComment);
// remove a note (in general)
$(document).on("click", ".js-note-delete", this.removeNote);
$(document).on('click', '.js-note-delete', this.removeNote);
// delete note attachment
$(document).on("click", ".js-note-attachment-delete", this.removeAttachment);
$(document).on('click', '.js-note-attachment-delete', this.removeAttachment);
// reset main target form when clicking discard
$(document).on("click", ".js-note-discard", this.resetMainTargetForm);
$(document).on('click', '.js-note-discard', this.resetMainTargetForm);
// update the file name when an attachment is selected
$(document).on("change", ".js-note-attachment-input", this.updateFormAttachment);
$(document).on('change', '.js-note-attachment-input', this.updateFormAttachment);
// reply to diff/discussion notes
$(document).on("click", ".js-discussion-reply-button", this.onReplyToDiscussionNote);
$(document).on('click', '.js-discussion-reply-button', this.onReplyToDiscussionNote);
// add diff note
$(document).on("click", ".js-add-diff-note-button", this.onAddDiffNote);
$(document).on('click', '.js-add-diff-note-button', this.onAddDiffNote);
// hide diff note form
$(document).on("click", ".js-close-discussion-note-form", this.cancelDiscussionForm);
$(document).on('click', '.js-close-discussion-note-form', this.cancelDiscussionForm);
// toggle commit list
$(document).on("click", '.system-note-commit-list-toggler', this.toggleCommitList);
$(document).on('click', '.system-note-commit-list-toggler', this.toggleCommitList);
// fetch notes when tab becomes visible
$(document).on("visibilitychange", this.visibilityChange);
$(document).on('visibilitychange', this.visibilityChange);
// when issue status changes, we need to refresh data
$(document).on("issuable:change", this.refresh);
$(document).on('issuable:change', this.refresh);
// ajax:events that happen on Form when actions like Reopen, Close are performed on Issues and MRs.
$(document).on("ajax:success", ".js-main-target-form", this.addNote);
$(document).on("ajax:success", ".js-discussion-note-form", this.addDiscussionNote);
$(document).on("ajax:success", ".js-main-target-form", this.resetMainTargetForm);
$(document).on("ajax:complete", ".js-main-target-form", this.reenableTargetFormSubmitButton);
$(document).on('ajax:success', '.js-main-target-form', this.addNote);
$(document).on('ajax:success', '.js-discussion-note-form', this.addDiscussionNote);
$(document).on('ajax:success', '.js-main-target-form', this.resetMainTargetForm);
$(document).on('ajax:complete', '.js-main-target-form', this.reenableTargetFormSubmitButton);
// when a key is clicked on the notes
return $(document).on("keydown", ".js-note-text", this.keydownNoteText);
return $(document).on('keydown', '.js-note-text', this.keydownNoteText);
};
Notes.prototype.cleanBinding = function() {
$(document).off("click", ".js-note-edit");
$(document).off("click", ".note-edit-cancel");
$(document).off("click", ".js-note-delete");
$(document).off("click", ".js-note-attachment-delete");
$(document).off("click", ".js-discussion-reply-button");
$(document).off("click", ".js-add-diff-note-button");
$(document).off("visibilitychange");
$(document).off("keyup input", ".js-note-text");
$(document).off("click", ".js-note-target-reopen");
$(document).off("click", ".js-note-target-close");
$(document).off("click", ".js-note-discard");
$(document).off("keydown", ".js-note-text");
$(document).off('click', '.js-note-edit');
$(document).off('click', '.note-edit-cancel');
$(document).off('click', '.js-note-delete');
$(document).off('click', '.js-note-attachment-delete');
$(document).off('click', '.js-discussion-reply-button');
$(document).off('click', '.js-add-diff-note-button');
$(document).off('visibilitychange');
$(document).off('keyup input', '.js-note-text');
$(document).off('click', '.js-note-target-reopen');
$(document).off('click', '.js-note-target-close');
$(document).off('click', '.js-note-discard');
$(document).off('keydown', '.js-note-text');
$(document).off('click', '.js-comment-resolve-button');
$(document).off("click", '.system-note-commit-list-toggler');
$(document).off("ajax:success", ".js-main-target-form");
$(document).off("ajax:success", ".js-discussion-note-form");
$(document).off("ajax:complete", ".js-main-target-form");
$(document).off('click', '.system-note-commit-list-toggler');
$(document).off('ajax:success', '.js-main-target-form');
$(document).off('ajax:success', '.js-discussion-note-form');
$(document).off('ajax:complete', '.js-main-target-form');
};
Notes.initCommentTypeToggle = function (form) {
......@@ -231,8 +237,8 @@ const normalizeNewlines = function(str) {
this.refreshing = true;
return $.ajax({
url: this.notes_url,
headers: { "X-Last-Fetched-At": this.last_fetched_at },
dataType: "json",
headers: { 'X-Last-Fetched-At': this.last_fetched_at },
dataType: 'json',
success: (function(_this) {
return function(data) {
var notes;
......@@ -303,7 +309,7 @@ const normalizeNewlines = function(str) {
*/
Notes.prototype.renderNote = function(noteEntity, $form, $notesList = $('.main-notes-list')) {
if (noteEntity.discussion_html != null) {
if (noteEntity.discussion_html) {
return this.renderDiscussionNote(noteEntity, $form);
}
......@@ -368,8 +374,8 @@ const normalizeNewlines = function(str) {
return;
}
this.note_ids.push(noteEntity.id);
form = $form || $(".js-discussion-note-form[data-discussion-id='" + noteEntity.discussion_id + "']");
row = form.closest("tr");
form = $form || $(`.js-discussion-note-form[data-discussion-id="${noteEntity.discussion_id}"]`);
row = form.closest('tr');
lineType = this.isParallelView() ? form.find('#line_type').val() : 'old';
diffAvatarContainer = row.prevAll('.line_holder').first().find('.js-avatar-container.' + lineType + '_line');
// is this the first note of discussion?
......@@ -386,7 +392,7 @@ const normalizeNewlines = function(str) {
row.after($discussion);
} else {
// Merge new discussion HTML in
var $notes = $discussion.find('.notes[data-discussion-id="' + noteEntity.discussion_id + '"]');
var $notes = $discussion.find(`.notes[data-discussion-id="${noteEntity.discussion_id}"]`);
var contentContainerClass = '.' + $notes.closest('.notes_content')
.attr('class')
.split(' ')
......@@ -397,7 +403,7 @@ const normalizeNewlines = function(str) {
}
// Init discussion on 'Discussion' page if it is merge request page
const page = $('body').attr('data-page');
if ((page && page.indexOf('projects:merge_request') === 0) || !noteEntity.diff_discussion_html) {
if ((page && page.indexOf('projects:merge_request') !== -1) || !noteEntity.diff_discussion_html) {
Notes.animateAppendNote(noteEntity.discussion_html, $('.main-notes-list'));
}
} else {
......@@ -450,13 +456,13 @@ const normalizeNewlines = function(str) {
Notes.prototype.resetMainTargetForm = function(e) {
var form;
form = $(".js-main-target-form");
form = $('.js-main-target-form');
// remove validation errors
form.find(".js-errors").remove();
form.find('.js-errors').remove();
// reset text and preview
form.find(".js-md-write-button").click();
form.find(".js-note-text").val("").trigger("input");
form.find(".js-note-text").data("autosave").reset();
form.find('.js-md-write-button').click();
form.find('.js-note-text').val('').trigger('input');
form.find('.js-note-text').data('autosave').reset();
var event = document.createEvent('Event');
event.initEvent('autosize:update', true, false);
......@@ -467,8 +473,8 @@ const normalizeNewlines = function(str) {
Notes.prototype.reenableTargetFormSubmitButton = function() {
var form;
form = $(".js-main-target-form");
return form.find(".js-note-text").trigger("input");
form = $('.js-main-target-form');
return form.find('.js-note-text').trigger('input');
};
/*
......@@ -480,18 +486,18 @@ const normalizeNewlines = function(str) {
Notes.prototype.setupMainTargetNoteForm = function() {
var form;
// find the form
form = $(".js-new-note-form");
form = $('.js-new-note-form');
// Set a global clone of the form for later cloning
this.formClone = form.clone();
// show the form
this.setupNoteForm(form);
// fix classes
form.removeClass("js-new-note-form");
form.addClass("js-main-target-form");
form.find("#note_line_code").remove();
form.find("#note_position").remove();
form.find("#note_type").val('');
form.find("#in_reply_to_discussion_id").remove();
form.removeClass('js-new-note-form');
form.addClass('js-main-target-form');
form.find('#note_line_code').remove();
form.find('#note_position').remove();
form.find('#note_type').val('');
form.find('#in_reply_to_discussion_id').remove();
form.find('.js-comment-resolve-button').closest('comment-and-resolve-btn').remove();
this.parentTimeline = form.parents('.timeline');
......@@ -512,20 +518,20 @@ const normalizeNewlines = function(str) {
Notes.prototype.setupNoteForm = function(form) {
var textarea, key;
new gl.GLForm(form, this.enableGFM);
textarea = form.find(".js-note-text");
textarea = form.find('.js-note-text');
key = [
"Note",
form.find("#note_noteable_type").val(),
form.find("#note_noteable_id").val(),
form.find("#note_commit_id").val(),
form.find("#note_type").val(),
form.find("#in_reply_to_discussion_id").val(),
'Note',
form.find('#note_noteable_type').val(),
form.find('#note_noteable_id').val(),
form.find('#note_commit_id').val(),
form.find('#note_type').val(),
form.find('#in_reply_to_discussion_id').val(),
// LegacyDiffNote
form.find("#note_line_code").val(),
form.find('#note_line_code').val(),
// DiffNote
form.find("#note_position").val()
form.find('#note_position').val()
];
return new Autosave(textarea, key);
};
......@@ -670,7 +676,8 @@ const normalizeNewlines = function(str) {
const $newNote = $(this.updatedNotesTrackingMap[noteId].html);
$note.replaceWith($newNote);
this.setupNewNote($newNote);
this.updatedNotesTrackingMap[noteId] = null;
// Now that we have taken care of the update, clear it out
delete this.updatedNotesTrackingMap[noteId];
}
else {
$note.find('.js-finish-edit-warning').hide();
......@@ -722,14 +729,14 @@ const normalizeNewlines = function(str) {
lineHolder = $(e.currentTarget).closest('.notes[data-discussion-id]')
.closest('.notes_holder')
.prev('.line_holder');
$(".note[id='" + noteElId + "']").each((function(_this) {
$(`.note[id="${noteElId}"]`).each((function(_this) {
// A same note appears in the "Discussion" and in the "Changes" tab, we have
// to remove all. Using $(".note[id='noteId']") ensure we get all the notes,
// where $("#noteId") would return only one.
// to remove all. Using $('.note[id='noteId']') ensure we get all the notes,
// where $('#noteId') would return only one.
return function(i, el) {
var $note, $notes;
$note = $(el);
$notes = $note.closest(".discussion-notes");
$notes = $note.closest('.discussion-notes');
if (typeof gl.diffNotesCompileComponents !== 'undefined') {
if (gl.diffNoteApps[noteElId]) {
......@@ -740,11 +747,11 @@ const normalizeNewlines = function(str) {
$note.remove();
// check if this is the last note for this line
if ($notes.find(".note").length === 0) {
var notesTr = $notes.closest("tr");
if ($notes.find('.note').length === 0) {
var notesTr = $notes.closest('tr');
// "Discussions" tab
$notes.closest(".timeline-entry").remove();
$notes.closest('.timeline-entry').remove();
// The notes tr can contain multiple lists of notes, like on the parallel diff
if (notesTr.find('.discussion-notes').length > 1) {
......@@ -768,11 +775,11 @@ const normalizeNewlines = function(str) {
*/
Notes.prototype.removeAttachment = function() {
const $note = $(this).closest(".note");
$note.find(".note-attachment").remove();
$note.find(".note-body > .note-text").show();
$note.find(".note-header").show();
return $note.find(".current-note-edit-form").remove();
const $note = $(this).closest('.note');
$note.find('.note-attachment').remove();
$note.find('.note-body > .note-text').show();
$note.find('.note-header').show();
return $note.find('.current-note-edit-form').remove();
};
/*
......@@ -788,7 +795,7 @@ const normalizeNewlines = function(str) {
Notes.prototype.replyToDiscussionNote = function(target) {
var form, replyLink;
form = this.cleanForm(this.formClone.clone());
replyLink = $(target).closest(".js-discussion-reply-button");
replyLink = $(target).closest('.js-discussion-reply-button');
// insert the form after the button
replyLink
.closest('.discussion-reply-holder')
......@@ -808,26 +815,26 @@ const normalizeNewlines = function(str) {
Notes.prototype.setupDiscussionNoteForm = function(dataHolder, form) {
// setup note target
var discussionID = dataHolder.data("discussionId");
var discussionID = dataHolder.data('discussionId');
if (discussionID) {
form.attr("data-discussion-id", discussionID);
form.find("#in_reply_to_discussion_id").val(discussionID);
form.attr('data-discussion-id', discussionID);
form.find('#in_reply_to_discussion_id').val(discussionID);
}
form.attr("data-line-code", dataHolder.data("lineCode"));
form.find("#line_type").val(dataHolder.data("lineType"));
form.attr('data-line-code', dataHolder.data('lineCode'));
form.find('#line_type').val(dataHolder.data('lineType'));
form.find("#note_noteable_type").val(dataHolder.data("noteableType"));
form.find("#note_noteable_id").val(dataHolder.data("noteableId"));
form.find("#note_commit_id").val(dataHolder.data("commitId"));
form.find("#note_type").val(dataHolder.data("noteType"));
form.find('#note_noteable_type').val(dataHolder.data('noteableType'));
form.find('#note_noteable_id').val(dataHolder.data('noteableId'));
form.find('#note_commit_id').val(dataHolder.data('commitId'));
form.find('#note_type').val(dataHolder.data('noteType'));
// LegacyDiffNote
form.find("#note_line_code").val(dataHolder.data("lineCode"));
form.find('#note_line_code').val(dataHolder.data('lineCode'));
// DiffNote
form.find("#note_position").val(dataHolder.attr("data-position"));
form.find('#note_position').val(dataHolder.attr('data-position'));
form.find('.js-note-discard').show().removeClass('js-note-discard').addClass('js-close-discussion-note-form').text(form.find('.js-close-discussion-note-form').data('cancel-text'));
form.find('.js-note-target-close').remove();
......@@ -836,7 +843,7 @@ const normalizeNewlines = function(str) {
form
.removeClass('js-main-target-form')
.addClass("discussion-form js-discussion-note-form");
.addClass('discussion-form js-discussion-note-form');
if (typeof gl.diffNotesCompileComponents !== 'undefined') {
var $commentBtn = form.find('comment-and-resolve-btn');
......@@ -845,7 +852,7 @@ const normalizeNewlines = function(str) {
gl.diffNotesCompileComponents();
}
form.find(".js-note-text").focus();
form.find('.js-note-text').focus();
form
.find('.js-comment-resolve-button')
.attr('data-discussion-id', discussionID);
......@@ -878,21 +885,21 @@ const normalizeNewlines = function(str) {
}) {
var $link, addForm, hasNotes, newForm, noteForm, replyButton, row, rowCssToAdd, targetContent, isDiffCommentAvatar;
$link = $(target);
row = $link.closest("tr");
row = $link.closest('tr');
const nextRow = row.next();
let targetRow = row;
if (nextRow.is('.notes_holder')) {
targetRow = nextRow;
}
hasNotes = targetRow.is(".notes_holder");
hasNotes = nextRow.is('.notes_holder');
addForm = false;
let lineTypeSelector = '';
rowCssToAdd = "<tr class=\"notes_holder js-temp-notes-holder\"><td class=\"notes_line\" colspan=\"2\"></td><td class=\"notes_content\"><div class=\"content\"></div></td></tr>";
rowCssToAdd = '<tr class="notes_holder js-temp-notes-holder"><td class="notes_line" colspan="2"></td><td class="notes_content"><div class="content"></div></td></tr>';
// In parallel view, look inside the correct left/right pane
if (this.isParallelView()) {
lineTypeSelector = `.${lineType}`;
rowCssToAdd = "<tr class=\"notes_holder js-temp-notes-holder\"><td class=\"notes_line old\"></td><td class=\"notes_content parallel old\"><div class=\"content\"></div></td><td class=\"notes_line new\"></td><td class=\"notes_content parallel new\"><div class=\"content\"></div></td></tr>";
rowCssToAdd = '<tr class="notes_holder js-temp-notes-holder"><td class="notes_line old"></td><td class="notes_content parallel old"><div class="content"></div></td><td class="notes_line new"></td><td class="notes_content parallel new"><div class="content"></div></td></tr>';
}
const notesContentSelector = `.notes_content${lineTypeSelector} .content`;
let notesContent = targetRow.find(notesContentSelector);
......@@ -902,12 +909,12 @@ const normalizeNewlines = function(str) {
notesContent = targetRow.find(notesContentSelector);
if (notesContent.length) {
notesContent.show();
replyButton = notesContent.find(".js-discussion-reply-button:visible");
replyButton = notesContent.find('.js-discussion-reply-button:visible');
if (replyButton.length) {
this.replyToDiscussionNote(replyButton[0]);
} else {
// In parallel view, the form may not be present in one of the panes
noteForm = notesContent.find(".js-discussion-note-form");
noteForm = notesContent.find('.js-discussion-note-form');
if (noteForm.length === 0) {
addForm = true;
}
......@@ -945,15 +952,15 @@ const normalizeNewlines = function(str) {
Notes.prototype.removeDiscussionNoteForm = function(form) {
var glForm, row;
row = form.closest("tr");
row = form.closest('tr');
glForm = form.data('gl-form');
glForm.destroy();
form.find(".js-note-text").data("autosave").reset();
form.find('.js-note-text').data('autosave').reset();
// show the reply button (will only work for replies)
form
.prev('.discussion-reply-holder')
.show();
if (row.is(".js-temp-notes-holder")) {
if (row.is('.js-temp-notes-holder')) {
// remove temporary row for diff lines
return row.remove();
} else {
......@@ -965,7 +972,7 @@ const normalizeNewlines = function(str) {
Notes.prototype.cancelDiscussionForm = function(e) {
var form;
e.preventDefault();
form = $(e.target).closest(".js-discussion-note-form");
form = $(e.target).closest('.js-discussion-note-form');
return this.removeDiscussionNoteForm(form);
};
......@@ -977,10 +984,10 @@ const normalizeNewlines = function(str) {
Notes.prototype.updateFormAttachment = function() {
var filename, form;
form = $(this).closest("form");
form = $(this).closest('form');
// get only the basename
filename = $(this).val().replace(/^.*[\\\/]/, "");
return form.find(".js-attachment-filename").text(filename);
filename = $(this).val().replace(/^.*[\\\/]/, '');
return form.find('.js-attachment-filename').text(filename);
};
/*
......
spec/javascripts/notes_spec.js
......@@ -128,7 +128,6 @@ import '~/notes';
beforeEach(() => {
note = {
id: 1,
discussion_html: null,
valid: true,
note: 'heya',
html: '<div>heya</div>',
......
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