From 4741a52cc4695f4e1d70da98df6c2b38c727cdb6 Mon Sep 17 00:00:00 2001 From: evazion Date: Tue, 1 Dec 2020 16:01:30 -0600 Subject: [PATCH] aliases/implications: remove 'error' state. Remove the error status from aliases and implications. Aliases and implications normally shouldn't fail because they're validated beforehand. If they do, just let the delayed job itself record the failure. Also disable the delayed job from retrying if the alias/implication somehow fails. --- app/jobs/process_tag_relationship_job.rb | 1 + app/logical/d_text.rb | 2 -- app/models/tag_alias.rb | 3 --- app/models/tag_implication.rb | 3 --- app/models/tag_relationship.rb | 6 +----- test/unit/tag_alias_test.rb | 1 - test/unit/tag_implication_test.rb | 1 - 7 files changed, 2 insertions(+), 15 deletions(-) diff --git a/app/jobs/process_tag_relationship_job.rb b/app/jobs/process_tag_relationship_job.rb index 01b22a233..14940c6d6 100644 --- a/app/jobs/process_tag_relationship_job.rb +++ b/app/jobs/process_tag_relationship_job.rb @@ -1,5 +1,6 @@ class ProcessTagRelationshipJob < ApplicationJob queue_as :bulk_update + retry_on Exception, attempts: 0 def perform(class_name:, approver:, antecedent_name:, consequent_name:, forum_topic: nil) relation_class = Kernel.const_get(class_name) diff --git a/app/logical/d_text.rb b/app/logical/d_text.rb index db4d4c7d5..37b3189c3 100644 --- a/app/logical/d_text.rb +++ b/app/logical/d_text.rb @@ -98,8 +98,6 @@ class DText "The #{obj.relationship} ##{obj.id} [[#{obj.antecedent_name}]] -> [[#{obj.consequent_name}]] has been rejected." elsif obj.is_pending? "The #{obj.relationship} ##{obj.id} [[#{obj.antecedent_name}]] -> [[#{obj.consequent_name}]] is pending approval." - elsif obj.is_errored? - "The #{obj.relationship} ##{obj.id} [[#{obj.antecedent_name}]] -> [[#{obj.consequent_name}]] (#{relationship} failed during processing." else # should never happen "The #{obj.relationship} ##{obj.id} [[#{obj.antecedent_name}]] -> [[#{obj.consequent_name}]] has an unknown status." end diff --git a/app/models/tag_alias.rb b/app/models/tag_alias.rb index 72fee2ccb..f3884ef03 100644 --- a/app/models/tag_alias.rb +++ b/app/models/tag_alias.rb @@ -12,9 +12,6 @@ class TagAlias < TagRelationship def process! TagMover.new(antecedent_name, consequent_name, user: User.system).move! - rescue Exception => e - update!(status: "error: #{e}") - DanbooruLogger.log(e, tag_alias_id: id, antecedent_name: antecedent_name, consequent_name: consequent_name) end def absence_of_transitive_relation diff --git a/app/models/tag_implication.rb b/app/models/tag_implication.rb index 052dac101..f8dc34bbe 100644 --- a/app/models/tag_implication.rb +++ b/app/models/tag_implication.rb @@ -113,9 +113,6 @@ class TagImplication < TagRelationship CurrentUser.scoped(User.system) do update_posts end - rescue Exception => e - update(status: "error: #{e}") - DanbooruLogger.log(e, tag_implication_id: id, antecedent_name: antecedent_name, consequent_name: consequent_name) end def create_mod_action diff --git a/app/models/tag_relationship.rb b/app/models/tag_relationship.rb index a1cab6891..3693f84cb 100644 --- a/app/models/tag_relationship.rb +++ b/app/models/tag_relationship.rb @@ -20,7 +20,7 @@ class TagRelationship < ApplicationRecord scope :retired, -> {where(status: "retired")} before_validation :normalize_names - validates_format_of :status, :with => /\A(active|deleted|retired|error: .*)\Z/ + validates :status, inclusion: { in: %w[active deleted retired] } validates_presence_of :antecedent_name, :consequent_name validates :approver, presence: { message: "must exist" }, if: -> { approver_id.present? } validates :forum_topic, presence: { message: "must exist" }, if: -> { forum_topic_id.present? } @@ -47,10 +47,6 @@ class TagRelationship < ApplicationRecord status == "active" end - def is_errored? - status =~ /\Aerror:/ - end - def reject! update!(status: "deleted") end diff --git a/test/unit/tag_alias_test.rb b/test/unit/tag_alias_test.rb index 87dfe6228..9e1b77b2e 100644 --- a/test/unit/tag_alias_test.rb +++ b/test/unit/tag_alias_test.rb @@ -26,7 +26,6 @@ class TagAliasTest < ActiveSupport::TestCase should allow_value('active').for(:status) should allow_value('deleted').for(:status) - should allow_value('error: derp').for(:status) should_not allow_value('ACTIVE').for(:status) should_not allow_value('error').for(:status) diff --git a/test/unit/tag_implication_test.rb b/test/unit/tag_implication_test.rb index f284d489b..99a84de10 100644 --- a/test/unit/tag_implication_test.rb +++ b/test/unit/tag_implication_test.rb @@ -22,7 +22,6 @@ class TagImplicationTest < ActiveSupport::TestCase should allow_value('active').for(:status) should allow_value('deleted').for(:status) - should allow_value('error: derp').for(:status) should_not allow_value('ACTIVE').for(:status) should_not allow_value('error').for(:status)