summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>2013-01-15 12:43:09 +0200
committerDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>2013-01-15 12:43:09 +0200
commit6ddacaf9bfe02378e610ce277cd901097dcf6fdd (patch)
treef00911ad3a1f0e4042f20ded0c234cb88303199c
parentd8e697ac68d758d4f451594047056c459f546bf7 (diff)
parentac983319d3dca597d9a1dcfa46f6acc6ebcfa07a (diff)
downloadgitlab-ce-6ddacaf9bfe02378e610ce277cd901097dcf6fdd.tar.gz
Merge branch 'riyad-discussions'
-rw-r--r--app/assets/images/comment_add.pngbin781 -> 0 bytes
-rw-r--r--app/assets/images/diff_note_add.pngbin0 -> 691 bytes
-rw-r--r--app/assets/javascripts/behaviors/details_behavior.coffee5
-rw-r--r--app/assets/javascripts/behaviors/toggler_behavior.coffee5
-rw-r--r--app/assets/javascripts/extensions/array.js7
-rw-r--r--app/assets/javascripts/notes.js675
-rw-r--r--app/assets/stylesheets/application.scss5
-rw-r--r--app/assets/stylesheets/behaviors.scss14
-rw-r--r--app/assets/stylesheets/common.scss6
-rw-r--r--app/assets/stylesheets/sections/commits.scss15
-rw-r--r--app/assets/stylesheets/sections/notes.scss427
-rw-r--r--app/contexts/notes/create_context.rb4
-rw-r--r--app/contexts/notes/load_context.rb6
-rw-r--r--app/controllers/commit_controller.rb16
-rw-r--r--app/controllers/issues_controller.rb2
-rw-r--r--app/controllers/merge_requests_controller.rb7
-rw-r--r--app/controllers/notes_controller.rb41
-rw-r--r--app/controllers/projects_controller.rb5
-rw-r--r--app/controllers/snippets_controller.rb2
-rw-r--r--app/helpers/commits_helper.rb10
-rw-r--r--app/helpers/notes_helper.rb34
-rw-r--r--app/mailers/notify.rb4
-rw-r--r--app/models/concerns/issuable.rb24
-rw-r--r--app/models/note.rb116
-rw-r--r--app/observers/note_observer.rb2
-rw-r--r--app/views/commit/show.html.haml4
-rw-r--r--app/views/commits/_diffs.html.haml4
-rw-r--r--app/views/commits/_text_diff.html.haml (renamed from app/views/commits/_text_file.html.haml)12
-rw-r--r--app/views/issues/show.html.haml2
-rw-r--r--app/views/merge_requests/_show.html.haml6
-rw-r--r--app/views/merge_requests/diffs.html.haml5
-rw-r--r--app/views/merge_requests/diffs.js.haml3
-rw-r--r--app/views/merge_requests/show.js.haml2
-rw-r--r--app/views/notes/_common_form.html.haml39
-rw-r--r--app/views/notes/_create_common_note.js.haml13
-rw-r--r--app/views/notes/_create_per_line_note.js.haml19
-rw-r--r--app/views/notes/_diff_note_link.html.haml10
-rw-r--r--app/views/notes/_diff_notes_with_reply.html.haml11
-rw-r--r--app/views/notes/_discussion.html.haml63
-rw-r--r--app/views/notes/_discussion_diff.html.haml25
-rw-r--r--app/views/notes/_discussion_reply_button.html.haml10
-rw-r--r--app/views/notes/_form.html.haml44
-rw-r--r--app/views/notes/_form_errors.html.haml3
-rw-r--r--app/views/notes/_note.html.haml67
-rw-r--r--app/views/notes/_notes.html.haml15
-rw-r--r--app/views/notes/_notes_with_form.html.haml10
-rw-r--r--app/views/notes/_per_line_form.html.haml40
-rw-r--r--app/views/notes/_per_line_note.html.haml5
-rw-r--r--app/views/notes/_per_line_note_link.html.haml1
-rw-r--r--app/views/notes/_per_line_notes_with_reply.html.haml3
-rw-r--r--app/views/notes/_per_line_reply_button.html.haml4
-rw-r--r--app/views/notes/_reversed_notes_with_form.html.haml11
-rw-r--r--app/views/notes/create.js.haml27
-rw-r--r--app/views/notes/index.js.haml14
-rw-r--r--app/views/projects/wall.html.haml2
-rw-r--r--app/views/snippets/show.html.haml2
-rw-r--r--features/project/commits/commit_comments.feature46
-rw-r--r--features/project/commits/commit_diff_comments.feature91
-rw-r--r--features/project/merge_requests.feature28
-rw-r--r--features/steps/project/comments_on_commit_diffs.rb6
-rw-r--r--features/steps/project/comments_on_commits.rb (renamed from features/steps/project/project_comment_commit.rb)4
-rw-r--r--features/steps/project/project_merge_requests.rb138
-rw-r--r--features/steps/shared/diff_note.rb158
-rw-r--r--features/steps/shared/note.rb105
-rw-r--r--features/steps/shared/paths.rb5
-rw-r--r--lib/gitlab/markdown.rb34
-rw-r--r--spec/factories.rb30
-rw-r--r--spec/lib/votes_spec.rb116
-rw-r--r--spec/models/note_spec.rb83
-rw-r--r--spec/requests/notes_on_merge_requests_spec.rb233
-rw-r--r--spec/requests/notes_on_wall_spec.rb85
71 files changed, 2232 insertions, 838 deletions
diff --git a/app/assets/images/comment_add.png b/app/assets/images/comment_add.png
deleted file mode 100644
index 836557ac846..00000000000
--- a/app/assets/images/comment_add.png
+++ /dev/null
Binary files differ
diff --git a/app/assets/images/diff_note_add.png b/app/assets/images/diff_note_add.png
new file mode 100644
index 00000000000..8ec15b701fc
--- /dev/null
+++ b/app/assets/images/diff_note_add.png
Binary files differ
diff --git a/app/assets/javascripts/behaviors/details_behavior.coffee b/app/assets/javascripts/behaviors/details_behavior.coffee
new file mode 100644
index 00000000000..7ad5c818946
--- /dev/null
+++ b/app/assets/javascripts/behaviors/details_behavior.coffee
@@ -0,0 +1,5 @@
+$ ->
+ $("body").on "click", ".js-details-target", ->
+ container = $(@).closest(".js-details-container")
+
+ container.toggleClass("open")
diff --git a/app/assets/javascripts/behaviors/toggler_behavior.coffee b/app/assets/javascripts/behaviors/toggler_behavior.coffee
new file mode 100644
index 00000000000..3fefbf8e121
--- /dev/null
+++ b/app/assets/javascripts/behaviors/toggler_behavior.coffee
@@ -0,0 +1,5 @@
+$ ->
+ $("body").on "click", ".js-toggler-target", ->
+ container = $(@).closest(".js-toggler-container")
+
+ container.toggleClass("on")
diff --git a/app/assets/javascripts/extensions/array.js b/app/assets/javascripts/extensions/array.js
new file mode 100644
index 00000000000..7fccc9c9d5f
--- /dev/null
+++ b/app/assets/javascripts/extensions/array.js
@@ -0,0 +1,7 @@
+Array.prototype.first = function() {
+ return this[0];
+}
+
+Array.prototype.last = function() {
+ return this[this.length-1];
+} \ No newline at end of file
diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js
index b6f65b7aa5e..8a7e08dd0e8 100644
--- a/app/assets/javascripts/notes.js
+++ b/app/assets/javascripts/notes.js
@@ -9,72 +9,315 @@ var NoteList = {
loading_more_disabled: false,
reversed: false,
- init:
- function(tid, tt, path) {
- this.notes_path = path + ".js";
- this.target_id = tid;
- this.target_type = tt;
- this.reversed = $("#notes-list").is(".reversed");
- this.target_params = "target_type=" + this.target_type + "&target_id=" + this.target_id;
-
- // get initial set of notes
- this.getContent();
-
- $("#notes-list, #new-notes-list").on("ajax:success", ".delete-note", function() {
- $(this).closest('li').fadeOut(function() {
- $(this).remove();
- NoteList.updateVotes();
- });
+ init: function(tid, tt, path) {
+ NoteList.notes_path = path + ".js";
+ NoteList.target_id = tid;
+ NoteList.target_type = tt;
+ NoteList.reversed = $("#notes-list").is(".reversed");
+ NoteList.target_params = "target_type=" + NoteList.target_type + "&target_id=" + NoteList.target_id;
+
+ NoteList.setupMainTargetNoteForm();
+
+ if(NoteList.reversed) {
+ var form = $(".js-main-target-form");
+ form.find(".buttons, .note_options").hide();
+ var textarea = form.find(".js-note-text");
+ textarea.css("height", "40px");
+ textarea.on("focus", function(){
+ textarea.css("height", "80px");
+ form.find(".buttons, .note_options").show();
});
+ }
- $(".note-form-holder").on("ajax:before", function(){
- $(".submit_note").disable();
- })
+ // get initial set of notes
+ NoteList.getContent();
- $(".note-form-holder").on("ajax:complete", function(){
- $(".submit_note").enable();
- $('#preview-note').hide();
- $('#note_note').show();
- })
+ // add a new diff note
+ $(document).on("click",
+ ".js-add-diff-note-button",
+ NoteList.addDiffNote);
- disableButtonIfEmptyField(".note-text", ".submit_note");
+ // reply to diff/discussion notes
+ $(document).on("click",
+ ".js-discussion-reply-button",
+ NoteList.replyToDiscussionNote);
- $("#note_attachment").change(function(e){
- var val = $('.input-file').val();
- var filename = val.replace(/^.*[\\\/]/, '');
- $(".file_name").text(filename);
- });
+ // setup note preview
+ $(document).on("click",
+ ".js-note-preview-button",
+ NoteList.previewNote);
+
+ // update the file name when an attachment is selected
+ $(document).on("change",
+ ".js-note-attachment-input",
+ NoteList.updateFormAttachment);
+
+ // hide diff note form
+ $(document).on("click",
+ ".js-close-discussion-note-form",
+ NoteList.removeDiscussionNoteForm);
+
+ // remove a note (in general)
+ $(document).on("click",
+ ".js-note-delete",
+ NoteList.removeNote);
+
+ // reset main target form after submit
+ $(document).on("ajax:complete",
+ ".js-main-target-form",
+ NoteList.resetMainTargetForm);
+
+
+ $(document).on("click",
+ ".js-choose-note-attachment-button",
+ NoteList.chooseNoteAttachment);
+
+ $(document).on("click",
+ ".js-show-outdated-discussion",
+ function(e) { $(this).next('.outdated-discussion').show(); e.preventDefault() });
+ },
+
+
+ /**
+ * When clicking on buttons
+ */
+
+ /**
+ * Called when clicking on the "add a comment" button on the side of a diff line.
+ *
+ * Inserts a temporary row for the form below the line.
+ * Sets up the form and shows it.
+ */
+ addDiffNote: function(e) {
+ e.preventDefault();
+
+ // find the form
+ var form = $(".js-new-note-form");
+ var row = $(this).closest("tr");
+ var nextRow = row.next();
+
+ // does it already have notes?
+ if (nextRow.is(".notes_holder")) {
+ $.proxy(NoteList.replyToDiscussionNote,
+ nextRow.find(".js-discussion-reply-button")
+ ).call();
+ } else {
+ // add a notes row and insert the form
+ row.after('<tr class="notes_holder js-temp-notes-holder"><td class="notes_line" colspan="2"></td><td class="notes_content"></td></tr>');
+ form.clone().appendTo(row.next().find(".notes_content"));
+
+ // show the form
+ NoteList.setupDiscussionNoteForm($(this), row.next().find("form"));
+ }
+ },
+
+ /**
+ * Called when clicking the "Choose File" button.
+ *
+ * Opesn the file selection dialog.
+ */
+ chooseNoteAttachment: function() {
+ var form = $(this).closest("form");
- if(this.reversed) {
- var textarea = $(".note-text");
- $('.note_advanced_opts').hide();
- textarea.css("height", "40px");
- textarea.on("focus", function(){
- $(this).css("height", "80px");
- $('.note_advanced_opts').show();
+ form.find(".js-note-attachment-input").click();
+ },
+
+ /**
+ * Shows the note preview.
+ *
+ * Lets the server render GFM into Html and displays it.
+ *
+ * Note: uses the Toggler behavior to toggle preview/edit views/buttons
+ */
+ previewNote: function(e) {
+ e.preventDefault();
+
+ var form = $(this).closest("form");
+ var preview = form.find('.js-note-preview');
+ var noteText = form.find('.js-note-text').val();
+
+ if(noteText.trim().length === 0) {
+ preview.text('Nothing to preview.');
+ } else {
+ preview.text('Loading...');
+ $.post($(this).data('url'), {note: noteText})
+ .success(function(previewData) {
+ preview.html(previewData);
});
- }
+ }
+ },
+
+ /**
+ * Called in response to "cancel" on a diff note form.
+ *
+ * Shows the reply button again.
+ * Removes the form and if necessary it's temporary row.
+ */
+ removeDiscussionNoteForm: function() {
+ var form = $(this).closest("form");
+ var row = form.closest("tr");
+
+ // show the reply button (will only work for replys)
+ form.prev(".js-discussion-reply-button").show();
+
+ if (row.is(".js-temp-notes-holder")) {
+ // remove temporary row for diff lines
+ row.remove();
+ } else {
+ // only remove the form
+ form.remove();
+ }
+ },
- // Setup note preview
- $(document).on('click', '#preview-link', function(e) {
- $('#preview-note').text('Loading...');
+ /**
+ * Called in response to deleting a note of any kind.
+ *
+ * Removes the actual note from view.
+ * Removes the whole discussion if the last note is being removed.
+ */
+ removeNote: function() {
+ var note = $(this).closest(".note");
+ var notes = note.closest(".notes");
- $(this).text($(this).text() === "Edit" ? "Preview" : "Edit");
+ // check if this is the last note for this line
+ if (notes.find(".note").length === 1) {
+ // for discussions
+ notes.closest(".discussion").remove();
- var note_text = $('#note_note').val();
+ // for diff lines
+ notes.closest("tr").remove();
+ }
- if(note_text.trim().length === 0) {
- $('#preview-note').text('Nothing to preview.');
- } else {
- $.post($(this).attr('href'), {note: note_text}).success(function(data) {
- $('#preview-note').html(data);
- });
- }
+ note.remove();
+ NoteList.updateVotes();
+ },
- $('#preview-note, #note_note').toggle();
- e.preventDefault();
- });
- },
+ /**
+ * Called when clicking on the "reply" button for a diff line.
+ *
+ * Shows the note form below the notes.
+ */
+ replyToDiscussionNote: function() {
+ // find the form
+ var form = $(".js-new-note-form");
+
+ // hide reply button
+ $(this).hide();
+ // insert the form after the button
+ form.clone().insertAfter($(this));
+
+ // show the form
+ NoteList.setupDiscussionNoteForm($(this), $(this).next("form"));
+ },
+
+
+ /**
+ * Helper for inserting and setting up note forms.
+ */
+
+
+ /**
+ * Called in response to creating a note failing validation.
+ *
+ * Adds the rendered errors to the respective form.
+ * If "discussionId" is null or undefined, the main target form is assumed.
+ */
+ errorsOnForm: function(errorsHtml, discussionId) {
+ // find the form
+ if (discussionId) {
+ var form = $("form[rel='"+discussionId+"']");
+ } else {
+ var form = $(".js-main-target-form");
+ }
+
+ form.find(".js-errors").remove();
+ form.prepend(errorsHtml);
+
+ form.find(".js-note-text").focus();
+ },
+
+
+ /**
+ * Shows the diff/discussion form and does some setup on it.
+ *
+ * Sets some hidden fields in the form.
+ *
+ * Note: dataHolder must have the "discussionId", "lineCode", "noteableType"
+ * and "noteableId" data attributes set.
+ */
+ setupDiscussionNoteForm: function(dataHolder, form) {
+ // setup note target
+ form.attr("rel", dataHolder.data("discussionId"));
+ form.find("#note_commit_id").val(dataHolder.data("commitId"));
+ form.find("#note_line_code").val(dataHolder.data("lineCode"));
+ form.find("#note_noteable_type").val(dataHolder.data("noteableType"));
+ form.find("#note_noteable_id").val(dataHolder.data("noteableId"));
+
+ NoteList.setupNoteForm(form);
+
+ form.find(".js-note-text").focus();
+ },
+
+ /**
+ * Shows the main form and does some setup on it.
+ *
+ * Sets some hidden fields in the form.
+ */
+ setupMainTargetNoteForm: function() {
+ // find the form
+ var form = $(".js-new-note-form");
+ // insert the form after the button
+ form.clone().replaceAll($(".js-main-target-form"));
+
+ form = form.prev("form");
+
+ // show the form
+ NoteList.setupNoteForm(form);
+
+ // fix classes
+ form.removeClass("js-new-note-form");
+ form.addClass("js-main-target-form");
+
+ // remove unnecessary fields and buttons
+ form.find("#note_line_code").remove();
+ form.find(".js-close-discussion-note-form").remove();
+ },
+
+ /**
+ * General note form setup.
+ *
+ * * deactivates the submit button when text is empty
+ * * hides the preview button when text is empty
+ * * setup GFM auto complete
+ * * show the form
+ */
+ setupNoteForm: function(form) {
+ disableButtonIfEmptyField(form.find(".js-note-text"), form.find(".js-comment-button"));
+
+ form.removeClass("js-new-note-form");
+
+ // setup preview buttons
+ form.find(".js-note-edit-button, .js-note-preview-button")
+ .tooltip({ placement: 'left' });
+
+ previewButton = form.find(".js-note-preview-button");
+ form.find(".js-note-text").on("input", function() {
+ if ($(this).val().trim() !== "") {
+ previewButton.removeClass("turn-off").addClass("turn-on");
+ } else {
+ previewButton.removeClass("turn-on").addClass("turn-off");
+ }
+ });
+
+ // remove notify commit author checkbox for non-commit notes
+ if (form.find("#note_noteable_type").val() !== "Commit") {
+ form.find(".js-notify-commit-author").remove();
+ }
+
+ GitLab.GfmAutoComplete.setup();
+
+ form.show();
+ },
/**
@@ -86,40 +329,39 @@ var NoteList = {
/**
* Gets an inital set of notes.
*/
- getContent:
- function() {
- $.ajax({
- type: "GET",
- url: this.notes_path,
- data: this.target_params,
- complete: function(){ $('.notes-status').removeClass("loading")},
- beforeSend: function() { $('.notes-status').addClass("loading") },
- dataType: "script"});
- },
+ getContent: function() {
+ $.ajax({
+ url: NoteList.notes_path,
+ data: NoteList.target_params,
+ complete: function(){ $('.js-notes-busy').removeClass("loading")},
+ beforeSend: function() { $('.js-notes-busy').addClass("loading") },
+ dataType: "script"
+ });
+ },
/**
* Called in response to getContent().
* Replaces the content of #notes-list with the given html.
*/
- setContent:
- function(first_id, last_id, html) {
- this.top_id = first_id;
- this.bottom_id = last_id;
- $("#notes-list").html(html);
+ setContent: function(newNoteIds, html) {
+ NoteList.top_id = newNoteIds.first();
+ NoteList.bottom_id = newNoteIds.last();
+ $("#notes-list").html(html);
+ // for the wall
+ if (NoteList.reversed) {
// init infinite scrolling
- this.initLoadMore();
+ NoteList.initLoadMore();
// init getting new notes
- if (this.reversed) {
- this.initRefreshNew();
- }
- },
+ NoteList.initRefreshNew();
+ }
+ },
/**
* Handle loading more notes when scrolling to the bottom of the page.
- * The id of the last note in the list is in this.bottom_id.
+ * The id of the last note in the list is in NoteList.bottom_id.
*
* Set up refreshing only new notes after all notes have been loaded.
*/
@@ -128,65 +370,57 @@ var NoteList = {
/**
* Initializes loading more notes when scrolling to the bottom of the page.
*/
- initLoadMore:
- function() {
- $(document).endlessScroll({
- bottomPixels: 400,
- fireDelay: 1000,
- fireOnce:true,
- ceaseFire: function() {
- return NoteList.loading_more_disabled;
- },
- callback: function(i) {
- NoteList.getMore();
- }
- });
- },
+ initLoadMore: function() {
+ $(document).endlessScroll({
+ bottomPixels: 400,
+ fireDelay: 1000,
+ fireOnce:true,
+ ceaseFire: function() {
+ return NoteList.loading_more_disabled;
+ },
+ callback: function(i) {
+ NoteList.getMore();
+ }
+ });
+ },
/**
* Gets an additional set of notes.
*/
- getMore:
- function() {
- // only load more notes if there are no "new" notes
- $('.loading').show();
- $.ajax({
- type: "GET",
- url: this.notes_path,
- data: this.target_params + "&loading_more=1&" + (this.reversed ? "before_id" : "after_id") + "=" + this.bottom_id,
- complete: function(){ $('.notes-status').removeClass("loading")},
- beforeSend: function() { $('.notes-status').addClass("loading") },
- dataType: "script"});
- },
+ getMore: function() {
+ // only load more notes if there are no "new" notes
+ $('.loading').show();
+ $.ajax({
+ url: NoteList.notes_path,
+ data: NoteList.target_params + "&loading_more=1&" + (NoteList.reversed ? "before_id" : "after_id") + "=" + NoteList.bottom_id,
+ complete: function(){ $('.js-notes-busy').removeClass("loading")},
+ beforeSend: function() { $('.js-notes-busy').addClass("loading") },
+ dataType: "script"
+ });
+ },
/**
* Called in response to getMore().
* Append notes to #notes-list.
*/
- appendMoreNotes:
- function(id, html) {
- if(id != this.bottom_id) {
- this.bottom_id = id;
- $("#notes-list").append(html);
- }
- },
+ appendMoreNotes: function(newNoteIds, html) {
+ var lastNewNoteId = newNoteIds.last();
+ if(lastNewNoteId != NoteList.bottom_id) {
+ NoteList.bottom_id = lastNewNoteId;
+ $("#notes-list").append(html);
+ }
+ },
/**
* Called in response to getMore().
* Disables loading more notes when scrolling to the bottom of the page.
- * Initalizes refreshing new notes.
*/
- finishedLoadingMore:
- function() {
- this.loading_more_disabled = true;
+ finishedLoadingMore: function() {
+ NoteList.loading_more_disabled = true;
- // from now on only get new notes
- if (!this.reversed) {
- this.initRefreshNew();
- }
- // make sure we are up to date
- this.updateVotes();
- },
+ // make sure we are up to date
+ NoteList.updateVotes();
+ },
/**
@@ -194,52 +428,118 @@ var NoteList = {
*
* New notes are all notes that are created after the site has been loaded.
* The "old" notes are in #notes-list the "new" ones will be in #new-notes-list.
- * The id of the last "old" note is in this.bottom_id.
+ * The id of the last "old" note is in NoteList.bottom_id.
*/
/**
* Initializes getting new notes every n seconds.
+ *
+ * Note: only used on wall.
*/
- initRefreshNew:
- function() {
- setInterval("NoteList.getNew()", 10000);
- },
+ initRefreshNew: function() {
+ setInterval("NoteList.getNew()", 10000);
+ },
/**
* Gets the new set of notes.
+ *
+ * Note: only used on wall.
*/
- getNew:
- function() {
- $.ajax({
- type: "GET",
- url: this.notes_path,
- data: this.target_params + "&loading_new=1&after_id=" + (this.reversed ? this.top_id : this.bottom_id),
- dataType: "script"});
- },
+ getNew: function() {
+ $.ajax({
+ url: NoteList.notes_path,
+ data: NoteList.target_params + "&loading_new=1&after_id=" + (NoteList.reversed ? NoteList.top_id : NoteList.bottom_id),
+ dataType: "script"
+ });
+ },
/**
* Called in response to getNew().
* Replaces the content of #new-notes-list with the given html.
+ *
+ * Note: only used on wall.
*/
- replaceNewNotes:
- function(html) {
- $("#new-notes-list").html(html);
- this.updateVotes();
- },
+ replaceNewNotes: function(newNoteIds, html) {
+ $("#new-notes-list").html(html);
+ NoteList.updateVotes();
+ },
/**
- * Adds a single note to #new-notes-list.
+ * Adds a single common note to #notes-list.
*/
- appendNewNote:
- function(id, html) {
- if (this.reversed) {
- $("#new-notes-list").prepend(html);
- } else {
- $("#new-notes-list").append(html);
- }
- this.updateVotes();
- },
+ appendNewNote: function(id, html) {
+ $("#notes-list").append(html);
+ NoteList.updateVotes();
+ },
+
+ /**
+ * Adds a single discussion note to #notes-list.
+ *
+ * Also removes the corresponding form.
+ */
+ appendNewDiscussionNote: function(discussionId, diffRowHtml, noteHtml) {
+ var form = $("form[rel='"+discussionId+"']");
+ var row = form.closest("tr");
+
+ // is this the first note of discussion?
+ if (row.is(".js-temp-notes-holder")) {
+ // insert the note and the reply button after the temp row
+ row.after(diffRowHtml);
+ // remove the note (will be added again below)
+ row.next().find(".note").remove();
+ }
+
+ // append new note to all matching discussions
+ $(".notes[rel='"+discussionId+"']").append(noteHtml);
+
+ // cleanup after successfully creating a diff/discussion note
+ $.proxy(NoteList.removeDiscussionNoteForm, form).call();
+ },
+
+ /**
+ * Adds a single wall note to #new-notes-list.
+ *
+ * Note: only used on wall.
+ */
+ appendNewWallNote: function(id, html) {
+ $("#new-notes-list").prepend(html);
+ },
+
+ /**
+ * Called in response the main target form has been successfully submitted.
+ *
+ * Removes any errors.
+ * Resets text and preview.
+ * Resets buttons.
+ */
+ resetMainTargetForm: function(){
+ var form = $(this);
+
+ // remove validation errors
+ form.find(".js-errors").remove();
+
+ // reset text and preview
+ var previewContainer = form.find(".js-toggler-container.note_text_and_preview");
+ if (previewContainer.is(".on")) {
+ previewContainer.removeClass("on");
+ }
+ form.find(".js-note-text").val("").trigger("input");
+ },
+
+ /**
+ * Called after an attachment file has been selected.
+ *
+ * Updates the file name for the selected attachment.
+ */
+ updateFormAttachment: function() {
+ var form = $(this).closest("form");
+
+ // get only the basename
+ var filename = $(this).val().replace(/^.*[\\\/]/, '');
+
+ form.find(".js-attachment-filename").text(filename);
+ },
/**
* Recalculates the votes and updates them (if they are displayed at all).
@@ -249,67 +549,24 @@ var NoteList = {
* Might produce inaccurate results when not all notes have been loaded and a
* recalculation is triggered (e.g. when deleting a note).
*/
- updateVotes:
- function() {
- var votes = $("#votes .votes");
- var notes = $("#notes-list, #new-notes-list").find(".note .vote");
-
- // only update if there is a vote display
- if (votes.size()) {
- var upvotes = notes.filter(".upvote").size();
- var downvotes = notes.filter(".downvote").size();
- var votesCount = upvotes + downvotes;
- var upvotesPercent = votesCount ? (100.0 / votesCount * upvotes) : 0;
- var downvotesPercent = votesCount ? (100.0 - upvotesPercent) : 0;
-
- // change vote bar lengths
- votes.find(".bar-success").css("width", upvotesPercent+"%");
- votes.find(".bar-danger").css("width", downvotesPercent+"%");
- // replace vote numbers
- votes.find(".upvotes").text(votes.find(".upvotes").text().replace(/\d+/, upvotes));
- votes.find(".downvotes").text(votes.find(".downvotes").text().replace(/\d+/, downvotes));
- }
+ updateVotes: function() {
+ var votes = $("#votes .votes");
+ var notes = $("#notes-list .note .vote");
+
+ // only update if there is a vote display
+ if (votes.size()) {
+ var upvotes = notes.filter(".upvote").size();
+ var downvotes = notes.filter(".downvote").size();
+ var votesCount = upvotes + downvotes;
+ var upvotesPercent = votesCount ? (100.0 / votesCount * upvotes) : 0;
+ var downvotesPercent = votesCount ? (100.0 - upvotesPercent) : 0;
+
+ // change vote bar lengths
+ votes.find(".bar-success").css("width", upvotesPercent+"%");
+ votes.find(".bar-danger").css("width", downvotesPercent+"%");
+ // replace vote numbers
+ votes.find(".upvotes").text(votes.find(".upvotes").text().replace(/\d+/, upvotes));
+ votes.find(".downvotes").text(votes.find(".downvotes").text().replace(/\d+/, downvotes));
}
+ }
};
-
-var PerLineNotes = {
- init:
- function() {
- /**
- * Called when clicking on the "add note" or "reply" button for a diff line.
- *
- * Shows the note form below the line.
- * Sets some hidden fields in the form.
- */
- $(".diff_file_content").on("click", ".line_note_link, .line_note_reply_link", function(e) {
- var form = $(".per_line_form");
- $(this).closest("tr").after(form);
- form.find("#note_line_code").val($(this).data("lineCode"));
- form.show();
- e.preventDefault();
- });
-
- disableButtonIfEmptyField(".line-note-text", ".submit_inline_note");
-
- /**
- * Called in response to successfully deleting a note on a diff line.
- *
- * Removes the actual note from view.
- * Removes the reply button if the last note for that line has been removed.
- */
- $(".diff_file_content").on("ajax:success", ".delete-note", function() {
- var trNote = $(this).closest("tr");
- trNote.fadeOut(function() {
- $(this).remove();
- });
-
- // check if this is the last note for this line
- // elements must really be removed for this to work reliably
- var trLine = trNote.prev();
- var trRpl = trNote.next();
- if (trLine.is(".line_holder") && trRpl.is(".reply")) {
- trRpl.fadeOut(function() { $(this).remove(); });
- }
- });
- }
-}
diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss
index 0e951e7f68c..6b500b88823 100644
--- a/app/assets/stylesheets/application.scss
+++ b/app/assets/stylesheets/application.scss
@@ -46,3 +46,8 @@
@import "themes/ui_gray.scss";
@import "themes/ui_color.scss";
+/**
+ * Styles for JS behaviors.
+ */
+@import "behaviors.scss";
+
diff --git a/app/assets/stylesheets/behaviors.scss b/app/assets/stylesheets/behaviors.scss
new file mode 100644
index 00000000000..3fdc20485f2
--- /dev/null
+++ b/app/assets/stylesheets/behaviors.scss
@@ -0,0 +1,14 @@
+// Details
+//--------
+.js-details-container .content { display: none; }
+.js-details-container .content.hide { display: block; }
+.js-details-container.open .content { display: block; }
+.js-details-container.open .content.hide { display: none; }
+
+
+// Toggler
+//--------
+.js-toggler-container .turn-on { display: inherit; }
+.js-toggler-container .turn-off { display: none; }
+.js-toggler-container.on .turn-on { display: none; }
+.js-toggler-container.on .turn-off { display: inherit; }
diff --git a/app/assets/stylesheets/common.scss b/app/assets/stylesheets/common.scss
index f7a93436b3c..dd586de3a56 100644
--- a/app/assets/stylesheets/common.scss
+++ b/app/assets/stylesheets/common.scss
@@ -546,3 +546,9 @@ h1.http_status_code {
}
}
}
+
+img.emoji {
+ height: 20px;
+ vertical-align: middle;
+ width: 20px;
+}
diff --git a/app/assets/stylesheets/sections/commits.scss b/app/assets/stylesheets/sections/commits.scss
index ebec51c9715..c60aae49eaa 100644
--- a/app/assets/stylesheets/sections/commits.scss
+++ b/app/assets/stylesheets/sections/commits.scss
@@ -119,12 +119,14 @@
}
}
}
- .old_line, .new_line {
- margin: 0px;
- padding: 0px;
- border: none;
- background: #EEE;
- color: #666;
+ .new_line,
+ .old_line,
+ .notes_line {
+ margin:0px;
+ padding:0px;
+ border:none;
+ background:#EEE;
+ color:#666;
padding: 0px 5px;
border-right: 1px solid #ccc;
text-align: right;
@@ -134,6 +136,7 @@
moz-user-select: none;
-khtml-user-select: none;
user-select: none;
+
a {
float: left;
width: 35px;
diff --git a/app/assets/stylesheets/sections/notes.scss b/app/assets/stylesheets/sections/notes.scss
index 34c7391da18..3a1b650664f 100644
--- a/app/assets/stylesheets/sections/notes.scss
+++ b/app/assets/stylesheets/sections/notes.scss
@@ -1,233 +1,314 @@
/**
* Notes
- *
*/
-#notes-list,
-#new-notes-list {
+ul.notes {
display: block;
list-style: none;
margin: 0px;
padding: 0px;
-}
-.issue_notes,
-.wiki_notes {
- .note_content {
- float: left;
- width: 400px;
- }
-}
+ .discussion-header,
+ .note-header {
+ @extend .cgray;
+ padding-top: 5px;
+ padding-bottom: 15px;
-/* Note textare */
-#note_note {
- height: 80px;
- width: 98%;
- font-size: 14px;
-}
+ .avatar {
+ float: left;
+ margin-right: 10px;
+ }
-#new_note {
- .attach_holder {
- display: none;
+ .discussion-last-update,
+ .note-last-update {
+ font-style: italic;
+ }
+ .note-author {
+ color: $style_color;
+ font-weight: bold;
+ &:hover {
+ color: $primary_color;
+ }
+ }
}
-}
-.preview_note {
- margin: 2px;
- border: 1px solid #ddd;
- padding: 10px;
- min-height: 60px;
- background: #f5f5f5;
-}
+ .discussion {
+ padding: 8px 0;
+ overflow: hidden;
+ display: block;
+ position:relative;
-.note {
- padding: 8px 0;
- overflow: hidden;
- display: block;
- position: relative;
- img {float: left; margin-right: 10px;}
- img.emoji {float: none;margin: 0;}
- .note-author cite{font-style: italic;}
- p { color: $style_color; }
- .note-author { color: $style_color;}
+ .discussion-body {
+ margin-left: 50px;
- .note-title { margin-left: 45px; padding-top: 5px;}
- .avatar {
- margin-top: 3px;
- }
+ .diff_file,
+ .discussion-hidden,
+ .notes {
+ @extend .borders;
+ background-color: #F9F9F9;
+ }
+ .diff_file .notes {
+ /* reset */
+ background: inherit;
+ border: none;
+ @include box-shadow(none);
- .delete-note {
- display: none;
- position: absolute;
- right: 0;
- top: 0;
+ }
+ .discussion-hidden .note {
+ @extend .cgray;
+ padding: 8px;
+ text-align: center;
+ }
+ .notes .note {
+ border-color: #ddd;
+ padding: 8px;
+ }
+ .reply-btn {
+ margin-top: 8px;
+ }
+ }
}
- &:hover {
- .delete-note { display: block; }
- }
-}
-#notes-list:not(.reversed) .note,
-#new-notes-list:not(.reversed) .note {
- border-bottom: 1px solid #eee;
-}
-#notes-list.reversed .note,
-#new-notes-list.reversed .note {
- border-top: 1px solid #eee;
-}
-
-/* mark vote notes */
-.voting_notes .note {
- padding: 8px 0;
-}
+ .note {
+ padding: 8px 0;
+ overflow: hidden;
+ display: block;
+ position:relative;
+ p { color: $style_color; }
-.notes-status {
- margin: 18px;
-}
+ .avatar {
+ margin-top: 3px;
+ }
+ .attachment {
+ font-size: 14px;
+ margin-top: -20px;
+ .icon-attachment {
+ @extend .icon-paper-clip;
+ font-size: 24px;
+ position: relative;
+ text-align: right;
+ top: 6px;
+ }
+ }
+ .note-body {
+ margin-left: 45px;
+ }
+ .note-header {
+ padding-bottom: 5px;
+ }
+ }
-p.notify_controls input{
- margin: 5px;
+ // paint top or bottom borders depending on notes direction
+ &:not(.reversed) .note,
+ &:not(.reversed) .discussion {
+ border-bottom: 1px solid #eee;
+ }
+ &.reversed .note,
+ &.reversed .discussion {
+ border-top: 1px solid #eee;
+ }
}
-p.notify_controls span{
- font-weight: 700;
-}
+.diff_file .notes_holder {
+ font-family: $sansFontFamily;
+ font-size: 13px;
+ line-height: 18px;
-tr.line_notes_row {
- border-bottom: 1px solid #DDD;
- border-left: 7px solid #2A79A3;
+ td {
+ border: 1px solid #ddd;
+ border-left: none;
- &.reply {
- background: #eee;
- border-left: 7px solid #2A79A3;
- border-top: 1px solid #ddd;
- td {
- padding: 7px 10px;
+ &.notes_line {
+ text-align: center;
+ padding: 10px 0;
}
- a.line_note_reply_link {
- border: 1px solid #eaeaea;
- @include border-radius(4px);
- padding: 3px 10px;
- margin-left: 5px;
- color: white;
- background: #2A79A3;
- border-color: #2A79A3;
+ &.notes_content {
+ background-color: $white;
+ border-width: 1px 0;
+ padding-top: 0;
}
}
- ul {
- margin: 0;
- li {
- padding: 0;
- border: none;
- }
+
+ .reply-btn {
+ margin-top: 8px;
}
}
-.line_notes_row, .per_line_form { font-family: "Helvetica Neue", Helvetica, Arial, sans-serif; }
-.per_line_form {
- background: #f5f5f5;
- border-top: 1px solid #eee;
- form { margin: 0; }
- td {
- border-bottom: 1px solid #ddd;
+
+/**
+ * Actions for Discussions/Notes
+ */
+
+.discussion,
+.note {
+ &.note:hover {
+ .note-actions { display: block; }
+ }
+ .discussion-header:hover {
+ .discussion-actions { display: block; }
}
- .note_actions {
- margin: 0;
- padding-top: 10px;
- .buttons {
- float: left;
- width: 300px;
- }
- .options {
- .labels {
- float: left;
- padding-left: 10px;
- label {
- padding: 6px 0;
- margin: 0;
- width: 120px;
- }
+ .discussion-actions,
+ .note-actions {
+ display: none;
+ float: right;
+
+ [class^="icon-"],
+ [class*="icon-"] {
+ font-size: 16px;
+ line-height: 16px;
+ vertical-align: middle;
+ }
+
+ a {
+ @extend .cgray;
+
+ &:hover {
+ color: $primary_color;
+ &.danger { @extend .cred; }
}
}
}
}
+.diff_file .note .note-actions {
+ right: 0;
+ top: 0;
+}
+
+
+
+/**
+ * Line note button on the side of diffs
+ */
-td .line_note_link {
- position: absolute;
- margin-left:-70px;
- margin-top:-10px;
- z-index: 10;
- background: url("comment_add.png") no-repeat left 0;
- width: 32px;
- height: 32px;
+.diff_file tr.line_holder {
+ .add-diff-note {
+ background: url("diff_note_add.png") no-repeat left 0;
+ height: 22px;
+ margin-left: -65px;
+ position: absolute;
+ width: 22px;
+ z-index: 10;
- opacity: 0.0;
- filter: alpha(opacity=0);
+ // "hide" it by default
+ opacity: 0.0;
+ filter: alpha(opacity=0);
- &:hover {
- opacity: 1.0;
- filter: alpha(opacity=100);
+ &:hover {
+ opacity: 1.0;
+ filter: alpha(opacity=100);
+ }
+ }
+
+ // "show" the icon also if we just hover somwhere over the line
+ &:hover > td {
+ background: $hover !important;
+
+ .add-diff-note {
+ opacity: 1.0;
+ filter: alpha(opacity=100);
+ }
}
}
-.diff_file_content tr.line_holder:hover > td { background: $hover !important; }
-.diff_file_content tr.line_holder:hover > td .line_note_link {
- opacity: 1.0;
- filter: alpha(opacity=100);
+
+
+/**
+ * Note Form
+ */
+
+.comment-btn,
+.reply-btn {
+ @extend .save-btn;
}
+.diff_file,
+.discussion {
+ .new_note {
+ margin: 8px 5px 8px 0;
-.new_note {
- .input-file {
- font: 500px monospace;
- opacity: 0;
- filter: alpha(opacity=0);
- position: absolute;
- z-index: 1;
- top: 0;
- right: 0;
- padding: 0;
- margin: 0;
+ .note_options {
+ // because of the smaller width and the extra "cancel" button
+ margin-top: 8px;
+ }
}
+}
+.new_note {
+ display: none;
- .note_advanced_opts {
+ .buttons {
+ float: left;
+ margin-top: 8px;
+ }
+ .clearfix {
+ margin-bottom: 0;
+ }
+ .note_options {
h6 {
- line-height: 32px;
- padding-right: 15px;
+ @extend .left;
+ line-height: 20px;
+ padding-right: 16px;
+ padding-bottom: 16px;
+ }
+ label {
+ padding: 0;
}
- }
- .attachments {
- position: relative;
- width: 350px;
- height: 50px;
- overflow: hidden;
- margin:0 0 5px !important;
+ .attachment {
+ @extend .right;
+ position: relative;
+ width: 350px;
+ height: 50px;
+ margin:0 0 5px !important;
- .input_file {
- .file_upload {
- position: absolute;
- right: 14px;
- top: 7px;
+ // hide the actual file field
+ input {
+ display: none;
}
- .file_name {
- line-height: 30px;
- width: 240px;
- height: 28px;
- overflow: hidden;
- }
- .input-file {
- width: 260px;
- height: 41px;
+ .choose-btn {
float: right;
}
}
+ .notify_options {
+ @extend .right;
+ }
}
+ .note_text_and_preview {
+ // makes the "absolute" position for links relative to this
+ position: relative;
+
+ // preview/edit buttons
+ > a {
+ font-size: 24px;
+ padding: 4px;
+ position: absolute;
+ right: 10px;
+ }
+ .note_preview {
+ background: #f5f5f5;
+ border: 1px solid #ddd;
+ @include border-radius(4px);
+ min-height: 80px;
+ padding: 4px 6px;
+ }
+ .note_text {
+ border: 1px solid #DDD;
+ box-shadow: none;
+ font-size: 14px;
+ height: 80px;
+ width: 98.6%;
+ }
+ }
+}
+
+/* loading indicator */
+.notes-busy {
+ margin: 18px;
}
-.note-text {
- border: 1px solid #DDD;
- box-shadow: none;
+.note-image-attach {
+ @extend .span4;
+ @extend .thumbnail;
+ margin-left: 45px;
}
diff --git a/app/contexts/notes/create_context.rb b/app/contexts/notes/create_context.rb
index d93adb835ef..1367dff4699 100644
--- a/app/contexts/notes/create_context.rb
+++ b/app/contexts/notes/create_context.rb
@@ -3,8 +3,8 @@ module Notes
def execute
note = project.notes.new(params[:note])
note.author = current_user
- note.notify = true if params[:notify] == '1'
- note.notify_author = true if params[:notify_author] == '1'
+ note.notify = params[:notify].present?
+ note.notify_author = params[:notify_author].present?
note.save
note
end
diff --git a/app/contexts/notes/load_context.rb b/app/contexts/notes/load_context.rb
index a8f617a782b..e3875e1d4e2 100644
--- a/app/contexts/notes/load_context.rb
+++ b/app/contexts/notes/load_context.rb
@@ -9,11 +9,11 @@ module Notes
@notes = case target_type
when "commit"
- project.notes.for_commit_id(target_id).not_inline.fresh.limit(20)
+ project.notes.for_commit_id(target_id).not_inline.fresh
when "issue"
- project.issues.find(target_id).notes.inc_author.fresh.limit(20)
+ project.issues.find(target_id).notes.inc_author.fresh
when "merge_request"
- project.merge_requests.find(target_id).mr_and_commit_notes.inc_author.fresh.limit(20)
+ project.merge_requests.find(target_id).mr_and_commit_notes.inc_author.fresh
when "snippet"
project.snippets.find(target_id).notes.fresh
when "wall"
diff --git a/app/controllers/commit_controller.rb b/app/controllers/commit_controller.rb
index d671e9f9e9e..1329410891d 100644
--- a/app/controllers/commit_controller.rb
+++ b/app/controllers/commit_controller.rb
@@ -13,11 +13,17 @@ class CommitController < ProjectResourceController
@commit = result[:commit]
git_not_found! unless @commit
- @suppress_diff = result[:suppress_diff]
- @note = result[:note]
- @line_notes = result[:line_notes]
- @notes_count = result[:notes_count]
- @comments_allowed = true
+ @suppress_diff = result[:suppress_diff]
+
+ @note = result[:note]
+ @line_notes = result[:line_notes]
+ @notes_count = result[:notes_count]
+ @target_type = :commit
+ @target_id = @commit.id
+
+ @comments_allowed = @reply_allowed = true
+ @comments_target = { noteable_type: 'Commit',
+ commit_id: @commit.id }
respond_to do |format|
format.html do
diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index 5a1ce2cfcc4..9917d198cbf 100644
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -35,6 +35,8 @@ class IssuesController < ProjectResourceController
def show
@note = @project.notes.new(noteable: @issue)
+ @target_type = :issue
+ @target_id = @issue.id
respond_to do |format|
format.html
diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb
index 6ead406aac5..ab6bf595982 100644
--- a/app/controllers/merge_requests_controller.rb
+++ b/app/controllers/merge_requests_controller.rb
@@ -18,6 +18,9 @@ class MergeRequestsController < ProjectResourceController
end
def show
+ @target_type = :merge_request
+ @target_id = @merge_request.id
+
respond_to do |format|
format.html
format.js
@@ -31,7 +34,9 @@ class MergeRequestsController < ProjectResourceController
@diffs = @merge_request.diffs
@commit = @merge_request.last_commit
- @comments_allowed = true
+ @comments_allowed = @reply_allowed = true
+ @comments_target = { noteable_type: 'MergeRequest',
+ noteable_id: @merge_request.id }
@line_notes = @merge_request.notes.where("line_code is not null")
end
diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb
index d794f368f57..000c7bbb641 100644
--- a/app/controllers/notes_controller.rb
+++ b/app/controllers/notes_controller.rb
@@ -6,10 +6,12 @@ class NotesController < ProjectResourceController
respond_to :js
def index
- notes
+ @notes = Notes::LoadContext.new(project, current_user, params).execute
+ @target_type = params[:target_type].camelize
+ @target_id = params[:target_id]
+
if params[:target_type] == "merge_request"
- @mixed_targets = true
- @main_target_type = params[:target_type].camelize
+ @discussions = discussions_from_notes
end
respond_with(@notes)
@@ -17,6 +19,8 @@ class NotesController < ProjectResourceController
def create
@note = Notes::CreateContext.new(project, current_user, params).execute
+ @target_type = params[:target_type].camelize
+ @target_id = params[:target_id]
respond_to do |format|
format.html {redirect_to :back}
@@ -40,7 +44,34 @@ class NotesController < ProjectResourceController
protected
- def notes
- @notes = Notes::LoadContext.new(project, current_user, params).execute
+ def discussion_notes_for(note)
+ @notes.select do |other_note|
+ note.discussion_id == other_note.discussion_id
+ end
+ end
+
+ def discussions_from_notes
+ discussion_ids = []
+ discussions = []
+
+ @notes.each do |note|
+ next if discussion_ids.include?(note.discussion_id)
+
+ # don't group notes for the main target
+ if note_for_main_target?(note)
+ discussions << [note]
+ else
+ discussions << discussion_notes_for(note)
+ discussion_ids << note.discussion_id
+ end
+ end
+
+ discussions
+ end
+
+ # Helps to distinguish e.g. commit notes in mr notes list
+ def note_for_main_target?(note)
+ note.for_wall? ||
+ (@target_type.camelize == note.noteable_type && !note.for_diff_line?)
end
end
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 1a9c890ca80..6f1180eaa6b 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -80,7 +80,10 @@ class ProjectsController < ProjectResourceController
def wall
return render_404 unless @project.wall_enabled
- @note = Note.new
+
+ @target_type = :wall
+ @target_id = nil
+ @note = @project.notes.new
respond_to do |format|
format.html
diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb
index 119ef9b2be3..26898abfa82 100644
--- a/app/controllers/snippets_controller.rb
+++ b/app/controllers/snippets_controller.rb
@@ -50,6 +50,8 @@ class SnippetsController < ProjectResourceController
def show
@note = @project.notes.new(noteable: @snippet)
+ @target_type = :snippet
+ @target_id = @snippet.id
end
def destroy
diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb
index 8fc637a2bf6..4b5d8bd96a6 100644
--- a/app/helpers/commits_helper.rb
+++ b/app/helpers/commits_helper.rb
@@ -9,11 +9,13 @@ module CommitsHelper
end
end
- def build_line_anchor(index, line_new, line_old)
- "#{index}_#{line_old}_#{line_new}"
+ def build_line_anchor(diff, line_new, line_old)
+ "#{hexdigest(diff.new_path)}_#{line_old}_#{line_new}"
end
- def each_diff_line(diff_arr, index)
+ def each_diff_line(diff, index)
+ diff_arr = diff.diff.lines.to_a
+
line_old = 1
line_new = 1
type = nil
@@ -39,7 +41,7 @@ module CommitsHelper
next
else
type = identification_type(line)
- line_code = build_line_anchor(index, line_new, line_old)
+ line_code = build_line_anchor(diff, line_new, line_old)
yield(full_line, type, line_code, line_new, line_old)
end
diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index ffcc7acc8da..7a0ed251aa8 100644
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@@ -1,24 +1,32 @@
module NotesHelper
- def loading_more_notes?
- params[:loading_more].present?
+ # Helps to distinguish e.g. commit notes in mr notes list
+ def note_for_main_target?(note)
+ note.for_wall? ||
+ (@target_type.camelize == note.noteable_type && !note.for_diff_line?)
end
- def loading_new_notes?
- params[:loading_new].present?
+ def note_target_fields
+ hidden_field_tag(:target_type, @target_type) +
+ hidden_field_tag(:target_id, @target_id)
end
- # Helps to distinguish e.g. commit notes in mr notes list
- def note_for_main_target?(note)
- !@mixed_targets || @main_target_type == note.noteable_type
+ def link_to_commit_diff_line_note(note)
+ if note.for_commit_diff_line?
+ link_to "#{note.diff_file_name}:L#{note.diff_new_line}", project_commit_path(@project, note.noteable, anchor: note.line_code)
+ end
end
- def link_to_commit_diff_line_note(note)
- commit = note.noteable
- diff_index, diff_old_line, diff_new_line = note.line_code.split('_')
+ def link_to_merge_request_diff_line_note(note)
+ if note.for_merge_request_diff_line? and note.diff
+ link_to "#{note.diff_file_name}:L#{note.diff_new_line}", diffs_project_merge_request_path(note.project, note.noteable_id, anchor: note.line_code)
+ end
+ end
- link_file = commit.diffs[diff_index.to_i].new_path
- link_line = diff_new_line
+ def loading_more_notes?
+ params[:loading_more].present?
+ end
- link_to "#{link_file}:L#{link_line}", project_commit_path(@project, commit, anchor: note.line_code)
+ def loading_new_notes?
+ params[:loading_new].present?
end
end
diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb
index c672940a05e..706d7a8e4ed 100644
--- a/app/mailers/notify.rb
+++ b/app/mailers/notify.rb
@@ -63,12 +63,12 @@ class Notify < ActionMailer::Base
# Note
#
- def note_commit_email(recipient_id, note_id)
+ def note_commit_email(commit_autor_email, note_id)
@note = Note.find(note_id)
@commit = @note.noteable
@commit = CommitDecorator.decorate(@commit)
@project = @note.project
- mail(to: recipient(recipient_id), subject: subject("note for commit #{@commit.short_id}", @commit.title))
+ mail(to: recipient(commit_autor_email), subject: subject("note for commit #{@commit.short_id}", @commit.title))
end
def note_issue_email(recipient_id, note_id)
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index f9dd74f9cab..d1717d3bbee 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -69,29 +69,33 @@ module Issuable
closed_changed? && !closed
end
- # Return the number of +1 comments (upvotes)
- def upvotes
- notes.select(&:upvote?).size
+ #
+ # Votes
+ #
+
+ # Return the number of -1 comments (downvotes)
+ def downvotes
+ notes.select(&:downvote?).size
end
- def upvotes_in_percent
+ def downvotes_in_percent
if votes_count.zero?
0
else
- 100.0 / votes_count * upvotes
+ 100.0 - upvotes_in_percent
end
end
- # Return the number of -1 comments (downvotes)
- def downvotes
- notes.select(&:downvote?).size
+ # Return the number of +1 comments (upvotes)
+ def upvotes
+ notes.select(&:upvote?).size
end
- def downvotes_in_percent
+ def upvotes_in_percent
if votes_count.zero?
0
else
- 100.0 - upvotes_in_percent
+ 100.0 / votes_count * upvotes
end
end
diff --git a/app/models/note.rb b/app/models/note.rb
index b055ae623b2..ded126b4bf1 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -19,7 +19,6 @@ require 'carrierwave/orm/activerecord'
require 'file_size_validator'
class Note < ActiveRecord::Base
-
attr_accessible :note, :noteable, :noteable_id, :noteable_type, :project_id,
:attachment, :line_code, :commit_id
@@ -34,6 +33,7 @@ class Note < ActiveRecord::Base
delegate :name, :email, to: :author, prefix: true
validates :note, :project, presence: true
+ validates :line_code, format: { with: /\A[a-z0-9]+_\d+_\d+\Z/ }, allow_blank: true
validates :attachment, file_size: { maximum: 10.megabytes.to_i }
validates :noteable_id, presence: true, if: ->(n) { n.noteable_type.present? && n.noteable_type != 'Commit' }
@@ -60,12 +60,74 @@ class Note < ActiveRecord::Base
}, without_protection: true)
end
- def notify
- @notify ||= false
+ def commit_author
+ @commit_author ||=
+ project.users.find_by_email(noteable.author_email) ||
+ project.users.find_by_name(noteable.author_name)
+ rescue
+ nil
end
- def notify_author
- @notify_author ||= false
+ def diff
+ if noteable.diffs.present?
+ noteable.diffs.select do |d|
+ if d.b_path
+ Digest::SHA1.hexdigest(d.b_path) == diff_file_index
+ end
+ end.first
+ end
+ end
+
+ def diff_file_index
+ line_code.split('_')[0]
+ end
+
+ def diff_file_name
+ diff.b_path
+ end
+
+ def diff_new_line
+ line_code.split('_')[2].to_i
+ end
+
+ def discussion_id
+ @discussion_id ||= [:discussion, noteable_type.try(:underscore), noteable_id, line_code].join("-").to_sym
+ end
+
+ # Returns true if this is a downvote note,
+ # otherwise false is returned
+ def downvote?
+ votable? && (note.start_with?('-1') ||
+ note.start_with?(':-1:')
+ )
+ end
+
+ def for_commit?
+ noteable_type == "Commit"
+ end
+
+ def for_commit_diff_line?
+ for_commit? && for_diff_line?
+ end
+
+ def for_diff_line?
+ line_code.present?
+ end
+
+ def for_issue?
+ noteable_type == "Issue"
+ end
+
+ def for_merge_request?
+ noteable_type == "MergeRequest"
+ end
+
+ def for_merge_request_diff_line?
+ for_merge_request? && for_diff_line?
+ end
+
+ def for_wall?
+ noteable_type.blank?
end
# override to return commits, which are not active record
@@ -81,50 +143,24 @@ class Note < ActiveRecord::Base
nil
end
- # Check if we can notify commit author
- # with email about our comment
- #
- # If commit author email exist in project
- # and commit author is not passed user we can
- # send email to him
- #
- # params:
- # user - current user
- #
- # return:
- # Boolean
- #
- def notify_only_author?(user)
- for_commit? && commit_author &&
- commit_author.email != user.email
- end
-
- def for_commit?
- noteable_type == "Commit"
- end
-
- def for_diff_line?
- line_code.present?
+ def notify
+ @notify ||= false
end
- def commit_author
- @commit_author ||=
- project.users.find_by_email(noteable.author_email) ||
- project.users.find_by_name(noteable.author_name)
- rescue
- nil
+ def notify_author
+ @notify_author ||= false
end
# Returns true if this is an upvote note,
# otherwise false is returned
def upvote?
- note.start_with?('+1') || note.start_with?(':+1:')
+ votable? && (note.start_with?('+1') ||
+ note.start_with?(':+1:')
+ )
end
- # Returns true if this is a downvote note,
- # otherwise false is returned
- def downvote?
- note.start_with?('-1') || note.start_with?(':-1:')
+ def votable?
+ for_issue? || (for_merge_request? && !for_diff_line?)
end
def noteable_type_name
diff --git a/app/observers/note_observer.rb b/app/observers/note_observer.rb
index 3f6d1dfcb70..2ec644ef7c1 100644
--- a/app/observers/note_observer.rb
+++ b/app/observers/note_observer.rb
@@ -11,7 +11,7 @@ class NoteObserver < ActiveRecord::Observer
notify_team(note)
elsif note.notify_author
# Notify only author of resource
- Notify.delay.note_commit_email(note.commit_author.id, note.id)
+ Notify.delay.note_commit_email(note.noteable.author_email, note.id)
else
# Otherwise ignore it
nil
diff --git a/app/views/commit/show.html.haml b/app/views/commit/show.html.haml
index 0144e4754c5..f920534e03a 100644
--- a/app/views/commit/show.html.haml
+++ b/app/views/commit/show.html.haml
@@ -7,12 +7,10 @@
%span.cred #{@commit.stats.deletions} deletions
= render "commits/diffs", diffs: @commit.diffs
-= render "notes/notes_with_form", tid: @commit.id, tt: "commit"
-= render "notes/per_line_form"
+= render "notes/notes_with_form"
:javascript
$(function(){
- PerLineNotes.init();
var w, h;
$('.diff_file').each(function(){
$('.image.diff_removed img', this).on('load', $.proxy(function(event){
diff --git a/app/views/commits/_diffs.html.haml b/app/views/commits/_diffs.html.haml
index e7733f0506b..7fe45aa25bc 100644
--- a/app/views/commits/_diffs.html.haml
+++ b/app/views/commits/_diffs.html.haml
@@ -38,10 +38,10 @@
%br/
.diff_file_content
- -# Skipp all non non-supported blobs
+ -# Skip all non-supported blobs
- next unless file.respond_to?('text?')
- if file.text?
- = render "commits/text_file", diff: diff, index: i
+ = render "commits/text_diff", diff: diff, index: i
- elsif file.image?
- old_file = (@commit.prev_commit.tree / diff.old_path) if !@commit.prev_commit.nil?
- if diff.renamed_file || diff.new_file || diff.deleted_file
diff --git a/app/views/commits/_text_file.html.haml b/app/views/commits/_text_diff.html.haml
index ecdae2f3715..8afad96bde2 100644
--- a/app/views/commits/_text_file.html.haml
+++ b/app/views/commits/_text_diff.html.haml
@@ -3,7 +3,7 @@
%a.supp_diff_link Diff suppressed. Click to show
%table{class: "#{'hide' if too_big}"}
- - each_diff_line(diff.diff.lines.to_a, index) do |line, type, line_code, line_new, line_old|
+ - each_diff_line(diff, index) do |line, type, line_code, line_new, line_old|
%tr.line_holder{ id: line_code }
- if type == "match"
%td.old_line= "..."
@@ -13,11 +13,11 @@
%td.old_line
= link_to raw(type == "new" ? "&nbsp;" : line_old), "##{line_code}", id: line_code
- if @comments_allowed
- = render "notes/per_line_note_link", line_code: line_code
+ = render "notes/diff_note_link", line_code: line_code
%td.new_line= link_to raw(type == "old" ? "&nbsp;" : line_new) , "##{line_code}", id: line_code
%td.line_content{class: "noteable_line #{type} #{line_code}", "line_code" => line_code}= raw diff_line_content(line)
- - if @comments_allowed
- - comments = @line_notes.select { |n| n.line_code == line_code }.sort_by(&:created_at)
- - unless comments.empty?
- = render "notes/per_line_notes_with_reply", notes: comments
+ - if @reply_allowed
+ - comments = @line_notes.select { |n| n.line_code == line_code }.sort_by(&:created_at)
+ - unless comments.empty?
+ = render "notes/diff_notes_with_reply", notes: comments
diff --git a/app/views/issues/show.html.haml b/app/views/issues/show.html.haml
index 10f647de3d2..6bf78929699 100644
--- a/app/views/issues/show.html.haml
+++ b/app/views/issues/show.html.haml
@@ -56,4 +56,4 @@
= markdown @issue.description
-.issue_notes.voting_notes#notes= render "notes/notes_with_form", tid: @issue.id, tt: "issue"
+.voting_notes#notes= render "notes/notes_with_form"
diff --git a/app/views/merge_requests/_show.html.haml b/app/views/merge_requests/_show.html.haml
index b65d1596f53..cefd33c0cdf 100644
--- a/app/views/merge_requests/_show.html.haml
+++ b/app/views/merge_requests/_show.html.haml
@@ -12,20 +12,18 @@
%li.notes-tab{data: {action: 'notes'}}
= link_to project_merge_request_path(@project, @merge_request) do
%i.icon-comment
- Comments
+ Discussion
%li.diffs-tab{data: {action: 'diffs'}}
= link_to diffs_project_merge_request_path(@project, @merge_request) do
%i.icon-list-alt
Diff
.notes.tab-content.voting_notes#notes{ class: (controller.action_name == 'show') ? "" : "hide" }
- = render("notes/notes_with_form", tid: @merge_request.id, tt: "merge_request")
+ = render "notes/notes_with_form"
.diffs.tab-content
= render "merge_requests/show/diffs" if @diffs
.status
- = render "notes/per_line_form"
-
:javascript
var merge_request;
$(function(){
diff --git a/app/views/merge_requests/diffs.html.haml b/app/views/merge_requests/diffs.html.haml
index a755491c42e..2a5b8b1441e 100644
--- a/app/views/merge_requests/diffs.html.haml
+++ b/app/views/merge_requests/diffs.html.haml
@@ -1,6 +1 @@
= render "show"
-
-:javascript
- $(function(){
- PerLineNotes.init();
- });
diff --git a/app/views/merge_requests/diffs.js.haml b/app/views/merge_requests/diffs.js.haml
index 1d92f1a6fb8..266892c01ef 100644
--- a/app/views/merge_requests/diffs.js.haml
+++ b/app/views/merge_requests/diffs.js.haml
@@ -1,5 +1,2 @@
:plain
merge_request.$(".diffs").html("#{escape_javascript(render(partial: "merge_requests/show/diffs"))}");
-
- PerLineNotes.init();
-
diff --git a/app/views/merge_requests/show.js.haml b/app/views/merge_requests/show.js.haml
index a2a79307453..2ce6eb63290 100644
--- a/app/views/merge_requests/show.js.haml
+++ b/app/views/merge_requests/show.js.haml
@@ -1,2 +1,2 @@
:plain
- merge_request.$(".notes").html("#{escape_javascript(render "notes/notes_with_form", tid: @merge_request.id, tt: "merge_request")}");
+ merge_request.$(".notes").html("#{escape_javascript(render "notes/notes_with_form")}");
diff --git a/app/views/notes/_common_form.html.haml b/app/views/notes/_common_form.html.haml
deleted file mode 100644
index d76be75bc27..00000000000
--- a/app/views/notes/_common_form.html.haml
+++ /dev/null
@@ -1,39 +0,0 @@
-.note-form-holder
- = form_for [@project, @note], remote: "true", multipart: true do |f|
- %h3.page_title Leave a comment
- -if @note.errors.any?
- .alert-message.block-message.error
- - @note.errors.full_messages.each do |msg|
- %div= msg
-
- = f.hidden_field :noteable_id
- = f.hidden_field :commit_id
- = f.hidden_field :noteable_type
- = f.text_area :note, size: 255, class: 'note-text js-gfm-input'
- #preview-note.preview_note.hide
- .hint
- .right Comments are parsed with #{link_to "GitLab Flavored Markdown", help_markdown_path, target: '_blank'}.
- .clearfix
-
- .row.note_advanced_opts
- .span3
- = f.submit 'Add Comment', class: "btn success submit_note grouped", id: "submit_note"
- = link_to 'Preview', preview_project_notes_path(@project), class: 'btn grouped', id: 'preview-link'
- .span4.notify_opts
- %h6.left Notify via email:
- = label_tag :notify do
- = check_box_tag :notify, 1, @note.noteable_type != "Commit"
- %span Project team
-
- - if @note.notify_only_author?(current_user)
- = label_tag :notify_author do
- = check_box_tag :notify_author, 1 , @note.noteable_type == "Commit"
- %span Commit author
- .span5.attachments
- %h6.left Attachment:
- %span.file_name File name...
-
- .input.input_file
- %a.file_upload.btn.small Upload File
- = f.file_field :attachment, class: "input-file"
- %span.hint Any file less than 10 MB
diff --git a/app/views/notes/_create_common_note.js.haml b/app/views/notes/_create_common_note.js.haml
deleted file mode 100644
index 217c908064a..00000000000
--- a/app/views/notes/_create_common_note.js.haml
+++ /dev/null
@@ -1,13 +0,0 @@
-- if note.valid?
- :plain
- $(".note-form-holder .error").remove();
- $('.note-form-holder textarea').val("");
- $('.note-form-holder #preview-link').text('Preview');
- $('.note-form-holder #preview-note').hide();
- $('.note-form-holder').show();
- NoteList.appendNewNote(#{note.id}, "#{escape_javascript(render "notes/note", note: note)}");
-
-- else
- :plain
- $(".note-form-holder").replaceWith("#{escape_javascript(render 'notes/common_form')}");
- GitLab.GfmAutoComplete.setup();
diff --git a/app/views/notes/_create_per_line_note.js.haml b/app/views/notes/_create_per_line_note.js.haml
deleted file mode 100644
index 180960e38e4..00000000000
--- a/app/views/notes/_create_per_line_note.js.haml
+++ /dev/null
@@ -1,19 +0,0 @@
-- if note.valid?
- :plain
- // hide and reset the form
- $(".per_line_form").hide();
- $('.line-note-form-holder textarea').val("");
-
- // find the reply button for this line
- // (might not be there if this is the first note)
- var trRpl = $("a.line_note_reply_link[data-line-code='#{note.line_code}']").closest("tr");
- if (trRpl.size() == 0) {
- // find the commented line ...
- var trEl = $(".#{note.line_code}").parent();
- // ... and insert the note and the reply button after it
- trEl.after("#{escape_javascript(render "notes/per_line_reply_button", line_code: note.line_code)}");
- trEl.after("#{escape_javascript(render "notes/per_line_note", note: note)}");
- } else {
- // instert new note before reply button
- trRpl.before("#{escape_javascript(render "notes/per_line_note", note: note)}");
- }
diff --git a/app/views/notes/_diff_note_link.html.haml b/app/views/notes/_diff_note_link.html.haml
new file mode 100644
index 00000000000..377c926a204
--- /dev/null
+++ b/app/views/notes/_diff_note_link.html.haml
@@ -0,0 +1,10 @@
+- note = @project.notes.new(@comments_target.merge({ line_code: line_code }))
+= link_to "",
+ "javascript:;",
+ class: "add-diff-note js-add-diff-note-button",
+ data: { noteable_type: note.noteable_type,
+ noteable_id: note.noteable_id,
+ commit_id: note.commit_id,
+ line_code: note.line_code,
+ discussion_id: note.discussion_id },
+ title: "Add a comment to this line"
diff --git a/app/views/notes/_diff_notes_with_reply.html.haml b/app/views/notes/_diff_notes_with_reply.html.haml
new file mode 100644
index 00000000000..0808f86b090
--- /dev/null
+++ b/app/views/notes/_diff_notes_with_reply.html.haml
@@ -0,0 +1,11 @@
+- note = notes.first # example note
+%tr.notes_holder
+ %td.notes_line{ colspan: 2 }
+ %span.btn.disabled
+ %i.icon-comment
+ = notes.count
+ %td.notes_content
+ %ul.notes{ rel: note.discussion_id }
+ = render notes
+
+ = render "notes/discussion_reply_button", note: note
diff --git a/app/views/notes/_discussion.html.haml b/app/views/notes/_discussion.html.haml
new file mode 100644
index 00000000000..093775f0d88
--- /dev/null
+++ b/app/views/notes/_discussion.html.haml
@@ -0,0 +1,63 @@
+- note = discussion_notes.first
+.discussion.js-details-container.js-toggler-container.open{ class: note.discussion_id }
+ .discussion-header
+ .discussion-actions
+ = link_to "javascript:;", class: "js-details-target turn-on js-toggler-target" do
+ %i.icon-eye-close
+ Hide discussion
+ = link_to "javascript:;", class: "js-details-target turn-off js-toggler-target" do
+ %i.icon-eye-open
+ Show discussion
+ = image_tag gravatar_icon(note.author.email), class: "avatar s32"
+ %div
+ = link_to note.author_name, project_team_member_path(@project, @project.team_member_by_id(note.author)), class: "note-author"
+ - if note.for_merge_request?
+ - if note.diff
+ started a discussion on this merge request diff
+ = link_to_merge_request_diff_line_note(note)
+ - else
+ started
+ %strong
+ %i.icon-remove
+ outdated
+ discussion on this merge request diff
+ - elsif note.for_commit?
+ started a discussion on commit
+ #{link_to note.noteable.short_id, project_commit_path(@project, note.noteable)}
+ = link_to_commit_diff_line_note(note) if note.for_diff_line?
+ - else
+ %cite.cgray started a discussion
+ %div
+ - last_note = discussion_notes.last
+ last updated by
+ = link_to last_note.author_name, project_team_member_path(@project, @project.team_member_by_id(last_note.author)), class: "note-author"
+ %span.discussion-last-update
+ = time_ago_in_words(last_note.updated_at)
+ ago
+ .discussion-body
+ - if note.for_diff_line?
+ - if note.diff
+ .content
+ .diff_file= render "notes/discussion_diff", discussion_notes: discussion_notes, note: note
+ - else
+ = link_to 'show outdated discussion', '#', class: 'js-show-outdated-discussion'
+ %div.hide.outdated-discussion
+ .content
+ .notes{ rel: discussion_notes.first.discussion_id }
+ = render discussion_notes
+
+
+ - else
+ .content
+ .notes{ rel: discussion_notes.first.discussion_id }
+ = render discussion_notes
+ = render "notes/discussion_reply_button", note: discussion_notes.first
+
+ -# will be shown when the other one is hidden
+ .discussion-hidden.content.hide
+ .note
+ %em Hidden discussion.
+ = link_to "javascript:;", class: "js-details-target js-toggler-target" do
+ %i.icon-eye-open
+ Show
+
diff --git a/app/views/notes/_discussion_diff.html.haml b/app/views/notes/_discussion_diff.html.haml
new file mode 100644
index 00000000000..93ab59c72c5
--- /dev/null
+++ b/app/views/notes/_discussion_diff.html.haml
@@ -0,0 +1,25 @@
+- diff = note.diff
+.diff_file_header
+ - if diff.deleted_file
+ %span= diff.old_path
+ - else
+ %span= diff.new_path
+ - if diff.a_mode && diff.b_mode && diff.a_mode != diff.b_mode
+ %span.file-mode= "#{diff.a_mode} → #{diff.b_mode}"
+ %br/
+.diff_file_content
+ %table
+ - each_diff_line(diff, note.diff_file_index) do |line, type, line_code, line_new, line_old|
+ %tr.line_holder{ id: line_code }
+ - if type == "match"
+ %td.old_line= "..."
+ %td.new_line= "..."
+ %td.line_content.matched= line
+ - else
+ %td.old_line= raw(type == "new" ? "&nbsp;" : line_old)
+ %td.new_line= raw(type == "old" ? "&nbsp;" : line_new)
+ %td.line_content{class: "noteable_line #{type} #{line_code}", "line_code" => line_code}= raw "#{line} &nbsp;"
+
+ - if line_code == note.line_code
+ = render "notes/diff_notes_with_reply", notes: discussion_notes
+ - break # cut off diff after notes
diff --git a/app/views/notes/_discussion_reply_button.html.haml b/app/views/notes/_discussion_reply_button.html.haml
new file mode 100644
index 00000000000..d1c5ccc29db
--- /dev/null
+++ b/app/views/notes/_discussion_reply_button.html.haml
@@ -0,0 +1,10 @@
+= link_to "javascript:;",
+ class: "btn reply-btn js-discussion-reply-button",
+ data: { noteable_type: note.noteable_type,
+ noteable_id: note.noteable_id,
+ commit_id: note.commit_id,
+ line_code: note.line_code,
+ discussion_id: note.discussion_id },
+ title: "Add a reply" do
+ %i.icon-comment
+ Reply
diff --git a/app/views/notes/_form.html.haml b/app/views/notes/_form.html.haml
new file mode 100644
index 00000000000..d094119a9da
--- /dev/null
+++ b/app/views/notes/_form.html.haml
@@ -0,0 +1,44 @@
+= form_for [@project, @note], remote: true, html: { multipart: true, id: nil, class: "new_note js-new-note-form" } do |f|
+
+ = note_target_fields
+ = f.hidden_field :commit_id
+ = f.hidden_field :line_code
+ = f.hidden_field :noteable_id
+ = f.hidden_field :noteable_type
+
+ .note_text_and_preview.js-toggler-container
+ %a.js-note-preview-button.js-toggler-target.turn-off{ href: "javascript:;", title: "Preview", data: {url: preview_project_notes_path(@project)} }
+ %i.icon-eye-open
+ %a.js-note-edit-button.js-toggler-target.turn-off{ href: "javascript:;", title: "Edit" }
+ %i.icon-edit
+
+ = f.text_area :note, size: 255, class: 'note_text js-note-text js-gfm-input turn-on'
+ .note_preview.js-note-preview.turn-off
+
+ .buttons
+ = f.submit 'Add Comment', class: "btn comment-btn grouped js-comment-button"
+ %a.btn.grouped.js-close-discussion-note-form Cancel
+ .hint
+ .right Comments are parsed with #{link_to "GitLab Flavored Markdown", help_markdown_path, target: '_blank'}.
+ .clearfix
+
+ .note_options
+ .attachment
+ %h6 Attachment:
+ .file_name.js-attachment-filename File name...
+ %a.choose-btn.btn.small.js-choose-note-attachment-button Choose File ...
+ .hint Any file up to 10 MB
+
+ = f.file_field :attachment, class: "js-note-attachment-input"
+
+ .notify_options
+ %h6 Notify via email:
+ = label_tag :notify do
+ = check_box_tag :notify, 1, !@note.for_commit?
+ Project team
+
+ .js-notify-commit-author
+ = label_tag :notify_author do
+ = check_box_tag :notify_author, 1 , @note.for_commit?
+ Commit author
+ .clearfix
diff --git a/app/views/notes/_form_errors.html.haml b/app/views/notes/_form_errors.html.haml
new file mode 100644
index 00000000000..0851536f0da
--- /dev/null
+++ b/app/views/notes/_form_errors.html.haml
@@ -0,0 +1,3 @@
+.error_message.js-errors
+ - note.errors.full_messages.each do |msg|
+ %div= msg
diff --git a/app/views/notes/_note.html.haml b/app/views/notes/_note.html.haml
index 8591e7bd86b..dd5d7c1ba8c 100644
--- a/app/views/notes/_note.html.haml
+++ b/app/views/notes/_note.html.haml
@@ -1,42 +1,37 @@
-%li{id: dom_id(note), class: "note"}
- = image_tag gravatar_icon(note.author.email), class: "avatar s32"
- %div.note-author
- %strong= note.author_name
- = link_to "##{dom_id(note)}", name: dom_id(note) do
- %cite.cgray
- = time_ago_in_words(note.updated_at)
- ago
+%li{ id: dom_id(note), class: dom_class(note), data: { discussion: note.discussion_id } }
+ .note-header
+ .note-actions
+ = link_to "##{dom_id(note)}", name: dom_id(note) do
+ %i.icon-link
+ Link here
+ &nbsp;
+ - if(note.author_id == current_user.id) || can?(current_user, :admin_note, @project)
+ = link_to project_note_path(@project, note), title: "Remove comment", method: :delete, confirm: 'Are you sure you want to remove comment?', remote: true, class: "danger js-note-delete" do
+ %i.icon-trash.cred
+ = image_tag gravatar_icon(note.author.email), class: "avatar s32"
+ = link_to note.author_name, project_team_member_path(@project, @project.team_member_by_id(note.author)), class: "note-author"
+ %span.note-last-update
+ = time_ago_in_words(note.updated_at)
+ ago
- - unless note_for_main_target?(note)
- - if note.for_commit?
- %span.cgray
- on #{link_to note.noteable.short_id, project_commit_path(@project, note.noteable)}
- = link_to_commit_diff_line_note(note) if note.for_diff_line?
+ - if note.upvote?
+ %span.vote.upvote.label.label-success
+ %i.icon-thumbs-up
+ \+1
+ - if note.downvote?
+ %span.vote.downvote.label.label-error
+ %i.icon-thumbs-down
+ \-1
- -# only show vote if it's a note for the main target
- - if note_for_main_target?(note)
- - if note.upvote?
- %span.vote.upvote.label.label-success
- %i.icon-thumbs-up
- \+1
- - if note.downvote?
- %span.vote.downvote.label.label-error
- %i.icon-thumbs-down
- \-1
- -# remove button
- - if(note.author_id == current_user.id) || can?(current_user, :admin_note, @project)
- = link_to [@project, note], confirm: 'Are you sure?', method: :delete, remote: true, class: "cred delete-note btn very_small" do
- %i.icon-trash
- Remove
-
- %div.note-title
+ .note-body
= preserve do
= markdown(note.note)
- - if note.attachment.url
- - if note.attachment.image?
- = image_tag note.attachment.url, class: 'thumbnail span4'
- .right
- %div.file
- = link_to note.attachment_identifier, note.attachment.url, target: "_blank"
+ - if note.attachment.url
+ - if note.attachment.image?
+ = image_tag note.attachment.url, class: 'note-image-attach'
+ .attachment.right
+ = link_to note.attachment.url, target: "_blank" do
+ %i.icon-attachment
+ = note.attachment_identifier
.clear
diff --git a/app/views/notes/_notes.html.haml b/app/views/notes/_notes.html.haml
index adb5dfcbf18..4904249aeff 100644
--- a/app/views/notes/_notes.html.haml
+++ b/app/views/notes/_notes.html.haml
@@ -1,4 +1,11 @@
-- @notes.each do |note|
- - next unless note.author
- = render "note", note: note
-
+- if @discussions.present?
+ - @discussions.each do |discussion_notes|
+ - note = discussion_notes.first
+ - if note_for_main_target?(note)
+ = render discussion_notes
+ - else
+ = render 'discussion', discussion_notes: discussion_notes
+- else
+ - @notes.each do |note|
+ - next unless note.author
+ = render 'note', note: note
diff --git a/app/views/notes/_notes_with_form.html.haml b/app/views/notes/_notes_with_form.html.haml
index 53716c1d3f4..2566edd81ad 100644
--- a/app/views/notes/_notes_with_form.html.haml
+++ b/app/views/notes/_notes_with_form.html.haml
@@ -1,11 +1,11 @@
-%ul#notes-list
-%ul#new-notes-list
-.notes-status
+%ul#notes-list.notes
+.js-notes-busy
+.js-main-target-form
- if can? current_user, :write_note, @project
- = render "notes/common_form"
+ = render "notes/form"
:javascript
$(function(){
- NoteList.init("#{tid}", "#{tt}", "#{project_notes_path(@project)}");
+ NoteList.init("#{@target_id}", "#{@target_type}", "#{project_notes_path(@project)}");
});
diff --git a/app/views/notes/_per_line_form.html.haml b/app/views/notes/_per_line_form.html.haml
deleted file mode 100644
index ff80ad4e0d5..00000000000
--- a/app/views/notes/_per_line_form.html.haml
+++ /dev/null
@@ -1,40 +0,0 @@
-%table{style: "display:none;"}
- %tr.per_line_form
- %td{colspan: 3 }
- .line-note-form-holder
- = form_for [@project, @note], remote: "true", multipart: true do |f|
- %h3.page_title Leave a note
- %div.span10
- -if @note.errors.any?
- .alert-message.block-message.error
- - @note.errors.full_messages.each do |msg|
- %div= msg
-
- = f.hidden_field :noteable_id
- = f.hidden_field :commit_id
- = f.hidden_field :noteable_type
- = f.hidden_field :line_code
- = f.text_area :note, size: 255, class: 'line-note-text js-gfm-input'
- .note_actions
- .buttons
- = f.submit 'Add note', class: "btn save-btn submit_note submit_inline_note", id: "submit_note"
- = link_to "Cancel", "#", class: "btn hide-button"
- .options
- %h6.left Notify via email:
- .labels
- = label_tag :notify do
- = check_box_tag :notify, 1, @note.noteable_type != "Commit"
- %span Project team
-
- - if @note.notify_only_author?(current_user)
- = label_tag :notify_author do
- = check_box_tag :notify_author, 1 , @note.noteable_type == "Commit"
- %span Commit author
-
-:javascript
- $(function(){
- $(".per_line_form .hide-button").bind("click", function(){
- $('.per_line_form').hide();
- return false;
- });
- });
diff --git a/app/views/notes/_per_line_note.html.haml b/app/views/notes/_per_line_note.html.haml
deleted file mode 100644
index 28bcd6e0c94..00000000000
--- a/app/views/notes/_per_line_note.html.haml
+++ /dev/null
@@ -1,5 +0,0 @@
-%tr.line_notes_row
- %td{colspan: 3}
- %ul
- = render "notes/note", note: note
-
diff --git a/app/views/notes/_per_line_note_link.html.haml b/app/views/notes/_per_line_note_link.html.haml
deleted file mode 100644
index 72b59a596a3..00000000000
--- a/app/views/notes/_per_line_note_link.html.haml
+++ /dev/null
@@ -1 +0,0 @@
-= link_to "", "#", class: "line_note_link", data: { line_code: line_code }, title: "Add note for this line"
diff --git a/app/views/notes/_per_line_notes_with_reply.html.haml b/app/views/notes/_per_line_notes_with_reply.html.haml
deleted file mode 100644
index 1bcfc41fe20..00000000000
--- a/app/views/notes/_per_line_notes_with_reply.html.haml
+++ /dev/null
@@ -1,3 +0,0 @@
-- notes.each do |note|
- = render "notes/per_line_note", note: note
-= render "notes/per_line_reply_button", line_code: notes.first.line_code
diff --git a/app/views/notes/_per_line_reply_button.html.haml b/app/views/notes/_per_line_reply_button.html.haml
deleted file mode 100644
index 42c737c75ae..00000000000
--- a/app/views/notes/_per_line_reply_button.html.haml
+++ /dev/null
@@ -1,4 +0,0 @@
-%tr.line_notes_row.reply
- %td{colspan: 3}
- %i.icon-comment
- = link_to "Reply", "#", class: "line_note_reply_link", data: { line_code: line_code }, title: "Add note for this line"
diff --git a/app/views/notes/_reversed_notes_with_form.html.haml b/app/views/notes/_reversed_notes_with_form.html.haml
index 24d599244b4..bb583b8c3b8 100644
--- a/app/views/notes/_reversed_notes_with_form.html.haml
+++ b/app/views/notes/_reversed_notes_with_form.html.haml
@@ -1,11 +1,12 @@
+.js-main-target-form
- if can? current_user, :write_note, @project
- = render "notes/common_form"
+ = render "notes/form"
-%ul.reversed#new-notes-list
-%ul.reversed#notes-list
-.notes-status
+%ul#new-notes-list.reversed.notes
+%ul#notes-list.reversed.notes
+.notes-busy.js-notes-busy
:javascript
$(function(){
- NoteList.init("#{tid}", "#{tt}", "#{project_notes_path(@project)}");
+ NoteList.init("#{@target_id}", "#{@target_type}", "#{project_notes_path(@project)}");
});
diff --git a/app/views/notes/create.js.haml b/app/views/notes/create.js.haml
index 03866591c4f..c573d406b73 100644
--- a/app/views/notes/create.js.haml
+++ b/app/views/notes/create.js.haml
@@ -1,8 +1,21 @@
-- if @note.line_code
- = render "create_per_line_note", note: @note
-- else
- = render "create_common_note", note: @note
+- if @note.valid?
+ var noteHtml = "#{escape_javascript(render "notes/note", note: @note)}";
+
+ - if note_for_main_target?(@note)
+ - if @note.for_wall?
+ NoteList.appendNewWallNote(#{@note.id}, noteHtml);
+ - else
+ NoteList.appendNewNote(#{@note.id}, noteHtml);
+ - else
+ :plain
+ var firstDiscussionNoteHtml = "#{escape_javascript(render "notes/diff_notes_with_reply", notes: [@note])}";
+ NoteList.appendNewDiscussionNote("#{@note.discussion_id}",
+ firstDiscussionNoteHtml,
+ noteHtml);
--# Enable submit button
-:plain
- $("#submit_note").removeAttr("disabled");
+- else
+ var errorsHtml = "#{escape_javascript(render 'notes/form_errors', note: @note)}";
+ - if note_for_main_target?(@note)
+ NoteList.errorsOnForm(errorsHtml);
+ - else
+ NoteList.errorsOnForm(errorsHtml, "#{@note.discussion_id}"); \ No newline at end of file
diff --git a/app/views/notes/index.js.haml b/app/views/notes/index.js.haml
index 3814dbd46a2..f0826100fbf 100644
--- a/app/views/notes/index.js.haml
+++ b/app/views/notes/index.js.haml
@@ -1,17 +1,15 @@
- unless @notes.blank?
+ var notesHtml = "#{escape_javascript(render 'notes/notes')}";
+ - new_note_ids = @notes.map(&:id)
- if loading_more_notes?
- :plain
- NoteList.appendMoreNotes(#{@notes.last.id}, "#{escape_javascript(render 'notes/notes')}");
+ NoteList.appendMoreNotes(#{new_note_ids}, notesHtml);
- elsif loading_new_notes?
- :plain
- NoteList.replaceNewNotes("#{escape_javascript(render 'notes/notes')}");
+ NoteList.replaceNewNotes(#{new_note_ids}, notesHtml);
- else
- :plain
- NoteList.setContent(#{@notes.first.id}, #{@notes.last.id}, "#{escape_javascript(render 'notes/notes')}");
+ NoteList.setContent(#{new_note_ids}, notesHtml);
- else
- if loading_more_notes?
- :plain
- NoteList.finishedLoadingMore();
+ NoteList.finishedLoadingMore();
diff --git a/app/views/projects/wall.html.haml b/app/views/projects/wall.html.haml
index 591a8cd06d4..82b565def43 100644
--- a/app/views/projects/wall.html.haml
+++ b/app/views/projects/wall.html.haml
@@ -1,2 +1,2 @@
%div.wall_page
- = render "notes/reversed_notes_with_form", tid: nil, tt: "wall"
+ = render "notes/reversed_notes_with_form"
diff --git a/app/views/snippets/show.html.haml b/app/views/snippets/show.html.haml
index ad399533a3d..02022185f9a 100644
--- a/app/views/snippets/show.html.haml
+++ b/app/views/snippets/show.html.haml
@@ -8,4 +8,4 @@
%br
%div= render 'blob'
-%div#notes= render "notes/notes_with_form", tid: @snippet.id, tt: "snippet"
+%div#notes= render "notes/notes_with_form"
diff --git a/features/project/commits/commit_comments.feature b/features/project/commits/commit_comments.feature
index 5acf541ab96..a1aa745a681 100644
--- a/features/project/commits/commit_comments.feature
+++ b/features/project/commits/commit_comments.feature
@@ -1,10 +1,48 @@
-Feature: Project Comment commit
+Feature: Comments on commits
Background:
Given I sign in as a user
And I own project "Shop"
- Given I visit project commit page
+ And I visit project commit page
@javascript
- Scenario: I comment commit
+ Scenario: I can comment on a commit
Given I leave a comment like "XML attached"
- Then I should see comment "XML attached"
+ Then I should see a comment saying "XML attached"
+
+ @javascript
+ Scenario: I can't cancel the main form
+ Then I should not see the cancel comment button
+
+ @javascript
+ Scenario: I can't preview without text
+ Given I haven't written any comment text
+ Then I should not see the comment preview button
+
+ @javascript
+ Scenario: I can preview with text
+ Given I write a comment like "Nice"
+ Then I should see the comment preview button
+
+ @javascript
+ Scenario: I preview a comment
+ Given I preview a comment text like "Bug fixed :smile:"
+ Then I should see the comment preview
+ And I should not see the comment text field
+
+ @javascript
+ Scenario: I can edit after preview
+ Given I preview a comment text like "Bug fixed :smile:"
+ Then I should see the comment edit button
+
+ @javascript
+ Scenario: I have a reset form after posting from preview
+ Given I preview a comment text like "Bug fixed :smile:"
+ And I submit the comment
+ Then I should see an empty comment text field
+ And I should not see the comment preview
+
+ @javascript
+ Scenario: I can delete a comment
+ Given I leave a comment like "XML attached"
+ And I delete a comment
+ Then I should not see a comment saying "XML attached"
diff --git a/features/project/commits/commit_diff_comments.feature b/features/project/commits/commit_diff_comments.feature
new file mode 100644
index 00000000000..4323e8ce596
--- /dev/null
+++ b/features/project/commits/commit_diff_comments.feature
@@ -0,0 +1,91 @@
+Feature: Comments on commit diffs
+ Background:
+ Given I sign in as a user
+ And I own project "Shop"
+ And I visit project commit page
+
+ @javascript
+ Scenario: I can access add diff comment buttons
+ Then I should see add a diff comment button
+
+ @javascript
+ Scenario: I can comment on a commit diff
+ Given I leave a diff comment like "Typo, please fix"
+ Then I should see a diff comment saying "Typo, please fix"
+
+ @javascript
+ Scenario: I get a temporary form for the first comment on a diff line
+ Given I open a diff comment form
+ Then I should see a temporary diff comment form
+
+ @javascript
+ Scenario: I have a cancel button on the diff form
+ Given I open a diff comment form
+ Then I should see the cancel comment button
+
+ @javascript
+ Scenario: I can cancel a diff form
+ Given I open a diff comment form
+ And I cancel the diff comment
+ Then I should not see the diff comment form
+
+ @javascript
+ Scenario: I can't open a second form for a diff line
+ Given I open a diff comment form
+ And I open a diff comment form
+ Then I should only see one diff form
+
+ @javascript
+ Scenario: I can have multiple forms
+ Given I open a diff comment form
+ And I write a diff comment like ":-1: I don't like this"
+ And I open another diff comment form
+ Then I should see a diff comment form with ":-1: I don't like this"
+ And I should see an empty diff comment form
+
+ @javascript
+ Scenario: I can preview multiple forms separately
+ Given I preview a diff comment text like "Should fix it :smile:"
+ And I preview another diff comment text like "DRY this up"
+ Then I should see two separate previews
+
+ @javascript
+ Scenario: I have a reply button in discussions
+ Given I leave a diff comment like "Typo, please fix"
+ Then I should see a discussion reply button
+
+ @javascript
+ Scenario: I can't preview without text
+ Given I open a diff comment form
+ And I haven't written any diff comment text
+ Then I should not see the diff comment preview button
+
+ @javascript
+ Scenario: I can preview with text
+ Given I open a diff comment form
+ And I write a diff comment like ":-1: I don't like this"
+ Then I should see the diff comment preview button
+
+ @javascript
+ Scenario: I preview a diff comment
+ Given I preview a diff comment text like "Should fix it :smile:"
+ Then I should see the diff comment preview
+ And I should not see the diff comment text field
+
+ @javascript
+ Scenario: I can edit after preview
+ Given I preview a diff comment text like "Should fix it :smile:"
+ Then I should see the diff comment edit button
+
+ @javascript
+ Scenario: The form gets removed after posting
+ Given I preview a diff comment text like "Should fix it :smile:"
+ And I submit the diff comment
+ Then I should not see the diff comment form
+ And I should see a discussion reply button
+
+ @javascript
+ Scenario: I can delete a discussion comment
+ Given I leave a diff comment like "Typo, please fix"
+ And I delete a diff comment
+ Then I should not see a diff comment saying "Typo, please fix"
diff --git a/features/project/merge_requests.feature b/features/project/merge_requests.feature
index 80f00986466..5b8becbb6c9 100644
--- a/features/project/merge_requests.feature
+++ b/features/project/merge_requests.feature
@@ -35,8 +35,34 @@ Feature: Project Merge Requests
Then I should see merge request "Wiki Feature"
@javascript
- Scenario: I comment merge request
+ Scenario: I comment on a merge request
Given I visit merge request page "Bug NS-04"
And I leave a comment like "XML attached"
Then I should see comment "XML attached"
+ @javascript
+ Scenario: I comment on a merge request diff
+ Given project "Shop" have "Bug NS-05" open merge request with diffs inside
+ And I visit merge request page "Bug NS-05"
+ And I switch to the diff tab
+ And I leave a comment like "Line is wrong" on line 185 of the first file
+ And I switch to the merge request's comments tab
+ Then I should see a discussion has started on line 185
+
+ @javascript
+ Scenario: I comment on a line of a commit in merge request
+ Given project "Shop" have "Bug NS-05" open merge request with diffs inside
+ And I visit merge request page "Bug NS-05"
+ And I click on the first commit in the merge request
+ And I leave a comment like "Line is wrong" on line 185 of the first file
+ And I switch to the merge request's comments tab
+ Then I should see a discussion has started on commit bcf03b5de6c:L185
+
+ @javascript
+ Scenario: I comment on a commit in merge request
+ Given project "Shop" have "Bug NS-05" open merge request with diffs inside
+ And I visit merge request page "Bug NS-05"
+ And I click on the first commit in the merge request
+ And I leave a comment on the diff page
+ And I switch to the merge request's comments tab
+ Then I should see a discussion has started on commit bcf03b5de6c
diff --git a/features/steps/project/comments_on_commit_diffs.rb b/features/steps/project/comments_on_commit_diffs.rb
new file mode 100644
index 00000000000..fc397a4fa91
--- /dev/null
+++ b/features/steps/project/comments_on_commit_diffs.rb
@@ -0,0 +1,6 @@
+class CommentsOnCommitDiffs < Spinach::FeatureSteps
+ include SharedAuthentication
+ include SharedDiffNote
+ include SharedPaths
+ include SharedProject
+end
diff --git a/features/steps/project/project_comment_commit.rb b/features/steps/project/comments_on_commits.rb
index cb8385e1ce5..56bb12a8209 100644
--- a/features/steps/project/project_comment_commit.rb
+++ b/features/steps/project/comments_on_commits.rb
@@ -1,6 +1,6 @@
-class ProjectCommentCommit < Spinach::FeatureSteps
+class CommentsOnCommits < Spinach::FeatureSteps
include SharedAuthentication
- include SharedProject
include SharedNote
include SharedPaths
+ include SharedProject
end
diff --git a/features/steps/project/project_merge_requests.rb b/features/steps/project/project_merge_requests.rb
index 0ec8245a59a..5d5ad4490de 100644
--- a/features/steps/project/project_merge_requests.rb
+++ b/features/steps/project/project_merge_requests.rb
@@ -4,50 +4,55 @@ class ProjectMergeRequests < Spinach::FeatureSteps
include SharedNote
include SharedPaths
- Then 'I should see "Bug NS-04" in merge requests' do
- page.should have_content "Bug NS-04"
+ Given 'I click link "New Merge Request"' do
+ click_link "New Merge Request"
end
- And 'I should not see "Feature NS-03" in merge requests' do
- page.should_not have_content "Feature NS-03"
+ Given 'I click link "Bug NS-04"' do
+ click_link "Bug NS-04"
+ end
+
+ Given 'I click link "All"' do
+ click_link "All"
end
Given 'I click link "Closed"' do
click_link "Closed"
end
- Then 'I should see "Feature NS-03" in merge requests' do
- page.should have_content "Feature NS-03"
+ Then 'I should see merge request "Wiki Feature"' do
+ page.should have_content "Wiki Feature"
end
- And 'I should not see "Bug NS-04" in merge requests' do
- page.should_not have_content "Bug NS-04"
+ Then 'I should see closed merge request "Bug NS-04"' do
+ mr = MergeRequest.find_by_title("Bug NS-04")
+ mr.closed.should be_true
+ page.should have_content "Closed by"
end
- Given 'I click link "All"' do
- click_link "All"
+ Then 'I should see merge request "Bug NS-04"' do
+ page.should have_content "Bug NS-04"
end
- Given 'I click link "Bug NS-04"' do
- click_link "Bug NS-04"
+ Then 'I should see "Bug NS-04" in merge requests' do
+ page.should have_content "Bug NS-04"
end
- Then 'I should see merge request "Bug NS-04"' do
- page.should have_content "Bug NS-04"
+ Then 'I should see "Feature NS-03" in merge requests' do
+ page.should have_content "Feature NS-03"
end
- And 'I click link "Close"' do
- click_link "Close"
+ And 'I should not see "Feature NS-03" in merge requests' do
+ page.should_not have_content "Feature NS-03"
end
- Then 'I should see closed merge request "Bug NS-04"' do
- mr = MergeRequest.find_by_title("Bug NS-04")
- mr.closed.should be_true
- page.should have_content "Closed by"
+
+ And 'I should not see "Bug NS-04" in merge requests' do
+ page.should_not have_content "Bug NS-04"
end
- Given 'I click link "New Merge Request"' do
- click_link "New Merge Request"
+ And 'I click link "Close"' do
+ click_link "Close"
end
And 'I submit new merge request "Wiki Feature"' do
@@ -57,24 +62,91 @@ class ProjectMergeRequests < Spinach::FeatureSteps
click_button "Submit merge request"
end
- Then 'I should see merge request "Wiki Feature"' do
- page.should have_content "Wiki Feature"
- end
-
And 'project "Shop" have "Bug NS-04" open merge request' do
project = Project.find_by_name("Shop")
create(:merge_request,
- :title => "Bug NS-04",
- :project => project,
- :author => project.users.first)
+ title: "Bug NS-04",
+ project: project,
+ author: project.users.first)
+ end
+
+ And 'project "Shop" have "Bug NS-05" open merge request with diffs inside' do
+ project = Project.find_by_name("Shop")
+ create(:merge_request_with_diffs,
+ title: "Bug NS-05",
+ project: project,
+ author: project.users.first)
end
And 'project "Shop" have "Feature NS-03" closed merge request' do
project = Project.find_by_name("Shop")
create(:merge_request,
- :title => "Feature NS-03",
- :project => project,
- :author => project.users.first,
- :closed => true)
+ title: "Feature NS-03",
+ project: project,
+ author: project.users.first,
+ closed: true)
+ end
+
+ And 'I switch to the diff tab' do
+ mr = MergeRequest.find_by_title("Bug NS-05")
+ visit diffs_project_merge_request_path(mr.project, mr)
+ end
+
+ And 'I switch to the merge request\'s comments tab' do
+ mr = MergeRequest.find_by_title("Bug NS-05")
+ visit project_merge_request_path(mr.project, mr)
+ end
+
+ And 'I click on the first commit in the merge request' do
+ mr = MergeRequest.find_by_title("Bug NS-05")
+ click_link mr.commits.first.short_id(8)
+ end
+
+ And 'I leave a comment on the diff page' do
+ within(:xpath, "//div[@class='note-form-holder']") do
+ fill_in "note_note", with: "One comment to rule them all"
+ click_button "Add Comment"
+ end
+ end
+
+ And 'I leave a comment like "Line is wrong" on line 185 of the first file' do
+ save_and_open_page
+ within(:xpath, "//div[@class='diff_file'][1]") do
+ click_link "add-diff-line-note-0_185_185"
+ end
+
+ within(:xpath, "//div[@class='line-note-form-holder']") do
+ fill_in "note_note", with: "Line is wrong"
+ click_button "Add Comment"
+ end
+ end
+
+ Then 'I should see a discussion has started on line 185' do
+ mr = MergeRequest.find_by_title("Bug NS-05")
+ first_commit = mr.commits.first
+ first_diff = mr.diffs.first
+ page.should have_content "#{current_user.name} started a discussion on this merge request diff"
+ page.should have_content "#{first_diff.b_path}:L185"
+ page.should have_content "Line is wrong"
+ end
+
+ Then 'I should see a discussion has started on commit bcf03b5de6c:L185' do
+ mr = MergeRequest.find_by_title("Bug NS-05")
+ first_commit = mr.commits.first
+ first_diff = mr.diffs.first
+ page.should have_content "#{current_user.name} started a discussion on commit"
+ page.should have_content first_commit.short_id(8)
+ page.should have_content "#{first_diff.b_path}:L185"
+ page.should have_content "Line is wrong"
+ end
+
+ Then 'I should see a discussion has started on commit bcf03b5de6c' do
+ mr = MergeRequest.find_by_title("Bug NS-05")
+ first_commit = mr.st_commits.first
+ first_diff = mr.diffs.first
+ page.should have_content "#{current_user.name} started a discussion on commit"
+ page.should have_content first_commit.short_id(8)
+ page.should have_content "One comment to rule them all"
+ page.should_not have_content "#{first_diff.b_path}:L185"
end
end
diff --git a/features/steps/shared/diff_note.rb b/features/steps/shared/diff_note.rb
new file mode 100644
index 00000000000..431ef022d0b
--- /dev/null
+++ b/features/steps/shared/diff_note.rb
@@ -0,0 +1,158 @@
+module SharedDiffNote
+ include Spinach::DSL
+
+ Given 'I cancel the diff comment' do
+ within(".diff_file") do
+ find(".js-close-discussion-note-form").trigger("click")
+ end
+ end
+
+ Given 'I delete a diff comment' do
+ within(".diff_file") do
+ first(".js-note-delete").trigger("click")
+ end
+ end
+
+ Given 'I haven\'t written any diff comment text' do
+ within(".diff_file") do
+ fill_in "note[note]", with: ""
+ end
+ end
+
+ Given 'I leave a diff comment like "Typo, please fix"' do
+ find("#586fb7c4e1add2d4d24e27566ed7064680098646_29_14.line_holder .js-add-diff-note-button").trigger("click")
+ within(".diff_file") do
+ fill_in "note[note]", with: "Typo, please fix"
+ #click_button("Add Comment")
+ find(".js-comment-button").trigger("click")
+ end
+ end
+
+ Given 'I preview a diff comment text like "Should fix it :smile:"' do
+ find("#586fb7c4e1add2d4d24e27566ed7064680098646_29_14.line_holder .js-add-diff-note-button").trigger("click")
+ within(".diff_file") do
+ fill_in "note[note]", with: "Should fix it :smile:"
+ find(".js-note-preview-button").trigger("click")
+ end
+ end
+
+ Given 'I preview another diff comment text like "DRY this up"' do
+ find("#586fb7c4e1add2d4d24e27566ed7064680098646_57_41.line_holder .js-add-diff-note-button").trigger("click")
+ within(".diff_file") do
+ fill_in "note[note]", with: "DRY this up"
+ find(".js-note-preview-button").trigger("click")
+ end
+ end
+
+ Given 'I open a diff comment form' do
+ find("#586fb7c4e1add2d4d24e27566ed7064680098646_29_14.line_holder .js-add-diff-note-button").trigger("click")
+ end
+
+ Given 'I open another diff comment form' do
+ find("#586fb7c4e1add2d4d24e27566ed7064680098646_57_41.line_holder .js-add-diff-note-button").trigger("click")
+ end
+
+ Given 'I write a diff comment like ":-1: I don\'t like this"' do
+ within(".diff_file") do
+ fill_in "note[note]", with: ":-1: I don\'t like this"
+ end
+ end
+
+ Given 'I submit the diff comment' do
+ within(".diff_file") do
+ click_button("Add Comment")
+ end
+ end
+
+
+
+ Then 'I should not see the diff comment form' do
+ within(".diff_file") do
+ page.should_not have_css("form.new_note")
+ end
+ end
+
+ Then 'I should not see the diff comment preview button' do
+ within(".diff_file") do
+ page.should have_css(".js-note-preview-button", visible: false)
+ end
+ end
+
+ Then 'I should not see the diff comment text field' do
+ within(".diff_file") do
+ page.should have_css(".js-note-text", visible: false)
+ end
+ end
+
+ Then 'I should only see one diff form' do
+ within(".diff_file") do
+ page.should have_css("form.new_note", count: 1)
+ end
+ end
+
+ Then 'I should see a diff comment form with ":-1: I don\'t like this"' do
+ within(".diff_file") do
+ page.should have_field("note[note]", with: ":-1: I don\'t like this")
+ end
+ end
+
+ Then 'I should see a diff comment saying "Typo, please fix"' do
+ within(".diff_file .note") do
+ page.should have_content("Typo, please fix")
+ end
+ end
+
+ Then 'I should see a discussion reply button' do
+ within(".diff_file") do
+ page.should have_link("Reply")
+ end
+ end
+
+ Then 'I should see a temporary diff comment form' do
+ within(".diff_file") do
+ page.should have_css(".js-temp-notes-holder form.new_note")
+ end
+ end
+
+ Then 'I should see add a diff comment button' do
+ page.should have_css(".js-add-diff-note-button", visible: false)
+ end
+
+ Then 'I should see an empty diff comment form' do
+ within(".diff_file") do
+ page.should have_field("note[note]", with: "")
+ end
+ end
+
+ Then 'I should see the cancel comment button' do
+ within(".diff_file form") do
+ page.should have_css(".js-close-discussion-note-form", text: "Cancel")
+ end
+ end
+
+ Then 'I should see the diff comment preview' do
+ within(".diff_file form") do
+ page.should have_css(".js-note-preview", visible: false)
+ end
+ end
+
+ Then 'I should see the diff comment edit button' do
+ within(".diff_file") do
+ page.should have_css(".js-note-edit-button", visible: true)
+ end
+ end
+
+ Then 'I should see the diff comment preview button' do
+ within(".diff_file") do
+ page.should have_css(".js-note-preview-button", visible: true)
+ end
+ end
+
+ Then 'I should see two separate previews' do
+ within(".diff_file") do
+ page.should have_css(".js-note-preview", visible: true, count: 2)
+ page.should have_content("Should fix it")
+ page.should have_content("DRY this up")
+ end
+ end
+end
diff --git a/features/steps/shared/note.rb b/features/steps/shared/note.rb
index 923e69b6b07..3d5441c40b0 100644
--- a/features/steps/shared/note.rb
+++ b/features/steps/shared/note.rb
@@ -1,18 +1,111 @@
module SharedNote
include Spinach::DSL
+ Given 'I delete a comment' do
+ first(".js-note-delete").trigger("click")
+ end
+
+ Given 'I haven\'t written any comment text' do
+ within(".js-main-target-form") do
+ fill_in "note[note]", with: ""
+ end
+ end
+
Given 'I leave a comment like "XML attached"' do
- fill_in "note_note", :with => "XML attached"
- click_button "Add Comment"
+ within(".js-main-target-form") do
+ fill_in "note[note]", with: "XML attached"
+ click_button "Add Comment"
+ end
end
- Then 'I should see comment "XML attached"' do
- page.should have_content "XML attached"
+ Given 'I preview a comment text like "Bug fixed :smile:"' do
+ within(".js-main-target-form") do
+ fill_in "note[note]", with: "Bug fixed :smile:"
+ find(".js-note-preview-button").trigger("click")
+ end
end
+ Given 'I submit the comment' do
+ within(".js-main-target-form") do
+ click_button "Add Comment"
+ end
+ end
+
+ Given 'I write a comment like "Nice"' do
+ within(".js-main-target-form") do
+ fill_in "note[note]", with: "Nice"
+ end
+ end
+
+
+
+ Then 'I should not see a comment saying "XML attached"' do
+ page.should_not have_css(".note")
+ end
+
+ Then 'I should not see the cancel comment button' do
+ within(".js-main-target-form") do
+ should_not have_link("Cancel")
+ end
+ end
+
+ Then 'I should not see the comment preview' do
+ within(".js-main-target-form") do
+ page.should have_css(".js-note-preview", visible: false)
+ end
+ end
+
+ Then 'I should not see the comment preview button' do
+ within(".js-main-target-form") do
+ page.should have_css(".js-note-preview-button", visible: false)
+ end
+ end
+
+ Then 'I should not see the comment text field' do
+ within(".js-main-target-form") do
+ page.should have_css(".js-note-text", visible: false)
+ end
+ end
+
+ Then 'I should see a comment saying "XML attached"' do
+ within(".note") do
+ page.should have_content("XML attached")
+ end
+ end
+
+ Then 'I should see an empty comment text field' do
+ within(".js-main-target-form") do
+ page.should have_field("note[note]", with: "")
+ end
+ end
+
+ Then 'I should see the comment edit button' do
+ within(".js-main-target-form") do
+ page.should have_css(".js-note-edit-button", visible: true)
+ end
+ end
+
+ Then 'I should see the comment preview' do
+ within(".js-main-target-form") do
+ page.should have_css(".js-note-preview", visible: true)
+ end
+ end
+
+ Then 'I should see the comment preview button' do
+ within(".js-main-target-form") do
+ page.should have_css(".js-note-preview-button", visible: true)
+ end
+ end
+
+
+
+ # Wall
+
Given 'I write new comment "my special test message"' do
- fill_in "note_note", :with => "my special test message"
- click_button "Add Comment"
+ within(".js-main-target-form") do
+ fill_in "note[note]", with: "my special test message"
+ click_button "Add Comment"
+ end
end
Then 'I should see project wall note "my special test message"' do
diff --git a/features/steps/shared/paths.rb b/features/steps/shared/paths.rb
index 1f5dfd243a1..c046c4e63e6 100644
--- a/features/steps/shared/paths.rb
+++ b/features/steps/shared/paths.rb
@@ -224,6 +224,11 @@ module SharedPaths
visit project_merge_request_path(mr.project, mr)
end
+ Given 'I visit merge request page "Bug NS-05"' do
+ mr = MergeRequest.find_by_title("Bug NS-05")
+ visit project_merge_request_path(mr.project, mr)
+ end
+
And 'I visit project "Shop" merge requests page' do
visit project_merge_requests_path(Project.find_by_name("Shop"))
end
diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb
index 59249a229eb..d3f678cfcbc 100644
--- a/lib/gitlab/markdown.rb
+++ b/lib/gitlab/markdown.rb
@@ -25,22 +25,6 @@ module Gitlab
# >> gfm(":trollface:")
# => "<img alt=\":trollface:\" class=\"emoji\" src=\"/images/trollface.png" title=\":trollface:\" />
module Markdown
- REFERENCE_PATTERN = %r{
- (?<prefix>\W)? # Prefix
- ( # Reference
- @(?<user>[a-zA-Z][a-zA-Z0-9_\-\.]*) # User name
- |\#(?<issue>\d+) # Issue ID
- |!(?<merge_request>\d+) # MR ID
- |\$(?<snippet>\d+) # Snippet ID
- |(?<commit>[\h]{6,40}) # Commit ID
- )
- (?<suffix>\W)? # Suffix
- }x.freeze
-
- TYPES = [:user, :issue, :merge_request, :snippet, :commit].freeze
-
- EMOJI_PATTERN = %r{(:(\S+):)}.freeze
-
attr_reader :html_options
# Public: Parse the provided text with GitLab-Flavored Markdown
@@ -96,6 +80,20 @@ module Gitlab
text
end
+ REFERENCE_PATTERN = %r{
+ (?<prefix>\W)? # Prefix
+ ( # Reference
+ @(?<user>[a-zA-Z][a-zA-Z0-9_\-\.]*) # User name
+ |\#(?<issue>\d+) # Issue ID
+ |!(?<merge_request>\d+) # MR ID
+ |\$(?<snippet>\d+) # Snippet ID
+ |(?<commit>[\h]{6,40}) # Commit ID
+ )
+ (?<suffix>\W)? # Suffix
+ }x.freeze
+
+ TYPES = [:user, :issue, :merge_request, :snippet, :commit].freeze
+
def parse_references(text)
# parse reference links
text.gsub!(REFERENCE_PATTERN) do |match|
@@ -115,11 +113,13 @@ module Gitlab
end
end
+ EMOJI_PATTERN = %r{(:(\S+):)}.freeze
+
def parse_emoji(text)
# parse emoji
text.gsub!(EMOJI_PATTERN) do |match|
if valid_emoji?($2)
- image_tag("emoji/#{$2}.png", size: "20x20", class: 'emoji', title: $1, alt: $1)
+ image_tag("emoji/#{$2}.png", class: 'emoji', title: $1, alt: $1, size: "20x20")
else
match
end
diff --git a/spec/factories.rb b/spec/factories.rb
index 77565a5d341..593b8350fda 100644
--- a/spec/factories.rb
+++ b/spec/factories.rb
@@ -73,8 +73,8 @@ FactoryGirl.define do
# pick 3 commits "at random" (from bcf03b5d~3 to bcf03b5d)
trait :with_diffs do
- target_branch "bcf03b5d~3"
- source_branch "bcf03b5d"
+ target_branch "master" # pretend bcf03b5d~3
+ source_branch "stable" # pretend bcf03b5d
st_commits do
[Commit.new(project.repo.commit('bcf03b5d')),
Commit.new(project.repo.commit('bcf03b5d~1')),
@@ -92,6 +92,32 @@ FactoryGirl.define do
factory :note do
project
note "Note"
+ author
+
+ factory :note_on_commit, traits: [:on_commit]
+ factory :note_on_commit_diff, traits: [:on_commit, :on_diff]
+ factory :note_on_issue, traits: [:on_issue], aliases: [:votable_note]
+ factory :note_on_merge_request, traits: [:on_merge_request]
+ factory :note_on_merge_request_diff, traits: [:on_merge_request, :on_diff]
+
+ trait :on_commit do
+ commit_id "bcf03b5de6c33f3869ef70d68cf06e679d1d7f9a"
+ noteable_type "Commit"
+ end
+
+ trait :on_diff do
+ line_code "0_184_184"
+ end
+
+ trait :on_merge_request do
+ noteable_id 1
+ noteable_type "MergeRequest"
+ end
+
+ trait :on_issue do
+ noteable_id 1
+ noteable_type "Issue"
+ end
end
factory :event do
diff --git a/spec/lib/votes_spec.rb b/spec/lib/votes_spec.rb
index 98666022a8f..7e49ac781c2 100644
--- a/spec/lib/votes_spec.rb
+++ b/spec/lib/votes_spec.rb
@@ -1,132 +1,138 @@
require 'spec_helper'
describe Issue do
- let(:issue) { create(:issue) }
+ it { should include_module(Votes) }
+end
+
+describe MergeRequest do
+ let(:merge_request) { FactoryGirl.create(:merge_request_with_diffs) }
+
+ it { should include_module(Votes) }
describe "#upvotes" do
it "with no notes has a 0/0 score" do
- issue.upvotes.should == 0
+ merge_request.upvotes.should == 0
end
it "should recognize non-+1 notes" do
- issue.notes << create(:note, note: "No +1 here")
- issue.should have(1).note
- issue.notes.first.upvote?.should be_false
- issue.upvotes.should == 0
+ merge_request.notes << create(:note, note: "No +1 here")
+ merge_request.should have(1).note
+ merge_request.notes.first.upvote?.should be_false
+ merge_request.upvotes.should == 0
end
it "should recognize a single +1 note" do
- issue.notes << create(:note, note: "+1 This is awesome")
- issue.upvotes.should == 1
+ merge_request.notes << create(:note, note: "+1 This is awesome")
+ merge_request.upvotes.should == 1
end
it "should recognize multiple +1 notes" do
- issue.notes << create(:note, note: "+1 This is awesome")
- issue.notes << create(:note, note: "+1 I want this")
- issue.upvotes.should == 2
+ merge_request.notes << create(:note, note: "+1 This is awesome")
+ merge_request.notes << create(:note, note: "+1 I want this")
+ merge_request.upvotes.should == 2
end
end
describe "#downvotes" do
it "with no notes has a 0/0 score" do
- issue.downvotes.should == 0
+ merge_request.downvotes.should == 0
end
it "should recognize non--1 notes" do
- issue.notes << create(:note, note: "Almost got a -1")
- issue.should have(1).note
- issue.notes.first.downvote?.should be_false
- issue.downvotes.should == 0
+ merge_request.notes << create(:note, note: "Almost got a -1")
+ merge_request.should have(1).note
+ merge_request.notes.first.downvote?.should be_false
+ merge_request.downvotes.should == 0
end
it "should recognize a single -1 note" do
- issue.notes << create(:note, note: "-1 This is bad")
- issue.downvotes.should == 1
+ merge_request.notes << create(:note, note: "-1 This is bad")
+ merge_request.downvotes.should == 1
end
it "should recognize multiple -1 notes" do
- issue.notes << create(:note, note: "-1 This is bad")
- issue.notes << create(:note, note: "-1 Away with this")
- issue.downvotes.should == 2
+ merge_request.notes << create(:note, note: "-1 This is bad")
+ merge_request.notes << create(:note, note: "-1 Away with this")
+ merge_request.downvotes.should == 2
end
end
describe "#votes_count" do
it "with no notes has a 0/0 score" do
- issue.votes_count.should == 0
+ merge_request.votes_count.should == 0
end
it "should recognize non notes" do
- issue.notes << create(:note, note: "No +1 here")
- issue.should have(1).note
- issue.votes_count.should == 0
+ merge_request.notes << create(:note, note: "No +1 here")
+ merge_request.should have(1).note
+ merge_request.votes_count.should == 0
end
it "should recognize a single +1 note" do
- issue.notes << create(:note, note: "+1 This is awesome")
- issue.votes_count.should == 1
+ merge_request.notes << create(:note, note: "+1 This is awesome")
+ merge_request.votes_count.should == 1
end
it "should recognize a single -1 note" do
- issue.notes << create(:note, note: "-1 This is bad")
- issue.votes_count.should == 1
+ merge_request.notes << create(:note, note: "-1 This is bad")
+ merge_request.votes_count.should == 1
end
it "should recognize multiple notes" do
- issue.notes << create(:note, note: "+1 This is awesome")
- issue.notes << create(:note, note: "-1 This is bad")
- issue.notes << create(:note, note: "+1 I want this")
- issue.votes_count.should == 3
+ merge_request.notes << create(:note, note: "+1 This is awesome")
+ merge_request.notes << create(:note, note: "-1 This is bad")
+ merge_request.notes << create(:note, note: "+1 I want this")
+ merge_request.votes_count.should == 3
end
end
describe "#upvotes_in_percent" do
it "with no notes has a 0% score" do
- issue.upvotes_in_percent.should == 0
+ merge_request.upvotes_in_percent.should == 0
end
it "should count a single 1 note as 100%" do
- issue.notes << create(:note, note: "+1 This is awesome")
- issue.upvotes_in_percent.should == 100
+ merge_request.notes << create(:note, note: "+1 This is awesome")
+ merge_request.upvotes_in_percent.should == 100
end
it "should count multiple +1 notes as 100%" do
- issue.notes << create(:note, note: "+1 This is awesome")
- issue.notes << create(:note, note: "+1 I want this")
- issue.upvotes_in_percent.should == 100
+ merge_request.notes << create(:note, note: "+1 This is awesome")
+ merge_request.notes << create(:note, note: "+1 I want this")
+ merge_request.upvotes_in_percent.should == 100
end
it "should count fractions for multiple +1 and -1 notes correctly" do
- issue.notes << create(:note, note: "+1 This is awesome")
- issue.notes << create(:note, note: "+1 I want this")
- issue.notes << create(:note, note: "-1 This is bad")
- issue.notes << create(:note, note: "+1 me too")
- issue.upvotes_in_percent.should == 75
+ merge_request.notes << create(:note, note: "+1 This is awesome")
+ merge_request.notes << create(:note, note: "+1 I want this")
+ merge_request.notes << create(:note, note: "-1 This is bad")
+ merge_request.notes << create(:note, note: "+1 me too")
+ merge_request.upvotes_in_percent.should == 75
end
end
describe "#downvotes_in_percent" do
it "with no notes has a 0% score" do
- issue.downvotes_in_percent.should == 0
+ merge_request.downvotes_in_percent.should == 0
end
it "should count a single -1 note as 100%" do
- issue.notes << create(:note, note: "-1 This is bad")
- issue.downvotes_in_percent.should == 100
+ merge_request.notes << create(:note, note: "-1 This is bad")
+ merge_request.downvotes_in_percent.should == 100
end
it "should count multiple -1 notes as 100%" do
- issue.notes << create(:note, note: "-1 This is bad")
- issue.notes << create(:note, note: "-1 Away with this")
- issue.downvotes_in_percent.should == 100
+ merge_request.notes << create(:note, note: "-1 This is bad")
+ merge_request.notes << create(:note, note: "-1 Away with this")
+ merge_request.downvotes_in_percent.should == 100
end
it "should count fractions for multiple +1 and -1 notes correctly" do
- issue.notes << create(:note, note: "+1 This is awesome")
- issue.notes << create(:note, note: "+1 I want this")
- issue.notes << create(:note, note: "-1 This is bad")
- issue.notes << create(:note, note: "+1 me too")
- issue.downvotes_in_percent.should == 25
+ merge_request.notes << create(:note, note: "+1 This is awesome")
+ merge_request.notes << create(:note, note: "+1 I want this")
+ merge_request.notes << create(:note, note: "-1 This is bad")
+ merge_request.notes << create(:note, note: "+1 me too")
+ merge_request.downvotes_in_percent.should == 25
end
end
end
diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index 62e83b31b7f..66450b244c1 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -38,34 +38,34 @@ describe Note do
let(:project) { create(:project) }
it "recognizes a neutral note" do
- note = create(:note, note: "This is not a +1 note")
+ note = create(:votable_note, note: "This is not a +1 note")
note.should_not be_upvote
note.should_not be_downvote
end
it "recognizes a neutral emoji note" do
- note = build(:note, note: "I would :+1: this, but I don't want to")
+ note = build(:votable_note, note: "I would :+1: this, but I don't want to")
note.should_not be_upvote
note.should_not be_downvote
end
it "recognizes a +1 note" do
- note = create(:note, note: "+1 for this")
+ note = create(:votable_note, note: "+1 for this")
note.should be_upvote
end
it "recognizes a +1 emoji as a vote" do
- note = build(:note, note: ":+1: for this")
+ note = build(:votable_note, note: ":+1: for this")
note.should be_upvote
end
it "recognizes a -1 note" do
- note = create(:note, note: "-1 for this")
+ note = create(:votable_note, note: "-1 for this")
note.should be_downvote
end
it "recognizes a -1 emoji as a vote" do
- note = build(:note, note: ":-1: for this")
+ note = build(:votable_note, note: ":-1: for this")
note.should be_downvote
end
end
@@ -74,43 +74,72 @@ describe Note do
let(:commit) { project.repository.commit }
describe "Commit notes" do
- before do
- @note = create(:note,
- commit_id: commit.id,
- noteable_type: "Commit")
- end
+ let!(:note) { create(:note_on_commit, note: "+1 from me") }
+ let!(:commit) { note.noteable }
it "should be accessible through #noteable" do
- @note.commit_id.should == commit.id
- @note.noteable.should be_a(Commit)
- @note.noteable.should == commit
+ note.commit_id.should == commit.id
+ note.noteable.should be_a(Commit)
+ note.noteable.should == commit
end
it "should save a valid note" do
- @note.commit_id.should == commit.id
- @note.noteable == commit
+ note.commit_id.should == commit.id
+ note.noteable == commit
end
it "should be recognized by #for_commit?" do
- @note.should be_for_commit
+ note.should be_for_commit
end
- end
- describe "Pre-line commit notes" do
- before do
- @note = create(:note,
- commit_id: commit.id,
- noteable_type: "Commit",
- line_code: "0_16_1")
+ it "should not be votable" do
+ note.should_not be_votable
end
+ end
+
+ describe "Commit diff line notes" do
+ let!(:note) { create(:note_on_commit_line, note: "+1 from me") }
+ let!(:commit) { note.noteable }
it "should save a valid note" do
- @note.commit_id.should == commit.id
- @note.noteable.id.should == commit.id
+ note.commit_id.should == commit.id
+ note.noteable.id.should == commit.id
end
it "should be recognized by #for_diff_line?" do
- @note.should be_for_diff_line
+ note.should be_for_diff_line
+ end
+
+ it "should be recognized by #for_commit_diff_line?" do
+ note.should be_for_commit_diff_line
+ end
+
+ it "should not be votable" do
+ note.should_not be_votable
+ end
+ end
+
+ describe "Issue notes" do
+ let!(:note) { create(:note_on_issue, note: "+1 from me") }
+
+ it "should not be votable" do
+ note.should be_votable
+ end
+ end
+
+ describe "Merge request notes" do
+ let!(:note) { create(:note_on_merge_request, note: "+1 from me") }
+
+ it "should not be votable" do
+ note.should be_votable
+ end
+ end
+
+ describe "Merge request diff line notes" do
+ let!(:note) { create(:note_on_merge_request_line, note: "+1 from me") }
+
+ it "should not be votable" do
+ note.should_not be_votable
end
end
diff --git a/spec/requests/notes_on_merge_requests_spec.rb b/spec/requests/notes_on_merge_requests_spec.rb
new file mode 100644
index 00000000000..8104582c920
--- /dev/null
+++ b/spec/requests/notes_on_merge_requests_spec.rb
@@ -0,0 +1,233 @@
+require 'spec_helper'
+
+describe "On a merge request", js: true do
+ let!(:project) { create(:project) }
+ let!(:merge_request) { create(:merge_request, project: project) }
+
+ before do
+ login_as :user
+ project.team << [@user, :master]
+
+ visit project_merge_request_path(project, merge_request)
+ end
+
+ subject { page }
+
+ describe "the note form" do
+ # main target form creation
+ it { should have_css(".js-main-target-form", visible: true, count: 1) }
+
+ # button initalization
+ it { within(".js-main-target-form") { should have_button("Add Comment") } }
+ it { within(".js-main-target-form") { should_not have_link("Cancel") } }
+
+ # notifiactions
+ it { within(".js-main-target-form") { should have_checked_field("Project team") } }
+ it { within(".js-main-target-form") { should_not have_checked_field("Commit author") } }
+ it { within(".js-main-target-form") { should_not have_unchecked_field("Commit author") } }
+
+ describe "without text" do
+ it { within(".js-main-target-form") { should have_css(".js-note-preview-button", visible: false) } }
+ end
+
+ describe "with text" do
+ before do
+ within(".js-main-target-form") do
+ fill_in "note[note]", with: "This is awesome"
+ end
+ end
+
+ it { within(".js-main-target-form") { should_not have_css(".js-comment-button[disabled]") } }
+
+ it { within(".js-main-target-form") { should have_css(".js-note-preview-button", visible: true) } }
+ end
+
+ describe "with preview" do
+ before do
+ within(".js-main-target-form") do
+ fill_in "note[note]", with: "This is awesome"
+ find(".js-note-preview-button").trigger("click")
+ end
+ end
+
+ it { within(".js-main-target-form") { should have_css(".js-note-preview", text: "This is awesome", visible: true) } }
+
+ it { within(".js-main-target-form") { should have_css(".js-note-preview-button", visible: false) } }
+ it { within(".js-main-target-form") { should have_css(".js-note-edit-button", visible: true) } }
+ end
+ end
+
+ describe "when posting a note" do
+ before do
+ within(".js-main-target-form") do
+ fill_in "note[note]", with: "This is awsome!"
+ find(".js-note-preview-button").trigger("click")
+ click_button "Add Comment"
+ end
+ end
+
+ # note added
+ it { within(".js-main-target-form") { should have_content("This is awsome!") } }
+
+ # reset form
+ it { within(".js-main-target-form") { should have_no_field("note[note]", with: "This is awesome!") } }
+
+ # return from preview
+ it { within(".js-main-target-form") { should have_css(".js-note-preview", visible: false) } }
+ it { within(".js-main-target-form") { should have_css(".js-note-text", visible: true) } }
+
+
+ it "should be removable" do
+ find(".js-note-delete").trigger("click")
+
+ should_not have_css(".note")
+ end
+ end
+end
+
+
+
+describe "On a merge request diff", js: true, focus: true do
+ let!(:project) { create(:project) }
+ let!(:merge_request) { create(:merge_request_with_diffs, project: project) }
+
+ before do
+ login_as :user
+ project.team << [@user, :master]
+
+ visit diffs_project_merge_request_path(project, merge_request)
+
+ click_link("Diff")
+ end
+
+ subject { page }
+
+ describe "when adding a note" do
+ before do
+ find("#4735dfc552ad7bf15ca468adc3cad9d05b624490_185_185.line_holder .js-add-diff-note-button").trigger("click")
+ end
+
+ describe "the notes holder" do
+ it { should have_css("#4735dfc552ad7bf15ca468adc3cad9d05b624490_185_185.line_holder + .js-temp-notes-holder") }
+
+ it { within(".js-temp-notes-holder") { should have_css(".new_note") } }
+ end
+
+ describe "the note form" do
+ # set up hidden fields correctly
+ it { within(".js-temp-notes-holder") { find("#note_noteable_type").value.should == "MergeRequest" } }
+ it { within(".js-temp-notes-holder") { find("#note_noteable_id").value.should == "" } }
+ it { within(".js-temp-notes-holder") { find("#note_commit_id").value.should == "bcf03b5de6c33f3869ef70d68cf06e679d1d7f9a" } }
+ it { within(".js-temp-notes-holder") { find("#note_line_code").value.should == "4735dfc552ad7bf15ca468adc3cad9d05b624490_185_185" } }
+
+ # buttons
+ it { should have_button("Add Comment") }
+ it { should have_css(".js-close-discussion-note-form", text: "Cancel") }
+
+ # notification options
+ it { should have_unchecked_field("Project team") }
+ it { should have_checked_field("Commit author") }
+
+ it "shouldn't add a second form for same row" do
+ find("#4735dfc552ad7bf15ca468adc3cad9d05b624490_185_185.line_holder .js-add-diff-note-button").trigger("click")
+
+ should have_css("#4735dfc552ad7bf15ca468adc3cad9d05b624490_185_185.line_holder + .js-temp-notes-holder form", count: 1)
+ end
+
+ it "should be removed when canceled" do
+ find(".js-close-discussion-note-form").trigger("click")
+
+ should have_no_css(".js-temp-notes-holder")
+ end
+ end
+ end
+
+ describe "with muliple note forms" do
+ before do
+ find("#4735dfc552ad7bf15ca468adc3cad9d05b624490_185_185.line_holder .js-add-diff-note-button").trigger("click")
+ find("#342e16cbbd482ac2047dc679b2749d248cc1428f_18_17.line_holder .js-add-diff-note-button").trigger("click")
+ end
+
+ # has two line forms
+ it { should have_css(".js-temp-notes-holder", count: 2) }
+
+ describe "previewing them separately" do
+ before do
+ # add two separate texts and trigger previews on both
+ within("#4735dfc552ad7bf15ca468adc3cad9d05b624490_185_185.line_holder + .js-temp-notes-holder") do
+ fill_in "note[note]", with: "One comment on line 185"
+ find(".js-note-preview-button").trigger("click")
+ end
+ within("#342e16cbbd482ac2047dc679b2749d248cc1428f_18_17.line_holder + .js-temp-notes-holder") do
+ fill_in "note[note]", with: "Another comment on line 17"
+ find(".js-note-preview-button").trigger("click")
+ end
+ end
+
+ # check if previews were rendered separately
+ it { within("#4735dfc552ad7bf15ca468adc3cad9d05b624490_185_185.line_holder + .js-temp-notes-holder") { should have_css(".js-note-preview", text: "One comment on line 185") } }
+ it { within("#342e16cbbd482ac2047dc679b2749d248cc1428f_18_17.line_holder + .js-temp-notes-holder") { should have_css(".js-note-preview", text: "Another comment on line 17") } }
+ end
+
+ describe "posting a note" do
+ before do
+ within("#342e16cbbd482ac2047dc679b2749d248cc1428f_18_17.line_holder + .js-temp-notes-holder") do
+ fill_in "note[note]", with: "Another comment on line 17"
+ click_button("Add Comment")
+ end
+ end
+
+ # removed form after submit
+ it { should have_no_css("#342e16cbbd482ac2047dc679b2749d248cc1428f_18_17.line_holder + .js-temp-notes-holder") }
+
+ # added discussion
+ it { should have_content("Another comment on line 17") }
+ it { should have_css("#342e16cbbd482ac2047dc679b2749d248cc1428f_18_17.line_holder + .notes_holder") }
+ it { should have_css("#342e16cbbd482ac2047dc679b2749d248cc1428f_18_17.line_holder + .notes_holder .note", count: 1) }
+ it { should have_link("Reply") }
+
+ it "should remove last note of a discussion" do
+ within("#342e16cbbd482ac2047dc679b2749d248cc1428f_18_17.line_holder + .notes_holder") do
+ find(".js-note-delete").trigger("click")
+ end
+
+ # removed whole discussion
+ should_not have_css(".note_holder")
+ should have_css("#342e16cbbd482ac2047dc679b2749d248cc1428f_18_17.line_holder + #342e16cbbd482ac2047dc679b2749d248cc1428f_18_18.line_holder")
+ end
+ end
+ end
+
+ describe "when replying to a note" do
+ before do
+ # create first note
+ find("#4735dfc552ad7bf15ca468adc3cad9d05b624490_184_184.line_holder .js-add-diff-note-button").trigger("click")
+ within("#4735dfc552ad7bf15ca468adc3cad9d05b624490_184_184.line_holder + .js-temp-notes-holder") do
+ fill_in "note[note]", with: "One comment on line 184"
+ click_button("Add Comment")
+ end
+ # create second note
+ within("#4735dfc552ad7bf15ca468adc3cad9d05b624490_184_184.line_holder + .notes_holder") do
+ find(".js-discussion-reply-button").trigger("click")
+ fill_in "note[note]", with: "An additional comment in reply"
+ click_button("Add Comment")
+ end
+ end
+
+ # inserted note
+ it { should have_content("An additional comment in reply") }
+ it { within("#4735dfc552ad7bf15ca468adc3cad9d05b624490_184_184.line_holder + .notes_holder") { should have_css(".note", count: 2) } }
+
+ # removed form after reply
+ it { within("#4735dfc552ad7bf15ca468adc3cad9d05b624490_184_184.line_holder + .notes_holder") { should have_no_css("form") } }
+ it { within("#4735dfc552ad7bf15ca468adc3cad9d05b624490_184_184.line_holder + .notes_holder") { should have_link("Reply") } }
+ end
+end
+
+
+
+describe "On merge request discussion", js: true do
+ describe "with merge request diff note"
+ describe "with commit note"
+ describe "with commit diff note"
+end
diff --git a/spec/requests/notes_on_wall_spec.rb b/spec/requests/notes_on_wall_spec.rb
new file mode 100644
index 00000000000..c644603d3fa
--- /dev/null
+++ b/spec/requests/notes_on_wall_spec.rb
@@ -0,0 +1,85 @@
+require 'spec_helper'
+
+describe "On the project wall", js: true do
+ let!(:project) { create(:project) }
+ let!(:commit) { project.commit("bcf03b5de6c33f3869ef70d68cf06e679d1d7f9a") }
+
+ before do
+ login_as :user
+ project.team << [@user, :master]
+ visit wall_project_path(project)
+ end
+
+ subject { page }
+
+ describe "the note form" do
+ # main target form creation
+ it { should have_css(".js-main-target-form", visible: true, count: 1) }
+
+ # button initalization
+ it { within(".js-main-target-form") { should have_button("Add Comment") } }
+ it { within(".js-main-target-form") { should_not have_link("Cancel") } }
+
+ # notifiactions
+ it { within(".js-main-target-form") { should have_checked_field("Project team") } }
+ it { within(".js-main-target-form") { should_not have_checked_field("Commit author") } }
+ it { within(".js-main-target-form") { should_not have_unchecked_field("Commit author") } }
+
+ describe "without text" do
+ it { within(".js-main-target-form") { should have_css(".js-note-preview-button", visible: false) } }
+ end
+
+ describe "with text" do
+ before do
+ within(".js-main-target-form") do
+ fill_in "note[note]", with: "This is awesome"
+ end
+ end
+
+ it { within(".js-main-target-form") { should_not have_css(".js-comment-button[disabled]") } }
+
+ it { within(".js-main-target-form") { should have_css(".js-note-preview-button", visible: true) } }
+ end
+
+ describe "with preview" do
+ before do
+ within(".js-main-target-form") do
+ fill_in "note[note]", with: "This is awesome"
+ find(".js-note-preview-button").trigger("click")
+ end
+ end
+
+ it { within(".js-main-target-form") { should have_css(".js-note-preview", text: "This is awesome", visible: true) } }
+
+ it { within(".js-main-target-form") { should have_css(".js-note-preview-button", visible: false) } }
+ it { within(".js-main-target-form") { should have_css(".js-note-edit-button", visible: true) } }
+ end
+ end
+
+ describe "when posting a note" do
+ before do
+ within(".js-main-target-form") do
+ fill_in "note[note]", with: "This is awsome!"
+ find(".js-note-preview-button").trigger("click")
+ click_button "Add Comment"
+ end
+ end
+
+ # note added
+ it { within(".js-main-target-form") { should have_content("This is awsome!") } }
+
+ # reset form
+ it { within(".js-main-target-form") { should have_no_field("note[note]", with: "This is awesome!") } }
+
+ # return from preview
+ it { within(".js-main-target-form") { should have_css(".js-note-preview", visible: false) } }
+ it { within(".js-main-target-form") { should have_css(".js-note-text", visible: true) } }
+
+
+ it "should be removable" do
+ find(".js-note-delete").trigger("click")
+
+ should_not have_css(".note")
+ end
+ end
+end