diff --git a/app/controllers/admin/emojis_controller.rb b/app/controllers/admin/emojis_controller.rb index 37d12df4dd6..2674936f6e3 100644 --- a/app/controllers/admin/emojis_controller.rb +++ b/app/controllers/admin/emojis_controller.rb @@ -14,25 +14,50 @@ class Admin::EmojisController < Admin::AdminController .gsub(/_{2,}/, '_') .downcase - data = if Emoji.exists?(name) - failed_json.merge(errors: [I18n.t("emoji.errors.name_already_exists", name: name)]) - elsif emoji = Emoji.create_for(file, name) - emoji - else - failed_json.merge(errors: [I18n.t("emoji.errors.error_while_storing_emoji")]) - end + upload = Upload.create_for( + current_user.id, + file.tempfile, + file.original_filename, + File.size(file.tempfile.path), + image_type: 'custom_emoji' + ) + + data = + if upload.persisted? + custom_emoji = CustomEmoji.new(name: name, upload: upload) + + if custom_emoji.save + Emoji.clear_cache + { name: custom_emoji.name, url: custom_emoji.upload.url } + else + failed_json.merge(errors: custom_emoji.errors.full_messages) + end + else + failed_json.merge(errors: upload.errors.full_messages) + end MessageBus.publish("/uploads/emoji", data.as_json, user_ids: [current_user.id]) end - render json: success_json end def destroy name = params.require(:id) - Emoji[name].try(:remove) - render nothing: true + + custom_emoji = CustomEmoji.find_by(name: name) + raise Discourse::InvalidParameters unless custom_emoji + + CustomEmoji.transaction do + custom_emoji.upload.destroy! + custom_emoji.destroy! + end + + Emoji.clear_cache + + Jobs.enqueue(:rebake_custom_emoji_posts, name: name) + + render json: success_json end end diff --git a/app/jobs/onceoff/migrate_custom_emojis.rb b/app/jobs/onceoff/migrate_custom_emojis.rb new file mode 100644 index 00000000000..5fcdc867e2c --- /dev/null +++ b/app/jobs/onceoff/migrate_custom_emojis.rb @@ -0,0 +1,35 @@ +module Jobs + class MigrateCustomEmojis < Jobs::Onceoff + def execute_onceoff(args) + return if Rails.env.test? + + CustomEmoji.transaction do + Dir["#{Rails.root}/#{Emoji.base_directory}/*.{png,gif}"].each do |path| + name = File.basename(path, File.extname(path)) + + File.open(path) do |file| + upload = Upload.create_for( + Discourse.system_user.id, + file, + File.basename(path), + file.size, + image_type: 'custom_emoji' + ) + + if upload.persisted? + CustomEmoji.create!(name: name, upload: upload) + else + raise "Failed to create upload for '#{name}' custom emoji" + end + end + end + + Emoji.clear_cache + + Post.where("cooked LIKE '%#{Emoji.base_url}%'").find_each do |post| + post.rebake! + end + end + end + end +end diff --git a/app/jobs/regular/rebake_custom_emoji_posts.rb b/app/jobs/regular/rebake_custom_emoji_posts.rb new file mode 100644 index 00000000000..d1afbc87324 --- /dev/null +++ b/app/jobs/regular/rebake_custom_emoji_posts.rb @@ -0,0 +1,8 @@ +module Jobs + class RebakeCustomEmojiPosts < Jobs::Base + def execute(args) + name = args[:name] + Post.where("raw LIKE '%:#{name}:%'").find_each { |post| post.rebake! } + end + end +end diff --git a/app/jobs/regular/resize_emoji.rb b/app/jobs/regular/resize_emoji.rb deleted file mode 100644 index fa30e629e7e..00000000000 --- a/app/jobs/regular/resize_emoji.rb +++ /dev/null @@ -1,18 +0,0 @@ -module Jobs - - class ResizeEmoji < Jobs::Base - - def execute(args) - path = args[:path] - return unless File.exists?(path) - - opts = { - allow_animation: true, - force_aspect_ratio: SiteSetting.enforce_square_emoji - } - # make sure emoji aren't too big - OptimizedImage.downsize(path, path, "100x100", opts) - end - end - -end diff --git a/app/jobs/scheduled/clean_up_uploads.rb b/app/jobs/scheduled/clean_up_uploads.rb index 9a12757609c..263e7563b7e 100644 --- a/app/jobs/scheduled/clean_up_uploads.rb +++ b/app/jobs/scheduled/clean_up_uploads.rb @@ -22,11 +22,13 @@ module Jobs .joins("LEFT JOIN user_avatars ua ON (ua.gravatar_upload_id = uploads.id OR ua.custom_upload_id = uploads.id)") .joins("LEFT JOIN user_profiles up ON up.profile_background = uploads.url OR up.card_background = uploads.url") .joins("LEFT JOIN categories c ON c.uploaded_logo_id = uploads.id OR c.uploaded_background_id = uploads.id") + .joins("LEFT JOIN custom_emojis ce ON ce.upload_id = uploads.id") .where("pu.upload_id IS NULL") .where("u.uploaded_avatar_id IS NULL") .where("ua.gravatar_upload_id IS NULL AND ua.custom_upload_id IS NULL") .where("up.profile_background IS NULL AND up.card_background IS NULL") .where("c.uploaded_logo_id IS NULL AND c.uploaded_background_id IS NULL") + .where("ce.upload_id IS NULL") .where("uploads.url NOT IN (?)", ignore_urls) result.find_each do |upload| diff --git a/app/models/custom_emoji.rb b/app/models/custom_emoji.rb new file mode 100644 index 00000000000..bdc2795846b --- /dev/null +++ b/app/models/custom_emoji.rb @@ -0,0 +1,6 @@ +class CustomEmoji < ActiveRecord::Base + belongs_to :upload + + validates :name, presence: true, uniqueness: true + validates :upload_id, presence: true +end diff --git a/app/models/emoji.rb b/app/models/emoji.rb index c58cff9dd32..7fb7e905caf 100644 --- a/app/models/emoji.rb +++ b/app/models/emoji.rb @@ -14,14 +14,6 @@ class Emoji @path = path end - def remove - return if path.blank? - if File.exists?(path) - File.delete(path) rescue nil - Emoji.clear_cache - end - end - def self.all Discourse.cache.fetch(cache_key("all_emojis")) { standard | custom } end @@ -46,14 +38,6 @@ class Emoji Emoji.custom.detect { |e| e.name == name } end - def self.create_from_path(path) - extension = File.extname(path) - Emoji.new(path).tap do |e| - e.name = File.basename(path, ".*") - e.url = "#{base_url}/#{e.name}#{extension}" - end - end - def self.create_from_db_item(emoji) name = emoji["name"] filename = "#{emoji['filename'] || name}.png" @@ -63,22 +47,6 @@ class Emoji end end - def self.create_for(file, name) - extension = File.extname(file.original_filename) - path = "#{Emoji.base_directory}/#{name}#{extension}" - full_path = "#{Rails.root}/#{path}" - - # store the emoji - FileUtils.mkdir_p(Pathname.new(path).dirname) - File.open(path, "wb") { |f| f << file.tempfile.read } - # clear the cache - Emoji.clear_cache - # launch resize job - Jobs.enqueue(:resize_emoji, path: full_path) - # return created emoji - Emoji[name] - end - def self.cache_key(name) "#{name}:#{EMOJI_VERSION}:#{Plugin::CustomEmoji.cache_key}" end @@ -124,9 +92,12 @@ class Emoji def self.load_custom result = [] - Dir.glob(File.join(Emoji.base_directory, "*.{png,gif}")) - .sort - .each { |emoji| result << Emoji.create_from_path(emoji) } + CustomEmoji.all.each do |emoji| + result << Emoji.new.tap do |e| + e.name = emoji.name + e.url = emoji.upload.url + end + end Plugin::CustomEmoji.emojis.each do |name, url| result << Emoji.new.tap do |e| diff --git a/app/models/upload.rb b/app/models/upload.rb index d6119abc13c..1200d2487eb 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -57,7 +57,12 @@ class Upload < ActiveRecord::Base end # list of image types that will be cropped - CROPPED_IMAGE_TYPES ||= %w{avatar profile_background card_background} + CROPPED_IMAGE_TYPES ||= %w{ + avatar + profile_background + card_background + custom_emoji + } WHITELISTED_SVG_ELEMENTS ||= %w{ circle @@ -92,7 +97,7 @@ class Upload < ActiveRecord::Base # options # - content_type # - origin (url) - # - image_type ("avatar", "profile_background", "card_background") + # - image_type ("avatar", "profile_background", "card_background", "custom_emoji") # - is_attachment_for_group_message (boolean) def self.create_for(user_id, file, filename, filesize, options = {}) upload = Upload.new @@ -145,6 +150,8 @@ class Upload < ActiveRecord::Base max_width = 590 * max_pixel_ratio width, height = ImageSizer.resize(w, h, max_width: max_width, max_height: max_width) OptimizedImage.downsize(file.path, file.path, "#{width}x#{height}", filename: filename, allow_animation: allow_animation) + when "custom_emoji" + OptimizedImage.downsize(file.path, file.path, "100x100", filename: filename, allow_animation: allow_animation) end end diff --git a/config/initializers/100-wrap_parameters.rb b/config/initializers/100-wrap_parameters.rb index 02e47b12b8a..999df20181e 100644 --- a/config/initializers/100-wrap_parameters.rb +++ b/config/initializers/100-wrap_parameters.rb @@ -12,4 +12,3 @@ end ActiveSupport.on_load(:active_record) do self.include_root_in_json = false end - diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 1619309dd96..b8e1d9711bb 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -391,6 +391,10 @@ en: attributes: payload_url: invalid: "URL is invalid. URL should includes http:// or https://. And no blank is allowed." + custom_emoji: + attributes: + name: + taken: is already in use by another emoji user_profile: no_info_me: "
the About Me field of your profile is currently blank, would you like to fill it out?
" @@ -1574,11 +1578,6 @@ en: post_revision_text: "Ownership transferred from %{old_user} to %{new_user}" deleted_user: "a deleted user" - emoji: - errors: - name_already_exists: "Sorry, the name '%{name}' is already used by another emoji." - error_while_storing_emoji: "Sorry, there has been an error while storing the emoji." - topic_statuses: archived_enabled: "This topic is now archived. It is frozen and cannot be changed in any way." archived_disabled: "This topic is now unarchived. It is no longer frozen, and can be changed." diff --git a/db/migrate/20170201085745_create_custom_emojis.rb b/db/migrate/20170201085745_create_custom_emojis.rb new file mode 100644 index 00000000000..15590fba611 --- /dev/null +++ b/db/migrate/20170201085745_create_custom_emojis.rb @@ -0,0 +1,12 @@ +class CreateCustomEmojis < ActiveRecord::Migration + def change + create_table :custom_emojis do |t| + t.string :name, null: false + t.integer :upload_id, null: false + + t.timestamps null: false + end + + add_index :custom_emojis, :name, unique: true + end +end diff --git a/lib/tasks/db.rake b/lib/tasks/db.rake index b460d550c6a..0c88c62c55c 100644 --- a/lib/tasks/db.rake +++ b/lib/tasks/db.rake @@ -6,6 +6,7 @@ end # we need to run seed_fu every time we run rake db:migrate task 'db:migrate' => ['environment', 'set_locale'] do SeedFu.seed + Jobs::Onceoff.enqueue_all end task 'test:prepare' => 'environment' do diff --git a/spec/components/pretty_text_spec.rb b/spec/components/pretty_text_spec.rb index 3a409471f4e..0a1e7f8a3c6 100644 --- a/spec/components/pretty_text_spec.rb +++ b/spec/components/pretty_text_spec.rb @@ -431,7 +431,7 @@ HTML describe "custom emoji" do it "replaces the custom emoji" do - Emoji.stubs(:custom).returns([ Emoji.create_from_path('trout') ]) + CustomEmoji.create!(name: 'trout', upload: Fabricate(:upload)) expect(PrettyText.cook("hello :trout:")).to match(/]+trout[^>]+>/) end end diff --git a/spec/controllers/admin/emojis_controller_spec.rb b/spec/controllers/admin/emojis_controller_spec.rb index fa776bb8921..5b769ab90ad 100644 --- a/spec/controllers/admin/emojis_controller_spec.rb +++ b/spec/controllers/admin/emojis_controller_spec.rb @@ -16,10 +16,6 @@ describe Admin::EmojisController do end end - it "is a subclass of AdminController" do - expect(Admin::EmojisController < Admin::AdminController).to eq(true) - end - context "when logged in" do let!(:user) { log_in(:admin) } @@ -33,56 +29,6 @@ describe Admin::EmojisController do expect(json[0]["url"]).to eq(custom_emoji.url) end end - - context ".create" do - - before { Emoji.expects(:custom).returns([custom_emoji]) } - - context "name already exist" do - it "throws an error" do - message = MessageBus.track_publish do - xhr :post, :create, { name: "hello", file: "" } - end.first - - expect(response).to be_success - expect(message.data["errors"]).to be - end - end - - context "error while saving emoji" do - it "throws an error" do - Emoji.expects(:create_for).returns(nil) - message = MessageBus.track_publish do - xhr :post, :create, { name: "garbage", file: "" } - end.first - - expect(response).to be_success - expect(message.data["errors"]).to be - end - end - - it "works" do - Emoji.expects(:create_for).returns(custom_emoji2) - - message = MessageBus.track_publish do - xhr :post, :create, { name: "hello2", file: ""} - end.first - - expect(response).to be_success - - expect(message.data["name"]).to eq(custom_emoji2.name) - expect(message.data["url"]).to eq(custom_emoji2.url) - end - end - - context ".destroy" do - it "deletes the custom emoji" do - custom_emoji.expects(:remove) - Emoji.expects(:custom).returns([custom_emoji]) - xhr :delete, :destroy, id: "hello" - expect(response).to be_success - end - end end end diff --git a/spec/integration/admin/emojis_spec.rb b/spec/integration/admin/emojis_spec.rb new file mode 100644 index 00000000000..5ea92ea6f3d --- /dev/null +++ b/spec/integration/admin/emojis_spec.rb @@ -0,0 +1,76 @@ +require 'rails_helper' + +RSpec.describe "Managing custom emojis" do + let(:admin) { Fabricate(:admin) } + let(:upload) { Fabricate(:upload) } + + before do + sign_in(admin) + end + + describe "creating a custom emoji" do + describe 'when upload is invalid' do + it 'should publish the right error' do + message = MessageBus.track_publish do + post("/admin/customize/emojis.json", { + name: 'test', + file: fixture_file_upload("#{Rails.root}/spec/fixtures/images/fake.jpg") + }) + end.first + + expect(message.channel).to eq("/uploads/emoji") + expect(message.data["errors"]).to eq([I18n.t('upload.images.size_not_found')]) + end + end + + describe 'when emoji name already exists' do + it 'should publish the right error' do + CustomEmoji.create!(name: 'test', upload: upload) + + message = MessageBus.track_publish do + post("/admin/customize/emojis.json", { + name: 'test', + file: fixture_file_upload("#{Rails.root}/spec/fixtures/images/logo.png") + }) + end.first + + expect(message.channel).to eq("/uploads/emoji") + + expect(message.data["errors"]).to eq([ + "Name #{I18n.t('activerecord.errors.models.custom_emoji.attributes.name.taken')}" + ]) + end + end + + it 'should allow an admin to add a custom emoji' do + Emoji.expects(:clear_cache) + + message = MessageBus.track_publish do + post("/admin/customize/emojis.json", { + name: 'test', + file: fixture_file_upload("#{Rails.root}/spec/fixtures/images/logo.png") + }) + end.first + + custom_emoji = CustomEmoji.last + upload = custom_emoji.upload + + expect(upload.original_filename).to eq('logo.png') + expect(message.channel).to eq("/uploads/emoji") + expect(message.data["errors"]).to eq(nil) + expect(message.data["name"]).to eq(custom_emoji.name) + expect(message.data["url"]).to eq(upload.url) + end + end + + describe 'deleting a custom emoji' do + it 'should allow an admin to delete a custom emoji' do + custom_emoji = CustomEmoji.create!(name: 'test', upload: upload) + Emoji.clear_cache + + expect { delete "/admin/customize/emojis/#{custom_emoji.name}.json", name: 'test' } + .to change { Upload.count }.by(-1) + .and change { CustomEmoji.count }.by(-1) + end + end +end diff --git a/spec/jobs/clean_up_uploads_spec.rb b/spec/jobs/clean_up_uploads_spec.rb index 7f0bf461ae0..e9f649cbe16 100644 --- a/spec/jobs/clean_up_uploads_spec.rb +++ b/spec/jobs/clean_up_uploads_spec.rb @@ -140,4 +140,14 @@ describe Jobs::CleanUpUploads do expect(Upload.find_by(id: upload.id)).to eq(upload) end + it "does not delete custom emojis" do + upload = fabricate_upload + CustomEmoji.create!(name: 'test', upload: upload) + + Jobs::CleanUpUploads.new.execute(nil) + + expect(Upload.find_by(id: @upload.id)).to eq(nil) + expect(Upload.find_by(id: upload.id)).to eq(upload) + end + end diff --git a/spec/jobs/rebake_custom_emoji_posts_spec.rb b/spec/jobs/rebake_custom_emoji_posts_spec.rb new file mode 100644 index 00000000000..68b554ab33c --- /dev/null +++ b/spec/jobs/rebake_custom_emoji_posts_spec.rb @@ -0,0 +1,19 @@ +require 'rails_helper' + +RSpec.describe Jobs::RebakeCustomEmojiPosts do + it 'should rebake posts that are using a given custom emoji' do + custom_emoji = CustomEmoji.create!(name: 'test', upload: Fabricate(:upload)) + Emoji.clear_cache + post = Fabricate(:post, raw: 'some post with :test: yay') + + expect(post.reload.cooked).to eq( + "

some post with \":test:\" yay

" + ) + + custom_emoji.destroy! + Emoji.clear_cache + described_class.new.execute(name: 'test') + + expect(post.reload.cooked).to eq('

some post with :test: yay

') + end +end