From eede2f075233fe8b292ebf70514242d84c467480 Mon Sep 17 00:00:00 2001 From: evazion Date: Mon, 18 Dec 2017 17:10:55 -0600 Subject: [PATCH 1/3] Fix #3324: Incorporate replacement comment info in the replacement history. --- app/models/post_replacement.rb | 20 ++++++++++++++++--- ...13037_add_metadata_to_post_replacements.rb | 17 ++++++++++++++++ db/structure.sql | 14 ++++++++++++- test/unit/post_replacement_test.rb | 19 +++++++++++++++++- 4 files changed, 65 insertions(+), 5 deletions(-) create mode 100644 db/migrate/20171218213037_add_metadata_to_post_replacements.rb diff --git a/app/models/post_replacement.rb b/app/models/post_replacement.rb index e92cac981..19df9392f 100644 --- a/app/models/post_replacement.rb +++ b/app/models/post_replacement.rb @@ -3,13 +3,19 @@ class PostReplacement < ApplicationRecord belongs_to :post belongs_to :creator, class_name: "User" - before_validation :initialize_fields + before_validation :initialize_fields, on: :create attr_accessor :replacement_file, :final_source, :tags def initialize_fields self.creator = CurrentUser.user self.original_url = post.source self.tags = post.tag_string + " " + self.tags.to_s + + self.file_ext_was = post.file_ext + self.file_size_was = post.file_size + self.image_width_was = post.image_width + self.image_height_was = post.image_height + self.md5_was = post.md5 end def undo! @@ -32,9 +38,9 @@ class PostReplacement < ApplicationRecord md5_changed = (upload.md5 != post.md5) if replacement_file.present? - update(replacement_url: "file://#{replacement_file.original_filename}") + self.replacement_url = "file://#{replacement_file.original_filename}" else - update(replacement_url: upload.downloaded_source) + self.replacement_url = upload.downloaded_source end # queue the deletion *before* updating the post so that we use the old @@ -44,6 +50,12 @@ class PostReplacement < ApplicationRecord Post.delay(queue: "default", run_at: Time.now + DELETION_GRACE_PERIOD).delete_files(post.id, post.file_path, post.large_file_path, post.preview_file_path) end + self.file_ext = upload.file_ext + self.file_size = upload.file_size + self.image_height = upload.image_height + self.image_width = upload.image_width + self.md5 = upload.md5 + post.md5 = upload.md5 post.file_ext = upload.file_ext post.image_width = upload.image_width @@ -51,6 +63,7 @@ class PostReplacement < ApplicationRecord post.file_size = upload.file_size post.source = final_source.presence || upload.source post.tag_string = upload.tag_string + rescale_notes update_ugoira_frame_data(upload) @@ -60,6 +73,7 @@ class PostReplacement < ApplicationRecord post.queue_backup end + save! post.save! end diff --git a/db/migrate/20171218213037_add_metadata_to_post_replacements.rb b/db/migrate/20171218213037_add_metadata_to_post_replacements.rb new file mode 100644 index 000000000..1a2e9becd --- /dev/null +++ b/db/migrate/20171218213037_add_metadata_to_post_replacements.rb @@ -0,0 +1,17 @@ +class AddMetadataToPostReplacements < ActiveRecord::Migration + def change + PostReplacement.without_timeout do + add_column :post_replacements, :file_ext_was, :string + add_column :post_replacements, :file_size_was, :integer + add_column :post_replacements, :image_width_was, :integer + add_column :post_replacements, :image_height_was, :integer + add_column :post_replacements, :md5_was, :string + + add_column :post_replacements, :file_ext, :string + add_column :post_replacements, :file_size, :integer + add_column :post_replacements, :image_width, :integer + add_column :post_replacements, :image_height, :integer + add_column :post_replacements, :md5, :string + end + end +end diff --git a/db/structure.sql b/db/structure.sql index bd655ffd4..4a7dba3ad 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -2666,7 +2666,17 @@ CREATE TABLE post_replacements ( original_url text NOT NULL, replacement_url text NOT NULL, created_at timestamp without time zone NOT NULL, - updated_at timestamp without time zone NOT NULL + updated_at timestamp without time zone NOT NULL, + file_ext_was character varying, + file_size_was integer, + image_width_was integer, + image_height_was integer, + md5_was character varying, + file_ext character varying, + file_size integer, + image_width integer, + image_height integer, + md5 character varying ); @@ -7530,3 +7540,5 @@ INSERT INTO schema_migrations (version) VALUES ('20171106075030'); INSERT INTO schema_migrations (version) VALUES ('20171127195124'); +INSERT INTO schema_migrations (version) VALUES ('20171218213037'); + diff --git a/test/unit/post_replacement_test.rb b/test/unit/post_replacement_test.rb index 3785bafea..d71327795 100644 --- a/test/unit/post_replacement_test.rb +++ b/test/unit/post_replacement_test.rb @@ -30,7 +30,7 @@ class PostReplacementTest < ActiveSupport::TestCase def teardown super - + CurrentUser.user = nil CurrentUser.ip_addr = nil Delayed::Worker.delay_jobs = false @@ -49,6 +49,7 @@ class PostReplacementTest < ActiveSupport::TestCase setup do @post.update(source: "https://www.google.com/images/branding/googlelogo/1x/googlelogo_color_272x92dp.png") @post.replace!(replacement_url: "https://www.google.com/intl/en_ALL/images/logo.gif", tags: "-tag1 tag2") + @replacement = @post.replacements.last @upload = Upload.last end @@ -79,6 +80,22 @@ class PostReplacementTest < ActiveSupport::TestCase assert_equal(@post.id, PostReplacement.last.post_id) end + should "record the old file metadata" do + assert_equal(500, @replacement.image_width_was) + assert_equal(335, @replacement.image_height_was) + assert_equal(28086, @replacement.file_size_was) + assert_equal("jpg", @replacement.file_ext_was) + assert_equal("ecef68c44edb8a0d6a3070b5f8e8ee76", @replacement.md5_was) + end + + should "record the new file metadata" do + assert_equal(276, @replacement.image_width) + assert_equal(110, @replacement.image_height) + assert_equal(8558, @replacement.file_size) + assert_equal("gif", @replacement.file_ext) + assert_equal("e80d1c59a673f560785784fb1ac10959", @replacement.md5) + end + should "correctly update the attributes" do assert_equal(@post.id, @upload.post.id) assert_equal("completed", @upload.status) From e6acd6f2d6aa9c73d10a2e144b70d72bc7947cc1 Mon Sep 17 00:00:00 2001 From: evazion Date: Mon, 18 Dec 2017 17:51:27 -0600 Subject: [PATCH 2/3] /post_replacements: list old and new md5 and image sizes. --- app/views/post_replacements/index.html.erb | 33 ++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/app/views/post_replacements/index.html.erb b/app/views/post_replacements/index.html.erb index 8ad4e9b6f..356aec3a0 100644 --- a/app/views/post_replacements/index.html.erb +++ b/app/views/post_replacements/index.html.erb @@ -15,6 +15,8 @@ Post Source + MD5 + Size Replacer @@ -36,6 +38,37 @@ + + + <% if post_replacement.md5_was.present? && post_replacement.md5.present? %> +
+
Original MD5
+
<%= post_replacement.md5_was %>
+ +
Replacement MD5
+
<%= post_replacement.md5 %>
+
+ <% end %> + + + + <% if %i[image_width_was image_height_was file_size_was file_ext_was image_width image_height file_size file_ext].all? { |k| post_replacement[k].present? } %> +
+
Original Size
+
+ <%= post_replacement.image_width_was %>x<%= post_replacement.image_height_was %> + (<%= post_replacement.file_size_was.to_s(:human_size, precision: 4) %>, <%= post_replacement.file_ext_was %>) +
+ +
Replacement Size
+
+ <%= post_replacement.image_width %>x<%= post_replacement.image_height %> + (<%= post_replacement.file_size.to_s(:human_size, precision: 4) %>, <%= post_replacement.file_ext %>) +
+
+ <% end %> + + <%= compact_time post_replacement.created_at %>
by <%= link_to_user post_replacement.creator %> From fa941e94807d5fca740b127dd85dbf0c5b3a54e5 Mon Sep 17 00:00:00 2001 From: evazion Date: Mon, 18 Dec 2017 17:57:05 -0600 Subject: [PATCH 3/3] /post_replacements: allow updating image metadata in past replacements. --- .../post_replacements_controller.rb | 15 +++++++++++++++ config/routes.rb | 2 +- .../post_replacements_controller_test.rb | 19 +++++++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/app/controllers/post_replacements_controller.rb b/app/controllers/post_replacements_controller.rb index 6b66cffe7..eebbedd81 100644 --- a/app/controllers/post_replacements_controller.rb +++ b/app/controllers/post_replacements_controller.rb @@ -14,6 +14,13 @@ class PostReplacementsController < ApplicationController respond_with(@post_replacement, location: @post) end + def update + @post_replacement = PostReplacement.find(params[:id]) + @post_replacement.update(update_params) + + respond_with(@post_replacement) + end + def index params[:search][:post_id] = params.delete(:post_id) if params.has_key?(:post_id) @post_replacements = PostReplacement.search(params[:search]).paginate(params[:page], limit: params[:limit]) @@ -25,4 +32,12 @@ private def create_params params.require(:post_replacement).permit(:replacement_url, :replacement_file, :final_source, :tags) end + + def update_params + params.require(:post_replacement).permit( + :file_ext_was, :file_size_was, :image_width_was, :image_height_was, :md5_was, + :file_ext, :file_size, :image_width, :image_height, :md5, + :original_url, :replacement_url + ) + end end diff --git a/config/routes.rb b/config/routes.rb index 9e8e17021..b2d602646 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -195,7 +195,7 @@ Rails.application.routes.draw do get :diff end end - resources :post_replacements, :only => [:index, :new, :create] + resources :post_replacements, :only => [:index, :new, :create, :update] resources :posts do resources :events, :only => [:index], :controller => "post_events" resources :replacements, :only => [:index, :new, :create], :controller => "post_replacements" diff --git a/test/functional/post_replacements_controller_test.rb b/test/functional/post_replacements_controller_test.rb index 84b9399a5..c4d05597e 100644 --- a/test/functional/post_replacements_controller_test.rb +++ b/test/functional/post_replacements_controller_test.rb @@ -43,6 +43,25 @@ class PostReplacementsControllerTest < ActionController::TestCase end end + context "update action" do + should "update the replacement" do + params = { + format: :json, + id: @post_replacement.id, + post_replacement: { + file_size_was: 23, + file_size: 42, + } + } + + put :update, params, { user_id: @user.id } + @post_replacement.reload + + assert_equal(23, @post_replacement.file_size_was) + assert_equal(42, @post_replacement.file_size) + end + end + context "index action" do should "render" do get :index, {format: :json}