close
Skip to content

Fix audit log error when custom emoji is copied from remote server#11876

Merged
Gargron merged 1 commit into
mastodon:masterfrom
highemerly:fix-log-action-when-copy-customemoji
Sep 17, 2019
Merged

Fix audit log error when custom emoji is copied from remote server#11876
Gargron merged 1 commit into
mastodon:masterfrom
highemerly:fix-log-action-when-copy-customemoji

Conversation

@highemerly
Copy link
Copy Markdown
Contributor

Expected behaviour

When I copy custom emojis from remote server,

  • Custom emoji can be copied.
  • This action is recorded to audit logs.

Actual behaviour

When I copy custom emojis from remote server,

  • Custom emoji can be copied.
  • This action is NOT recorded to audit logs.

Steps to reproduce the problem

In admin/custom_emojis pages, check some custom emoji and click "copy" button.

Specifications

master branch

Comment thread app/models/form/custom_emoji_batch.rb Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to check why output of custom_emoji.copy! doesn't work, but this would be wrong, since it will log the creation of the remote emoji, and not the local one...

Copy link
Copy Markdown
Contributor Author

@highemerly highemerly Sep 17, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand your concern. It seems that the output of custom_emoji.copy! is just true/false. Following changes are acceptable for you? I'll send pull request later.

diff --git a/app/models/custom_emoji.rb b/app/models/custom_emoji.rb
index edb1bec..0dacaf6 100644
--- a/app/models/custom_emoji.rb
+++ b/app/models/custom_emoji.rb
@@ -63,7 +63,7 @@ class CustomEmoji < ApplicationRecord
   def copy!
     copy = self.class.find_or_initialize_by(domain: nil, shortcode: shortcode)
     copy.image = image
-    copy.save!
+    copy.tap(&:save!)
   end

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes, that is a good fix! I've run into that mistake a few times before

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sent new fix.

@highemerly highemerly force-pushed the fix-log-action-when-copy-customemoji branch from bfec420 to 725115a Compare September 17, 2019 15:40
@Gargron Gargron merged commit 3919571 into mastodon:master Sep 17, 2019
hiyuki2578 pushed a commit to ProjectMyosotis/mastodon that referenced this pull request Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants