summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGrzegorz Bizon <grzegorz@gitlab.com>2015-12-07 12:26:56 +0000
committerGrzegorz Bizon <grzegorz@gitlab.com>2015-12-07 12:26:56 +0000
commit359d94607c2df324bc5cd9591bded05dbe9ca157 (patch)
tree1ab19b405514e2e980bb241dd379bb1a981b8932
parent104df74f512dd7174a49504dda0b0910ee43b787 (diff)
parent893d08c0dc6a1eba14db7694636707f30b28a7f4 (diff)
downloadgitlab-ce-359d94607c2df324bc5cd9591bded05dbe9ca157.tar.gz
Merge branch 'fix/award-emoji-conflict-in-notes' into 'master'
Fix problems with award-emoji-only comment This fixes a conflict between note with only a single emoji in content and award-emojis mechanisms. Closes #3734 cc @vsizov See merge request !1936
-rw-r--r--app/assets/javascripts/awards_handler.coffee7
-rw-r--r--app/assets/javascripts/flash.js.coffee16
-rw-r--r--app/assets/javascripts/notes.js.coffee7
-rw-r--r--app/assets/stylesheets/framework/flash.scss10
-rw-r--r--app/controllers/projects/notes_controller.rb29
-rw-r--r--app/models/note.rb30
-rw-r--r--app/services/notes/create_service.rb13
-rw-r--r--features/project/commits/diff_comments.feature6
-rw-r--r--features/project/issues/award_emoji.feature6
-rw-r--r--features/steps/project/issues/award_emoji.rb31
-rw-r--r--features/steps/shared/diff_note.rb17
11 files changed, 129 insertions, 43 deletions
diff --git a/app/assets/javascripts/awards_handler.coffee b/app/assets/javascripts/awards_handler.coffee
index 09b48fe5572..96fd8f8773e 100644
--- a/app/assets/javascripts/awards_handler.coffee
+++ b/app/assets/javascripts/awards_handler.coffee
@@ -88,4 +88,9 @@ class @AwardsHandler
callback.call()
findEmojiIcon: (emoji) ->
- $(".icon[data-emoji='" + emoji + "']") \ No newline at end of file
+ $(".icon[data-emoji='" + emoji + "']")
+
+ scrollToAwards: ->
+ $('body, html').animate({
+ scrollTop: $('.awards').offset().top - 80
+ }, 200)
diff --git a/app/assets/javascripts/flash.js.coffee b/app/assets/javascripts/flash.js.coffee
index b39ab0c4475..9b59d4e57f7 100644
--- a/app/assets/javascripts/flash.js.coffee
+++ b/app/assets/javascripts/flash.js.coffee
@@ -1,12 +1,16 @@
class @Flash
constructor: (message, type)->
- flash = $(".flash-container")
- flash.html("")
+ @flash = $(".flash-container")
+ @flash.html("")
- $('<div/>',
+ innerDiv = $('<div/>',
class: "flash-#{type}",
text: message
- ).appendTo(".flash-container")
+ )
+ innerDiv.appendTo(".flash-container")
- flash.click -> $(@).fadeOut()
- flash.show()
+ @flash.click -> $(@).fadeOut()
+ @flash.show()
+
+ pin: ->
+ @flash.addClass('flash-pinned flash-raised')
diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee
index 7de7632201d..dd6cbcfc70b 100644
--- a/app/assets/javascripts/notes.js.coffee
+++ b/app/assets/javascripts/notes.js.coffee
@@ -111,6 +111,12 @@ class @Notes
Note: for rendering inline notes use renderDiscussionNote
###
renderNote: (note) ->
+ unless note.valid
+ if note.award
+ flash = new Flash('You have already used this award emoji!', 'alert')
+ flash.pin()
+ return
+
# render note if it not present in loaded list
# or skip if rendered
if @isNewNote(note) && !note.award
@@ -122,6 +128,7 @@ class @Notes
if note.award
awards_handler.addAwardToEmojiBar(note.note, note.emoji_path)
+ awards_handler.scrollToAwards()
###
Check if note does not exists on page
diff --git a/app/assets/stylesheets/framework/flash.scss b/app/assets/stylesheets/framework/flash.scss
index 82eb50ad4be..1b723021d76 100644
--- a/app/assets/stylesheets/framework/flash.scss
+++ b/app/assets/stylesheets/framework/flash.scss
@@ -15,3 +15,13 @@
@extend .alert-danger;
}
}
+
+.flash-pinned {
+ position: fixed;
+ top: 80px;
+ width: 80%;
+}
+
+.flash-raised {
+ z-index: 1000;
+}
diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb
index 5ac18446aa7..88b949a27ab 100644
--- a/app/controllers/projects/notes_controller.rb
+++ b/app/controllers/projects/notes_controller.rb
@@ -131,16 +131,25 @@ class Projects::NotesController < Projects::ApplicationController
end
def render_note_json(note)
- render json: {
- id: note.id,
- discussion_id: note.discussion_id,
- html: note_to_html(note),
- award: note.is_award,
- emoji_path: note.is_award ? view_context.image_url(::AwardEmoji.path_to_emoji_image(note.note)) : "",
- note: note.note,
- discussion_html: note_to_discussion_html(note),
- discussion_with_diff_html: note_to_discussion_with_diff_html(note)
- }
+ if note.valid?
+ render json: {
+ valid: true,
+ id: note.id,
+ discussion_id: note.discussion_id,
+ html: note_to_html(note),
+ award: note.is_award,
+ emoji_path: note.is_award ? view_context.image_url(::AwardEmoji.path_to_emoji_image(note.note)) : "",
+ note: note.note,
+ discussion_html: note_to_discussion_html(note),
+ discussion_with_diff_html: note_to_discussion_with_diff_html(note)
+ }
+ else
+ render json: {
+ valid: false,
+ award: note.is_award,
+ errors: note.errors
+ }
+ end
end
def authorize_admin_note!
diff --git a/app/models/note.rb b/app/models/note.rb
index 1c6345e735c..239a0f77f8e 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -39,8 +39,11 @@ class Note < ActiveRecord::Base
delegate :name, to: :project, prefix: true
delegate :name, :email, to: :author, prefix: true
+ before_validation :set_award!
+
validates :note, :project, presence: true
validates :note, uniqueness: { scope: [:author, :noteable_type, :noteable_id] }, if: ->(n) { n.is_award }
+ validates :note, inclusion: { in: Emoji.emojis_names }, if: ->(n) { n.is_award }
validates :line_code, format: { with: /\A[a-z0-9]+_\d+_\d+\Z/ }, allow_blank: true
# Attachments are deprecated and are handled by Markdown uploader
validates :attachment, file_size: { maximum: :max_attachment_size }
@@ -348,4 +351,31 @@ class Note < ActiveRecord::Base
def editable?
!system?
end
+
+ # Checks if note is an award added as a comment
+ #
+ # If note is an award, this method sets is_award to true
+ # and changes content of the note to award name.
+ #
+ # Method is executed as a before_validation callback.
+ #
+ def set_award!
+ return unless awards_supported? && contains_emoji_only?
+ self.is_award = true
+ self.note = award_emoji_name
+ end
+
+ private
+
+ def awards_supported?
+ noteable.kind_of?(Issue) || noteable.is_a?(MergeRequest)
+ end
+
+ def contains_emoji_only?
+ note =~ /\A#{Gitlab::Markdown::EmojiFilter.emoji_pattern}\s?\Z/
+ end
+
+ def award_emoji_name
+ note.match(Gitlab::Markdown::EmojiFilter.emoji_pattern)[1]
+ end
end
diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb
index dbff58dfb9c..a8486e6a5a1 100644
--- a/app/services/notes/create_service.rb
+++ b/app/services/notes/create_service.rb
@@ -5,11 +5,6 @@ module Notes
note.author = current_user
note.system = false
- if contains_emoji_only?(params[:note])
- note.is_award = true
- note.note = emoji_name(params[:note])
- end
-
if note.save
notification_service.new_note(note)
@@ -33,13 +28,5 @@ module Notes
note.project.execute_hooks(note_data, :note_hooks)
note.project.execute_services(note_data, :note_hooks)
end
-
- def contains_emoji_only?(note)
- note =~ /\A:[-_+[:alnum:]]*:\s?\z/
- end
-
- def emoji_name(note)
- note.match(/\A:([-_+[:alnum:]]*):\s?/)[1]
- end
end
end
diff --git a/features/project/commits/diff_comments.feature b/features/project/commits/diff_comments.feature
index 4a2b870e082..d6e0c84537e 100644
--- a/features/project/commits/diff_comments.feature
+++ b/features/project/commits/diff_comments.feature
@@ -14,6 +14,12 @@ Feature: Project Commits Diff Comments
Then I should see a diff comment saying "Typo, please fix"
@javascript
+ Scenario: I can add a diff comment with a single emoji
+ Given I open a diff comment form
+ And I write a diff comment like ":smile:"
+ Then I should see a diff comment with an emoji image
+
+ @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
diff --git a/features/project/issues/award_emoji.feature b/features/project/issues/award_emoji.feature
index a9bc8ffb9bb..2609f129d07 100644
--- a/features/project/issues/award_emoji.feature
+++ b/features/project/issues/award_emoji.feature
@@ -11,4 +11,8 @@ Feature: Award Emoji
And I click to emoji in the picker
Then I have award added
And I can remove it by clicking to icon
- \ No newline at end of file
+
+ @javascript
+ Scenario: I add award emoji using regular comment
+ Given I leave comment with a single emoji
+ Then I have award added
diff --git a/features/steps/project/issues/award_emoji.rb b/features/steps/project/issues/award_emoji.rb
index 8f7a45dec0e..325eaf2ea6a 100644
--- a/features/steps/project/issues/award_emoji.rb
+++ b/features/steps/project/issues/award_emoji.rb
@@ -9,33 +9,40 @@ class Spinach::Features::AwardEmoji < Spinach::FeatureSteps
end
step 'I click to emoji-picker' do
- page.within ".awards-controls" do
- page.find(".add-award").click
+ page.within '.awards-controls' do
+ page.find('.add-award').click
end
end
step 'I click to emoji in the picker' do
- page.within ".awards-menu" do
- page.first("img").click
+ page.within '.awards-menu' do
+ page.first('img').click
end
end
step 'I can remove it by clicking to icon' do
- page.within ".awards" do
- page.first(".award").click
- expect(page).to_not have_selector ".award"
+ page.within '.awards' do
+ page.first('.award').click
+ expect(page).to_not have_selector '.award'
end
end
step 'I have award added' do
- page.within ".awards" do
- expect(page).to have_selector ".award"
- expect(page.find(".award .counter")).to have_content "1"
+ page.within '.awards' do
+ expect(page).to have_selector '.award'
+ expect(page.find('.award .counter')).to have_content '1'
end
end
step 'project "Shop" has issue "Bugfix"' do
- @project = Project.find_by(name: "Shop")
- @issue = create(:issue, title: "Bugfix", project: project)
+ @project = Project.find_by(name: 'Shop')
+ @issue = create(:issue, title: 'Bugfix', project: project)
+ end
+
+ step 'I leave comment with a single emoji' do
+ page.within('.js-main-target-form') do
+ fill_in 'note[note]', with: ':smile:'
+ click_button 'Add Comment'
+ end
end
end
diff --git a/features/steps/shared/diff_note.rb b/features/steps/shared/diff_note.rb
index 72621911a37..dd466cde28d 100644
--- a/features/steps/shared/diff_note.rb
+++ b/features/steps/shared/diff_note.rb
@@ -87,6 +87,17 @@ module SharedDiffNote
end
end
+ step 'I write a diff comment like ":smile:"' do
+ page.within(diff_file_selector) do
+ click_diff_line(sample_commit.line_code)
+
+ page.within("form[rel$='#{sample_commit.line_code}']") do
+ fill_in 'note[note]', with: ':smile:'
+ click_button('Add Comment')
+ end
+ end
+ end
+
step 'I submit the diff comment' do
page.within(diff_file_selector) do
click_button("Add Comment")
@@ -197,6 +208,12 @@ module SharedDiffNote
end
end
+ step 'I should see a diff comment with an emoji image' do
+ page.within("#{diff_file_selector} .note") do
+ expect(page).to have_xpath("//img[@alt=':smile:']")
+ end
+ end
+
step 'I click side-by-side diff button' do
find('#parallel-diff-btn').trigger('click')
end