From ea4e7f197001dc2d699ce702a129e8a3cbc6bc9e Mon Sep 17 00:00:00 2001 From: Albert Yi Date: Fri, 27 Jul 2018 17:56:28 -0700 Subject: [PATCH] Expire unused aliases and implications after 2 years of inactivity --- .../tag_relationship_retirement_service.rb | 67 +++++++++++ app/models/tag_alias.rb | 1 + app/models/tag_implication.rb | 1 + app/models/tag_relationship.rb | 14 ++- app/views/tag_aliases/index.html.erb | 2 +- app/views/tag_implications/index.html.erb | 2 +- bin/bundle | 106 +----------------- bin/rails | 31 +---- bin/rake | 31 +---- test/models/forum_post_vote_test.rb | 7 -- ...ag_relationship_retirement_service_test.rb | 75 +++++++++++++ 11 files changed, 167 insertions(+), 170 deletions(-) create mode 100644 app/logical/tag_relationship_retirement_service.rb delete mode 100644 test/models/forum_post_vote_test.rb create mode 100644 test/models/tag_relationship_retirement_service_test.rb diff --git a/app/logical/tag_relationship_retirement_service.rb b/app/logical/tag_relationship_retirement_service.rb new file mode 100644 index 000000000..407b3a0aa --- /dev/null +++ b/app/logical/tag_relationship_retirement_service.rb @@ -0,0 +1,67 @@ +module TagRelationshipRetirementService + THRESHOLD = 2.year + + extend self + + def forum_topic_title + return "Retired tag aliases & implications" + end + + def forum_topic_body + return "This topic deals with tag relationships created two or more years ago that have not been used since. They will be retired. This topic will be updated as an automated system retires expired relationships." + end + + def dry_run + [TagAlias, TagImplication].each do |model| + each_candidate(model) do |rel| + puts "#{rel.relationship} #{rel.antecedent_name} -> #{rel.consequent_name} retired" + end + end + end + + def forum_topic + topic = ForumTopic.where(title: forum_topic_title).first + if topic.nil? + topic = CurrentUser.as_system do + ForumTopic.create(title: forum_topic_title, category_id: 1, original_post_attributes: {body: forum_topic_body}) + end + end + return topic + end + + def find_and_retire! + messages = [] + + [TagAlias, TagImplication].each do |model| + each_candidate(model) do |rel| + rel.update(status: "retired") + messages << rel.retirement_message + end + end + + updater = ForumUpdater.new(forum_topic) + updater.update(messages.sort.join("\n")) + end + + def each_candidate(model) + model.active.where("created_at < ?", THRESHOLD.ago).find_each do |rel| + if is_unused?(rel.consequent_name) + yield(rel) + end + end + + # model.active.where("created_at < ?", SMALL_THRESHOLD.ago).find_each do |rel| + # if is_underused?(rel.consequent_name) + # yield(rel) + # end + # end + end + + def is_underused?(name) + (Tag.find_by_name(name).try(:post_count) || 0) < COUNT_THRESHOLD + end + + def is_unused?(name) + return !Post.tag_match("status:any #{name}").where("created_at > ?", THRESHOLD.ago).exists? + end +end diff --git a/app/models/tag_alias.rb b/app/models/tag_alias.rb index 9491b458f..c7dcc2a9a 100644 --- a/app/models/tag_alias.rb +++ b/app/models/tag_alias.rb @@ -2,6 +2,7 @@ class TagAlias < TagRelationship before_save :ensure_tags_exist after_save :clear_all_cache after_destroy :clear_all_cache + after_save :clear_all_cache, if: ->(rec) {rec.is_retired?} after_save :create_mod_action validates_uniqueness_of :antecedent_name validate :absence_of_transitive_relation diff --git a/app/models/tag_implication.rb b/app/models/tag_implication.rb index 222f03aa3..97a906aab 100644 --- a/app/models/tag_implication.rb +++ b/app/models/tag_implication.rb @@ -2,6 +2,7 @@ class TagImplication < TagRelationship before_save :update_descendant_names after_save :update_descendant_names_for_parents after_destroy :update_descendant_names_for_parents + after_save :update_descendant_names_for_parents, if: ->(rec) { rec.is_retired? } after_save :create_mod_action validates_uniqueness_of :antecedent_name, :scope => :consequent_name validate :absence_of_circular_relation diff --git a/app/models/tag_relationship.rb b/app/models/tag_relationship.rb index 091c01dfc..b7824fb4d 100644 --- a/app/models/tag_relationship.rb +++ b/app/models/tag_relationship.rb @@ -13,13 +13,15 @@ class TagRelationship < ApplicationRecord has_one :antecedent_tag, :class_name => "Tag", :foreign_key => "name", :primary_key => "antecedent_name" has_one :consequent_tag, :class_name => "Tag", :foreign_key => "name", :primary_key => "consequent_name" + scope :active, ->{where(status: "active")} scope :expired, ->{where("created_at < ?", EXPIRY.days.ago)} scope :old, ->{where("created_at >= ? and created_at < ?", EXPIRY.days.ago, EXPIRY_WARNING.days.ago)} scope :pending, ->{where(status: "pending")} + scope :retired, ->{where(status: "retired")} before_validation :initialize_creator, :on => :create before_validation :normalize_names - validates_format_of :status, :with => /\A(active|deleted|pending|processing|queued|error: .*)\Z/ + validates_format_of :status, :with => /\A(active|deleted|pending|processing|queued|retired|error: .*)\Z/ validates_presence_of :creator_id, :antecedent_name, :consequent_name validates :creator, presence: { message: "must exist" }, if: -> { creator_id.present? } validates :approver, presence: { message: "must exist" }, if: -> { approver_id.present? } @@ -35,6 +37,10 @@ class TagRelationship < ApplicationRecord self.consequent_name = consequent_name.mb_chars.downcase.tr(" ", "_") end + def is_retired? + status == "retired" + end + def is_pending? status == "pending" end @@ -98,6 +104,8 @@ class TagRelationship < ApplicationRecord if params[:status].present? q = q.status_matches(params[:status]) + else + q = q.active end if params[:category].present? @@ -140,6 +148,10 @@ class TagRelationship < ApplicationRecord "The #{relationship} [[#{antecedent_name}]] -> [[#{consequent_name}]] #{forum_link} has been rejected by @#{rejector.name}." end + def retirement_message + "The #{relationship} [[#{antecedent_name}]] -> [[#{consequent_name}]] #{forum_link} has been retired." + end + def conflict_message "The tag alias [[#{antecedent_name}]] -> [[#{consequent_name}]] #{forum_link} has conflicting wiki pages. [[#{consequent_name}]] should be updated to include information from [[#{antecedent_name}]] if necessary." end diff --git a/app/views/tag_aliases/index.html.erb b/app/views/tag_aliases/index.html.erb index 5b6759e81..d0b046162 100644 --- a/app/views/tag_aliases/index.html.erb +++ b/app/views/tag_aliases/index.html.erb @@ -2,7 +2,7 @@
<%= simple_form_for(:search, method: :get, url: tag_aliases_path, defaults: { required: false }, html: { class: "inline-form" }) do |f| %> <%= f.input :name_matches, label: "Name", input_html: { value: params[:search][:name_matches], data: { autocomplete: "tag" } } %> - <%= f.input :status, label: "Status", collection: ["", "Approved", "Pending"], selected: params[:search][:status] %> + <%= f.input :status, label: "Status", collection: ["", "Approved", "Pending", "Retired"], selected: params[:search][:status] %> <%= f.input :category, label: "Category", collection: TagCategory.canonical_mapping.to_a, include_blank: true, selected: params[:search][:category] %> <%= f.input :order, label: "Order", collection: [%w[Status status], %w[Recently\ created created_at], %w[Recently\ updated updated_at], %w[Name name], %w[Tag\ count tag_count]], selected: params[:search][:order] %> <%= f.submit "Search" %> diff --git a/app/views/tag_implications/index.html.erb b/app/views/tag_implications/index.html.erb index d0d4ff32f..0127bc75c 100644 --- a/app/views/tag_implications/index.html.erb +++ b/app/views/tag_implications/index.html.erb @@ -2,7 +2,7 @@
<%= simple_form_for(:search, method: :get, url: tag_implications_path, defaults: { required: false }, html: { class: "inline-form" }) do |f| %> <%= f.input :name_matches, label: "Name", input_html: { value: params[:search][:name_matches], data: { autocomplete: "tag" } } %> - <%= f.input :status, label: "Status", collection: ["", "Approved", "Pending"], selected: params[:search][:status] %> + <%= f.input :status, label: "Status", collection: ["", "Approved", "Pending", "Retired"], selected: params[:search][:status] %> <%= f.input :category, label: "Category", collection: TagCategory.canonical_mapping.to_a, include_blank: true, selected: params[:search][:category] %> <%= f.input :order, label: "Order", collection: [%w[Status status], %w[Recently\ created created_at], %w[Recently\ updated updated_at], %w[Name name], %w[Tag\ count tag_count]], selected: params[:search][:order] %> <%= f.submit "Search" %> diff --git a/bin/bundle b/bin/bundle index 524dfd3f2..f19acf5b5 100755 --- a/bin/bundle +++ b/bin/bundle @@ -1,105 +1,3 @@ #!/usr/bin/env ruby -# frozen_string_literal: true - -# -# This file was generated by Bundler. -# -# The application 'bundle' is installed as part of a gem, and -# this file is here to facilitate running it. -# - -require "rubygems" - -m = Module.new do - module_function - - def invoked_as_script? - File.expand_path($0) == File.expand_path(__FILE__) - end - - def env_var_version - ENV["BUNDLER_VERSION"] - end - - def cli_arg_version - return unless invoked_as_script? # don't want to hijack other binstubs - return unless "update".start_with?(ARGV.first || " ") # must be running `bundle update` - bundler_version = nil - update_index = nil - ARGV.each_with_index do |a, i| - if update_index && update_index.succ == i && a =~ Gem::Version::ANCHORED_VERSION_PATTERN - bundler_version = a - end - next unless a =~ /\A--bundler(?:[= ](#{Gem::Version::VERSION_PATTERN}))?\z/ - bundler_version = $1 || ">= 0.a" - update_index = i - end - bundler_version - end - - def gemfile - gemfile = ENV["BUNDLE_GEMFILE"] - return gemfile if gemfile && !gemfile.empty? - - File.expand_path("../../Gemfile", __FILE__) - end - - def lockfile - lockfile = - case File.basename(gemfile) - when "gems.rb" then gemfile.sub(/\.rb$/, gemfile) - else "#{gemfile}.lock" - end - File.expand_path(lockfile) - end - - def lockfile_version - return unless File.file?(lockfile) - lockfile_contents = File.read(lockfile) - return unless lockfile_contents =~ /\n\nBUNDLED WITH\n\s{2,}(#{Gem::Version::VERSION_PATTERN})\n/ - Regexp.last_match(1) - end - - def bundler_version - @bundler_version ||= begin - env_var_version || cli_arg_version || - lockfile_version || "#{Gem::Requirement.default}.a" - end - end - - def load_bundler! - ENV["BUNDLE_GEMFILE"] ||= gemfile - - # must dup string for RG < 1.8 compatibility - activate_bundler(bundler_version.dup) - end - - def activate_bundler(bundler_version) - if Gem::Version.correct?(bundler_version) && Gem::Version.new(bundler_version).release < Gem::Version.new("2.0") - bundler_version = "< 2" - end - gem_error = activation_error_handling do - gem "bundler", bundler_version - end - return if gem_error.nil? - require_error = activation_error_handling do - require "bundler/version" - end - return if require_error.nil? && Gem::Requirement.new(bundler_version).satisfied_by?(Gem::Version.new(Bundler::VERSION)) - warn "Activating bundler (#{bundler_version}) failed:\n#{gem_error.message}\n\nTo install the version of bundler this project requires, run `gem install bundler -v '#{bundler_version}'`" - exit 42 - end - - def activation_error_handling - yield - nil - rescue StandardError, LoadError => e - e - end -end - -m.load_bundler! - -if m.invoked_as_script? - load Gem.bin_path("bundler", "bundle") -end +ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../Gemfile', __dir__) +load Gem.bin_path('bundler', 'bundle') diff --git a/bin/rails b/bin/rails index 6065ef071..073966023 100755 --- a/bin/rails +++ b/bin/rails @@ -1,29 +1,4 @@ #!/usr/bin/env ruby -# frozen_string_literal: true - -# -# This file was generated by Bundler. -# -# The application 'rails' is installed as part of a gem, and -# this file is here to facilitate running it. -# - -require "pathname" -ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile", - Pathname.new(__FILE__).realpath) - -bundle_binstub = File.expand_path("../bundle", __FILE__) - -if File.file?(bundle_binstub) - if File.read(bundle_binstub, 150) =~ /This file was generated by Bundler/ - load(bundle_binstub) - else - abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run. -Replace `bin/bundle` by running `bundle binstubs bundler --force`, then run this command again.") - end -end - -require "rubygems" -require "bundler/setup" - -load Gem.bin_path("railties", "rails") +APP_PATH = File.expand_path('../config/application', __dir__) +require_relative '../config/boot' +require 'rails/commands' diff --git a/bin/rake b/bin/rake index ea0e2936b..17240489f 100755 --- a/bin/rake +++ b/bin/rake @@ -1,29 +1,4 @@ #!/usr/bin/env ruby -# frozen_string_literal: true - -# -# This file was generated by Bundler. -# -# The application 'rake' is installed as part of a gem, and -# this file is here to facilitate running it. -# - -require "pathname" -ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile", - Pathname.new(__FILE__).realpath) - -bundle_binstub = File.expand_path("../bundle", __FILE__) - -if File.file?(bundle_binstub) - if File.read(bundle_binstub, 150) =~ /This file was generated by Bundler/ - load(bundle_binstub) - else - abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run. -Replace `bin/bundle` by running `bundle binstubs bundler --force`, then run this command again.") - end -end - -require "rubygems" -require "bundler/setup" - -load Gem.bin_path("rake", "rake") +require_relative '../config/boot' +require 'rake' +Rake.application.run diff --git a/test/models/forum_post_vote_test.rb b/test/models/forum_post_vote_test.rb deleted file mode 100644 index 3c7d85c47..000000000 --- a/test/models/forum_post_vote_test.rb +++ /dev/null @@ -1,7 +0,0 @@ -require 'test_helper' - -class ForumPostVoteTest < ActiveSupport::TestCase - # test "the truth" do - # assert true - # end -end diff --git a/test/models/tag_relationship_retirement_service_test.rb b/test/models/tag_relationship_retirement_service_test.rb new file mode 100644 index 000000000..cdede5c66 --- /dev/null +++ b/test/models/tag_relationship_retirement_service_test.rb @@ -0,0 +1,75 @@ +require 'test_helper' + +class TagRelationshipRetirementServiceTest < ActiveSupport::TestCase + context ".forum_topic" do + subject { TagRelationshipRetirementService } + + should "create a new topic if one doesn't already exist" do + assert_difference(-> { ForumTopic.count }) do + subject.forum_topic + end + end + + should "create a new post if one doesn't already exist" do + assert_difference(-> { ForumPost.count }) do + subject.forum_topic + end + end + end + + context ".each_candidate" do + subject { TagRelationshipRetirementService } + + setup do + subject.stubs(:is_unused?).returns(true) + + @user = FactoryBot.create(:user) + as_user do + @new_alias = FactoryBot.create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb") + + travel_to(3.years.ago) do + @old_alias = FactoryBot.create(:tag_alias, antecedent_name: "ccc", consequent_name: "ddd") + end + end + end + + should "find old tag relationships" do + subject.each_candidate(TagAlias) do |rel| + assert_equal(@old_alias, rel) + end + end + + should "not find new tag relationships" do + subject.each_candidate(TagAlias) do |rel| + assert_not_equal(@new_alias, rel) + end + end + end + + context ".is_unused?" do + subject { TagRelationshipRetirementService } + + setup do + @user = FactoryBot.create(:user) + + as_user do + @new_alias = FactoryBot.create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb") + @new_post = FactoryBot.create(:post, tag_string: "bbb") + + travel_to(3.years.ago) do + @old_alias = FactoryBot.create(:tag_alias, antecedent_name: "ccc", consequent_name: "ddd") + @old_post = FactoryBot.create(:post, tag_string: "ddd") + end + end + end + + should "return true if no recent post exists" do + assert(subject.is_unused?("ddd")) + end + + should "return false if a recent post exists" do + refute(subject.is_unused?("bbb")) + end + end +end +