From 74007f2e306dfc7a42a333351cc103b4a2f38378 Mon Sep 17 00:00:00 2001 From: Donovan Daniels Date: Mon, 4 Dec 2023 13:56:14 -0600 Subject: [PATCH] [Blips] Add Updater & Tests/Move modactions to callbacks (#540) * [Blips] Add `updater_id` & update notice * [Blips] Move modaction generation logic into rails callbacks * [Tests] Add blip tests * Move update notice to right place * Assert amount of difference * Rebase oopsie --------- Co-authored-by: Earlopain <14981592+Earlopain@users.noreply.github.com> --- app/controllers/blips_controller.rb | 18 +-- app/models/blip.rb | 20 ++++ app/views/blips/partials/show/_blip.html.erb | 1 + .../20230531080817_add_blips_updater_id.rb | 6 + db/structure.sql | 14 ++- test/factories/blip.rb | 1 - test/unit/blip_test.rb | 106 ++++++++++++++++++ test/unit/paginator_test.rb | 5 + 8 files changed, 153 insertions(+), 18 deletions(-) create mode 100644 db/migrate/20230531080817_add_blips_updater_id.rb create mode 100644 test/unit/blip_test.rb diff --git a/app/controllers/blips_controller.rb b/app/controllers/blips_controller.rb index 50c13892f..1fac4506e 100644 --- a/app/controllers/blips_controller.rb +++ b/app/controllers/blips_controller.rb @@ -29,10 +29,7 @@ class BlipsController < ApplicationController def update @blip = Blip.find(params[:id]) check_edit_privilege(@blip) - Blip.transaction do - @blip.update(blip_params(:update)) - ModAction.log(:blip_update, { blip_id: @blip.id, user_id: @blip.creator_id }) if CurrentUser.user != @blip.creator - end + @blip.update(blip_params(:update)) flash[:notice] = 'Blip updated' respond_with(@blip) end @@ -40,27 +37,18 @@ class BlipsController < ApplicationController def hide @blip = Blip.find(params[:id]) check_hide_privilege(@blip) - - Blip.transaction do - @blip.update(is_hidden: true) - ModAction.log(:blip_hide, { blip_id: @blip.id, user_id: @blip.creator_id }) if CurrentUser.user != @blip.creator - end + @blip.hide! respond_with(@blip) end def unhide @blip = Blip.find(params[:id]) - Blip.transaction do - @blip.update(is_hidden: false) - ModAction.log(:blip_unhide, {blip_id: @blip.id, user_id: @blip.creator_id}) - end + @blip.unhide! respond_with(@blip) end def destroy @blip = Blip.find(params[:id]) - - ModAction.log(:blip_delete, {blip_id: @blip.id, user_id: @blip.creator_id}) @blip.destroy flash[:notice] = 'Blip deleted' respond_with(@blip) do |format| diff --git a/app/models/blip.rb b/app/models/blip.rb index 6ad526c03..654e6ea56 100644 --- a/app/models/blip.rb +++ b/app/models/blip.rb @@ -2,11 +2,23 @@ class Blip < ApplicationRecord include UserWarnable simple_versioning belongs_to_creator + belongs_to_updater optional: true validates :body, presence: true validates :body, length: { minimum: 5, maximum: Danbooru.config.blip_max_size } validate :validate_parent_exists, on: :create validate :validate_creator_is_not_limited, on: :create + after_update(if: ->(rec) { !rec.saved_change_to_is_hidden? && CurrentUser.id != rec.creator_id }) do |rec| + ModAction.log(:blip_update, { blip_id: rec.id, user_id: rec.creator_id }) + end + after_destroy do |rec| + ModAction.log(:blip_delete, { blip_id: rec.id, user_id: rec.creator_id }) + end + after_save(if: ->(rec) { rec.saved_change_to_is_hidden? && CurrentUser.id != rec.creator_id }) do |rec| + action = rec.is_hidden? ? :blip_hide : :blip_unhide + ModAction.log(action, { blip_id: rec.id, user_id: rec.creator_id }) + end + user_status_counter :blip_count belongs_to :parent, class_name: "Blip", foreign_key: "response_to", optional: true belongs_to :warning_user, class_name: "User", optional: true @@ -104,4 +116,12 @@ class Blip < ApplicationRecord include PermissionsMethods extend SearchMethods include ApiMethods + + def hide! + update(is_hidden: true) + end + + def unhide! + update(is_hidden: false) + end end diff --git a/app/views/blips/partials/show/_blip.html.erb b/app/views/blips/partials/show/_blip.html.erb index 36f551a79..9ae45903a 100644 --- a/app/views/blips/partials/show/_blip.html.erb +++ b/app/views/blips/partials/show/_blip.html.erb @@ -23,6 +23,7 @@
<%= format_text(blip.body, allow_color: blip.creator.is_privileged?) %>
+ <%= render "application/update_notice", record: blip %> <%= render "application/warned_notice", record: blip %>
diff --git a/db/migrate/20230531080817_add_blips_updater_id.rb b/db/migrate/20230531080817_add_blips_updater_id.rb new file mode 100644 index 000000000..ff9457c03 --- /dev/null +++ b/db/migrate/20230531080817_add_blips_updater_id.rb @@ -0,0 +1,6 @@ +class AddBlipsUpdaterId < ActiveRecord::Migration[7.0] + def change + add_column :blips, :updater_id, :integer + add_foreign_key :blips, :users, column: :updater_id + end +end diff --git a/db/structure.sql b/db/structure.sql index 480c9129a..b7b2f1d21 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -251,7 +251,8 @@ CREATE TABLE public.blips ( created_at timestamp without time zone NOT NULL, updated_at timestamp without time zone NOT NULL, warning_type integer, - warning_user_id integer + warning_user_id integer, + updater_id integer ); @@ -4420,6 +4421,14 @@ ALTER TABLE ONLY public.staff_audit_logs ADD CONSTRAINT fk_rails_02329e5ef9 FOREIGN KEY (user_id) REFERENCES public.users(id); +-- +-- Name: blips fk_rails_23e7479aac; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.blips + ADD CONSTRAINT fk_rails_23e7479aac FOREIGN KEY (updater_id) REFERENCES public.users(id); + + -- -- Name: tickets fk_rails_45cd696dba; Type: FK CONSTRAINT; Schema: public; Owner: - -- @@ -4733,6 +4742,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20230506161827'), ('20230513074838'), ('20230517155547'), -('20230518182034'); +('20230518182034'), +('20230531080817'); diff --git a/test/factories/blip.rb b/test/factories/blip.rb index 8dc2c807b..504caa01f 100644 --- a/test/factories/blip.rb +++ b/test/factories/blip.rb @@ -1,6 +1,5 @@ FactoryBot.define do factory(:blip) do - creator creator_ip_addr { "127.0.0.1" } sequence(:body) { |n| "blip_body_#{n}" } end diff --git a/test/unit/blip_test.rb b/test/unit/blip_test.rb new file mode 100644 index 000000000..86f80be36 --- /dev/null +++ b/test/unit/blip_test.rb @@ -0,0 +1,106 @@ +require "test_helper" + +class BlipTest < ActiveSupport::TestCase + context "A blip" do + setup do + @user = create(:user) + CurrentUser.user = @user + end + + context "created by a limited user" do + setup do + Danbooru.config.stubs(:disable_throttles?).returns(false) + end + + should "fail creation" do + blip = build(:blip) + blip.save + assert_equal(["Creator can not yet perform this action. Account is too new"], blip.errors.full_messages) + end + end + + context "created by an unlimited user" do + setup do + Danbooru.config.stubs(:blip_limit).returns(100) + end + + should "be created" do + blip = build(:blip) + blip.save + assert(blip.errors.empty?, blip.errors.full_messages.join(", ")) + end + + should "be searchable" do + b1 = create(:blip, body: "aaa bbb ccc") + b2 = create(:blip, body: "aaa ddd") + + matches = Blip.search(body_matches: "aaa") + assert_equal(2, matches.count) + assert_equal(b2.id, matches.all[0].id) + assert_equal(b1.id, matches.all[1].id) + end + + should "default to id_desc order when searched with no options specified" do + blips = create_list(:blip, 3) + matches = Blip.search({}) + + assert_equal([blips[2].id, blips[1].id, blips[0].id], matches.map(&:id)) + end + + context "that is edited by a moderator" do + setup do + @blip = create(:blip) + @mod = create(:moderator_user) + CurrentUser.user = @mod + end + + should "create a mod action" do + assert_difference(-> { ModAction.count }, 1) do + @blip.update(body: "nopearino") + end + end + + should "credit the moderator as the updater" do + @blip.update(body: "testing") + assert_equal(@mod.id, @blip.updater_id) + end + end + + context "that is hidden by a moderator" do + setup do + @blip = create(:blip) + @mod = create(:moderator_user) + CurrentUser.user = @mod + end + + should "create a mod action" do + assert_difference(-> { ModAction.count }, 1) do + @blip.update(is_hidden: true) + end + end + + should "credit the moderator as the updater" do + @blip.update(is_hidden: true) + assert_equal(@mod.id, @blip.updater_id) + end + end + + context "that is deleted" do + setup do + @blip = create(:blip) + end + + should "create a mod action" do + assert_difference(-> { ModAction.count }, 1) do + @blip.destroy + end + end + end + end + + context "during validation" do + subject { build(:blip) } + should_not allow_value(" ").for(:body) + end + end +end diff --git a/test/unit/paginator_test.rb b/test/unit/paginator_test.rb index e12f37dbc..c74f08a0f 100644 --- a/test/unit/paginator_test.rb +++ b/test/unit/paginator_test.rb @@ -10,6 +10,11 @@ class PaginatorTest < ActiveSupport::TestCase { active_record: Blip, opensearch: Post }.each do |name, model| # rubocop:disable Metrics/BlockLength context name do + setup do + @user = create(:user) + CurrentUser.user = @user + end + context "sequential pagination (before)" do should "return the correct set of records" do @records = create_list(model.name.underscore, 4)