From e21492b810bf9cd30f0e60836c056a72e830f427 Mon Sep 17 00:00:00 2001
From: dixpac <dino.onex@gmail.com>
Date: Sun, 3 Jul 2016 16:04:22 +0200
Subject: [PATCH] Fix not normalized emoji paths
* There where path where +1 was stored as +1 not as thumbsup
that was causing problems such as showing thumbsup icon 2 time.
I fixed this to always normalize and store +1 as tumbsup
---
CHANGELOG | 1 +
app/models/concerns/awardable.rb | 9 ++++++--
app/models/note.rb | 3 +--
app/services/notes/create_service.rb | 1 -
lib/api/award_emoji.rb | 4 ++--
spec/requests/api/award_emoji_spec.rb | 32 +++++++++++++++++++++++++++
6 files changed, 43 insertions(+), 7 deletions(-)
diff --git a/CHANGELOG b/CHANGELOG
index 19286abd74f00..139444ef70e04 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -74,6 +74,7 @@ v 8.10.0 (unreleased)
- Add basic system information like memory and disk usage to the admin panel
- Don't garbage collect commits that have related DB records like comments
- More descriptive message for git hooks and file locks
+ - Aliases of award emoji should be stored as original name. !5060 (dixpac)
- Handle custom Git hook result in GitLab UI
- Allow '?', or '&' for label names
- Fix importer for GitHub Pull Requests when a branch was reused across Pull Requests
diff --git a/app/models/concerns/awardable.rb b/app/models/concerns/awardable.rb
index 06beff177b17f..800a16ab246c8 100644
--- a/app/models/concerns/awardable.rb
+++ b/app/models/concerns/awardable.rb
@@ -65,8 +65,7 @@ def awarded_emoji?(emoji_name, current_user)
def create_award_emoji(name, current_user)
return unless emoji_awardable?
-
- award_emoji.create(name: name, user: current_user)
+ award_emoji.create(name: normalize_name(name), user: current_user)
end
def remove_award_emoji(name, current_user)
@@ -80,4 +79,10 @@ def toggle_award_emoji(emoji_name, current_user)
create_award_emoji(emoji_name, current_user)
end
end
+
+ private
+
+ def normalize_name(name)
+ Gitlab::AwardEmoji.normalize_emoji_name(name)
+ end
end
diff --git a/app/models/note.rb b/app/models/note.rb
index 8dca2ef09a8a5..0ce10c77de92d 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -229,8 +229,7 @@ def contains_emoji_only?
end
def award_emoji_name
- original_name = note.match(Banzai::Filter::EmojiFilter.emoji_pattern)[1]
- Gitlab::AwardEmoji.normalize_emoji_name(original_name)
+ note.match(Banzai::Filter::EmojiFilter.emoji_pattern)[1]
end
private
diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb
index 02fca5c0ea30c..18971bd0be3ee 100644
--- a/app/services/notes/create_service.rb
+++ b/app/services/notes/create_service.rb
@@ -8,7 +8,6 @@ def execute
if note.award_emoji?
noteable = note.noteable
todo_service.new_award_emoji(noteable, current_user)
-
return noteable.create_award_emoji(note.award_emoji_name, current_user)
end
diff --git a/lib/api/award_emoji.rb b/lib/api/award_emoji.rb
index c4fa1838b5a4b..2efe7e3adf347 100644
--- a/lib/api/award_emoji.rb
+++ b/lib/api/award_emoji.rb
@@ -56,9 +56,9 @@ class AwardEmoji < Grape::API
not_found!('Award Emoji') unless can_read_awardable?
- award = awardable.award_emoji.new(name: params[:name], user: current_user)
+ award = awardable.create_award_emoji(params[:name], current_user)
- if award.save
+ if award.persisted?
present award, with: Entities::AwardEmoji
else
not_found!("Award Emoji #{award.errors.messages}")
diff --git a/spec/requests/api/award_emoji_spec.rb b/spec/requests/api/award_emoji_spec.rb
index 72a6d45f47d92..2b74dd4bbb0c9 100644
--- a/spec/requests/api/award_emoji_spec.rb
+++ b/spec/requests/api/award_emoji_spec.rb
@@ -135,6 +135,22 @@
expect(response).to have_http_status(401)
end
+
+ it "normalizes +1 as thumbsup award" do
+ post api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user), name: '+1'
+
+ expect(issue.award_emoji.last.name).to eq("thumbsup")
+ end
+
+ context 'when the emoji already has been awarded' do
+ it 'returns a 404 status code' do
+ post api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user), name: 'thumbsup'
+ post api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user), name: 'thumbsup'
+
+ expect(response).to have_http_status(404)
+ expect(json_response["message"]).to match("has already been taken")
+ end
+ end
end
end
@@ -147,6 +163,22 @@
expect(response).to have_http_status(201)
expect(json_response['user']['username']).to eq(user.username)
end
+
+ it "normalizes +1 as thumbsup award" do
+ post api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji", user), name: '+1'
+
+ expect(note.award_emoji.last.name).to eq("thumbsup")
+ end
+
+ context 'when the emoji already has been awarded' do
+ it 'returns a 404 status code' do
+ post api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji", user), name: 'rocket'
+ post api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji", user), name: 'rocket'
+
+ expect(response).to have_http_status(404)
+ expect(json_response["message"]).to match("has already been taken")
+ end
+ end
end
describe 'DELETE /projects/:id/awardable/:awardable_id/award_emoji/:award_id' do
--
GitLab