From 66c8c1f53f8781a8285f2eea4b9d85c6107f89c9 Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Mon, 4 May 2020 02:54:10 -0500
Subject: [PATCH] artists: fix artist bans not being recorded in artist
 history.

Using update_column bypasses callbacks, so a new artist version wasn't
created when the is_banned flag was changed.
---
 app/models/artist.rb                       |  4 ++--
 test/functional/artists_controller_test.rb |  2 +-
 test/unit/artist_test.rb                   | 16 ++++++++++++----
 3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/app/models/artist.rb b/app/models/artist.rb
index fbc565c9a..f4d2d0c7e 100644
--- a/app/models/artist.rb
+++ b/app/models/artist.rb
@@ -178,7 +178,7 @@ class Artist < ApplicationRecord
             post.update(tag_string: fixed_tags)
           end
 
-          update_column(:is_banned, false)
+          update!(is_banned: false)
           ModAction.log("unbanned artist ##{id}", :artist_unban)
         end
       end
@@ -195,7 +195,7 @@ class Artist < ApplicationRecord
             tag_implication.approve!(approver: banner)
           end
 
-          update_column(:is_banned, true)
+          update!(is_banned: true)
           ModAction.log("banned artist ##{id}", :artist_ban)
         end
       end
diff --git a/test/functional/artists_controller_test.rb b/test/functional/artists_controller_test.rb
index 7da27b5be..c4f1714e6 100644
--- a/test/functional/artists_controller_test.rb
+++ b/test/functional/artists_controller_test.rb
@@ -111,7 +111,7 @@ class ArtistsControllerTest < ActionDispatch::IntegrationTest
 
     context "unban action" do
       should "unban an artist" do
-        @artist.ban!(banner: @admin)
+        as(@admin) { @artist.ban!(banner: @admin) }
         put_auth unban_artist_path(@artist.id), @admin
 
         assert_redirected_to(@artist)
diff --git a/test/unit/artist_test.rb b/test/unit/artist_test.rb
index 1f9e447d7..8f632b512 100644
--- a/test/unit/artist_test.rb
+++ b/test/unit/artist_test.rb
@@ -74,13 +74,17 @@ class ArtistTest < ActiveSupport::TestCase
       end
 
       should "allow unbanning" do
+        assert_equal(true, @artist.reload.is_banned?)
+        assert_equal(true, @post.reload.is_banned?)
+        assert_equal(true, @artist.versions.last.is_banned?)
+
         assert_difference("TagImplication.count", -1) do
           @artist.unban!
         end
-        @post.reload
-        @artist.reload
-        assert(!@artist.is_banned?, "artist should not be banned")
-        assert(!@post.is_banned?, "post should not be banned")
+
+        assert_equal(false, @artist.reload.is_banned?)
+        assert_equal(false, @post.reload.is_banned?)
+        assert_equal(false, @artist.versions.last.is_banned?)
         assert_equal("aaa", @post.tag_string)
       end
 
@@ -102,6 +106,10 @@ class ArtistTest < ActiveSupport::TestCase
         ta = TagImplication.where(:antecedent_name => "aaa", :consequent_name => "banned_artist").first
         assert_equal(@admin.id, ta.approver.id)
       end
+
+      should "update the artist history" do
+        assert_equal(true, @artist.versions.last.is_banned?)
+      end
     end
 
     should "normalize its name" do