tags: track tag histories.

Track the history of the tag `category` and `is_deprecated` fields in
the `tag_versions` table.

Adds generic Versionable and VersionFor concerns that encapsulate most
of the history tracking logic. These concerns are designed to make it
easy to add history to any model.

There are a couple notable differences between tag versions and other versions:

* There is no 1 hour edit merge window. All changes to the `category`
  and `is_deprecated` fields produce a new version in the tag history.

* New versions aren't created when a tag is created. Versions are only
  created when a tag is edited for the first time. The tag's initial
  version isn't created until *after* the tag is edited for the first time.

For example, if you change the category of a tag that was last updated
10 years ago, that will create an initial version of the tag backdated
to 10 years ago, plus a new version for your edit.

This is for a few reasons:

* So that we don't have to create new tag versions every time a new tag
  is created. This would be wasteful because most tags never have their
  category or deprecation status change.
* So that if you make a typo tag, your name isn't recorded in the tag's
  history forever.
* So that we can create new tags in various places without having to know
  who created the tag (which may be unknown if the current user isn't set).
* Because we don't know the full history of most tags, so we have to
  deal with incomplete histories anyway.

This has a few important consequences:

* Most tags won't have any tag versions. They only gain tag versions if
  they're edited.
* You can't track /tag_versions to see newly created tags. It only
  shows changes to already existing tags.
* Tag version IDs won't be in strict chronological order. Higher IDs may
  have created_at timestamps before lower IDs. For example, if you
  change the category of a tag that is 10 years old, that will create an
  initial version with a high ID, but with a created_at timestamp dated
  to 10 years ago.

Fixes #4402: Track tag category changes
This commit is contained in:
evazion
2022-09-09 22:16:59 -05:00
parent 0c327a2228
commit 54a45a3021
12 changed files with 388 additions and 24 deletions

View File

@@ -92,10 +92,24 @@ class TagsControllerTest < ActionDispatch::IntegrationTest
@mod = create(:moderator_user)
end
should "update the tag" do
put_auth tag_path(@tag), @user, params: {:tag => {:category => Tag.categories.general}}
should "update the category for an empty tag" do
@tag = create(:tag, category: Tag.categories.copyright, post_count: 0)
put_auth tag_path(@tag), @user, params: { tag: { category: Tag.categories.general }}
assert_redirected_to tag_path(@tag)
assert_equal(Tag.categories.general, @tag.reload.category)
assert_equal(2, @tag.versions.count)
assert_equal(1, @tag.first_version.version)
assert_equal(@tag.created_at, @tag.first_version.created_at)
assert_equal(@tag.created_at, @tag.first_version.updated_at)
assert_nil(@tag.first_version.updater)
assert_equal(Tag.categories.copyright, @tag.first_version.category)
assert_equal(2, @tag.last_version.version)
assert_equal(@user, @tag.last_version.updater)
assert_equal(Tag.categories.general, @tag.last_version.category)
end
context "for a tag with >50 posts" do
@@ -109,6 +123,7 @@ class TagsControllerTest < ActionDispatch::IntegrationTest
assert_response 403
assert_not_equal(Tag.categories.general, @tag.reload.category)
assert_equal(0, @tag.versions.count)
end
should "update the category for a builder" do
@@ -116,6 +131,12 @@ class TagsControllerTest < ActionDispatch::IntegrationTest
assert_redirected_to @tag
assert_equal(Tag.categories.general, @tag.reload.category)
assert_equal(2, @tag.versions.count)
assert_nil(@tag.first_version.updater)
assert_equal(@user, @tag.last_version.updater)
assert_equal(Tag.categories.copyright, @tag.first_version.category)
assert_equal(Tag.categories.general, @tag.last_version.category)
end
end
@@ -139,6 +160,7 @@ class TagsControllerTest < ActionDispatch::IntegrationTest
assert_response 403
assert_equal(true, @deprecated_tag.reload.is_deprecated?)
assert_equal(0, @tag.versions.count)
end
should "remove the deprecated status if the user is admin" do
@@ -146,6 +168,12 @@ class TagsControllerTest < ActionDispatch::IntegrationTest
assert_redirected_to @deprecated_tag
assert_equal(false, @deprecated_tag.reload.is_deprecated?)
assert_equal(2, @deprecated_tag.versions.count)
assert_nil(@deprecated_tag.first_version.updater)
assert_equal(@admin, @deprecated_tag.last_version.updater)
assert_equal(true, @deprecated_tag.first_version.is_deprecated)
assert_equal(false, @deprecated_tag.last_version.is_deprecated)
end
should "allow marking a tag as deprecated if it's empty" do
@@ -153,6 +181,12 @@ class TagsControllerTest < ActionDispatch::IntegrationTest
assert_redirected_to @nondeprecated_tag
assert_equal(true, @nondeprecated_tag.reload.is_deprecated?)
assert_equal(2, @nondeprecated_tag.versions.count)
assert_nil(@nondeprecated_tag.first_version.updater)
assert_equal(@normal_user, @nondeprecated_tag.last_version.updater)
assert_equal(false, @nondeprecated_tag.first_version.is_deprecated)
assert_equal(true, @nondeprecated_tag.last_version.is_deprecated)
end
should "not allow marking a tag as deprecated if it's not empty" do
@@ -160,6 +194,7 @@ class TagsControllerTest < ActionDispatch::IntegrationTest
assert_response 403
assert_equal(false, @normal_tag.reload.is_deprecated?)
assert_equal(0, @normal_tag.versions.count)
end
should "allow admins to mark tags as deprecated" do
@@ -167,6 +202,12 @@ class TagsControllerTest < ActionDispatch::IntegrationTest
assert_redirected_to @normal_tag
assert_equal(true, @normal_tag.reload.is_deprecated?)
assert_equal(2, @normal_tag.versions.count)
assert_nil(@normal_tag.first_version.updater)
assert_equal(@admin, @normal_tag.last_version.updater)
assert_equal(false, @normal_tag.first_version.is_deprecated)
assert_equal(true, @normal_tag.last_version.is_deprecated)
end
should "not allow deprecation of a tag with no wiki" do
@@ -174,15 +215,17 @@ class TagsControllerTest < ActionDispatch::IntegrationTest
assert_response 403
assert_equal(false, @tag_without_wiki.reload.is_deprecated?)
assert_equal(0, @tag_without_wiki.versions.count)
end
end
should "not change category when the tag is too large to be changed by a builder" do
@tag.update(category: Tag.categories.general, post_count: 1001)
@tag = create(:tag, category: Tag.categories.general, post_count: 1001)
put_auth tag_path(@tag), @user, params: {:tag => {:category => Tag.categories.artist}}
assert_response :forbidden
assert_equal(Tag.categories.general, @tag.reload.category)
assert_equal(0, @tag.versions.count)
end
end
end

View File

@@ -128,14 +128,12 @@ module PostSets
end
context "that has a matching artist" do
setup do
Tag.find_by(name: "a").update!(category: Tag.categories.artist)
@artist = FactoryBot.create(:artist, :name => "a")
end
should "find the artist" do
assert_not_nil(@set.artist)
assert_equal(@artist.id, @set.artist.id)
set = PostSets::Post.new("bkub")
artist = create(:artist, name: "bkub")
assert_not_nil(set.artist)
assert_equal(artist, set.artist)
end
end
end

View File

@@ -345,6 +345,22 @@ class PostTest < ActiveSupport::TestCase
@post = FactoryBot.create(:post)
end
context "with a new tag" do
should "create the new tag" do
tag1 = create(:tag, name: "foo", post_count: 100, category: Tag.categories.character)
create(:post, tag_string: "foo bar")
tag2 = Tag.find_by_name("bar")
assert_equal(101, tag1.reload.post_count)
assert_equal(Tag.categories.character, tag1.category)
assert_equal(0, tag1.versions.count)
assert_equal(1, tag2.post_count)
assert_equal(Tag.categories.general, tag2.category)
assert_equal(0, tag2.versions.count)
end
end
context "with a banned artist" do
setup do
CurrentUser.scoped(FactoryBot.create(:admin_user)) do
@@ -490,7 +506,7 @@ class PostTest < ActiveSupport::TestCase
should "not remove the tag if the tag was already in the post" do
bad_tag = create(:tag, name: "bad_tag")
old_post = create(:post, tag_string: "bad_tag")
bad_tag.update!(is_deprecated: true)
bad_tag.update!(is_deprecated: true, updater: create(:user))
old_post.update!(tag_string: "asd bad_tag")
assert_equal("asd bad_tag", old_post.reload.tag_string)
@@ -525,9 +541,30 @@ class PostTest < ActiveSupport::TestCase
context "tagged with a metatag" do
context "for a tag category prefix" do
should "set the category of a new tag" do
create(:post, tag_string: "char:hoge")
create(:post, tag_string: "char:chen")
tag = Tag.find_by_name("chen")
assert_equal(Tag.categories.character, Tag.find_by_name("hoge").category)
assert_equal(Tag.categories.character, tag.category)
assert_equal(0, tag.versions.count)
end
should "change the category of an existing tag" do
user = create(:user)
tag = create(:tag, name: "hoge", post_count: 1)
post = as(user) { create(:post, tag_string: "char:hoge") }
assert_equal(Tag.categories.character, tag.reload.category)
assert_equal(2, tag.versions.count)
assert_equal(1, tag.first_version.version)
assert_nil(tag.first_version.updater)
assert_nil(tag.first_version.previous_version)
assert_equal(Tag.categories.general, tag.first_version.category)
assert_equal(2, tag.last_version.version)
assert_equal(user, tag.last_version.updater)
assert_equal(tag.first_version, tag.last_version.previous_version)
assert_equal(Tag.categories.character, tag.last_version.category)
end
should "change the category for an aliased tag" do

View File

@@ -72,7 +72,7 @@ class TagTest < ActiveSupport::TestCase
tag = FactoryBot.create(:artist_tag)
assert_equal(Tag.categories.artist, Cache.get("tc:#{Cache.hash(tag.name)}"))
tag.update_attribute(:category, Tag.categories.copyright)
tag.update!(category: Tag.categories.copyright, updater: create(:user))
assert_equal(Tag.categories.copyright, Cache.get("tc:#{Cache.hash(tag.name)}"))
end
@@ -81,6 +81,70 @@ class TagTest < ActiveSupport::TestCase
end
end
context "When a tag is created" do
should "not create a new version" do
tag = create(:tag, category: Tag.categories.character)
assert_equal(0, tag.versions.count)
end
end
context "When a tag is updated" do
should "create the initial version before the new version" do
user = create(:user)
tag = create(:tag, created_at: 1.year.ago, updated_at: 6.months.ago)
tag.update!(updater: user, category: Tag.categories.character, is_deprecated: true)
assert_equal(2, tag.versions.count)
assert_equal(1, tag.first_version.version)
assert_equal(tag.updated_at_before_last_save.round(4), tag.first_version.created_at.round(4))
assert_equal(tag.updated_at_before_last_save.round(4), tag.first_version.updated_at.round(4))
assert_nil(tag.first_version.updater)
assert_nil(tag.first_version.previous_version)
assert_equal(Tag.categories.general, tag.first_version.category)
assert_equal(false, tag.first_version.is_deprecated)
assert_equal(2, tag.last_version.version)
assert_equal(user, tag.last_version.updater)
assert_equal(tag.first_version, tag.last_version.previous_version)
assert_equal(Tag.categories.character, tag.last_version.category)
assert_equal(true, tag.last_version.is_deprecated)
end
end
context "When a tag is updated twice by the same user" do
should "not merge the edits" do
updated_at = 6.months.ago
user = create(:user)
tag = create(:tag, created_at: 1.year.ago, updated_at: updated_at)
travel_to(1.minute.ago) { tag.update!(updater: user, category: Tag.categories.character, is_deprecated: true) }
tag.update!(updater: user, category: Tag.categories.copyright)
assert_equal(3, tag.versions.count)
assert_equal(1, tag.versions[0].version)
assert_equal(updated_at.round(4), tag.versions[0].created_at.round(4))
assert_equal(updated_at.round(4), tag.versions[0].updated_at.round(4))
assert_nil(tag.versions[0].updater)
assert_nil(tag.versions[0].previous_version)
assert_equal(Tag.categories.general, tag.versions[0].category)
assert_equal(false, tag.versions[0].is_deprecated)
assert_equal(2, tag.versions[1].version)
assert_equal(user, tag.versions[1].updater)
assert_equal(tag.versions[0], tag.versions[1].previous_version)
assert_equal(Tag.categories.character, tag.versions[1].category)
assert_equal(true, tag.versions[1].is_deprecated)
assert_equal(3, tag.versions[2].version)
assert_equal(user, tag.versions[2].updater)
assert_equal(tag.versions[1], tag.versions[2].previous_version)
assert_equal(Tag.categories.copyright, tag.versions[2].category)
assert_equal(true, tag.versions[2].is_deprecated)
end
end
context "A tag" do
should "be found when one exists" do
tag = FactoryBot.create(:tag)