From 3491642f98b4cd02f2c0f1f83f6731bac99eb357 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Wed, 6 Mar 2024 11:14:53 +0800 Subject: [PATCH] DEV: Make `discourse_narrative_bot` use Rails autoload (#26044) Why this change? Instead of manually loading files, we should just structure the plugin so that it relies on Rails autoload strategy and avoid all the manual `require_relative`s. What does this change do? 1. Structure the plugin to use Rails autoloading convention 2. Remove onceff jobs that were added 5-6 years ago. There is no need to carry these jobs anymore after such a long time. 3. Move setting of `SiteSetting.discourse_narrative_bot_enabled` to `false` in the test environment from core into the plugin. --- config/environments/test.rb | 5 - .../certificates_controller.rb | 44 +++++++ .../jobs/regular/bot_input.rb | 0 .../jobs/regular/narrative_init.rb | 0 .../jobs/regular/narrative_timeout.rb | 0 .../regular/send_default_welcome_message.rb | 0 .../discourse_narrative_bot/grant_badges.rb | 33 ----- .../remap_old_bot_images.rb | 44 ------- .../discourse-narrative-bot/config/routes.rb | 7 ++ .../lib/discourse_narrative_bot/engine.rb | 9 ++ .../lib/discourse_narrative_bot/store.rb | 19 +++ plugins/discourse-narrative-bot/plugin.rb | 119 +++--------------- .../spec/jobs/onceoff/grant_badges_spec.rb | 29 ----- .../jobs/onceoff/remap_old_bot_images_spec.rb | 43 ------- .../spec/plugin_spec.rb | 19 +++ .../requests/discobot_certificate_spec.rb | 3 +- 16 files changed, 116 insertions(+), 258 deletions(-) create mode 100644 plugins/discourse-narrative-bot/app/controllers/discourse_narrative_bot/certificates_controller.rb rename plugins/discourse-narrative-bot/{autoload => app}/jobs/regular/bot_input.rb (100%) rename plugins/discourse-narrative-bot/{autoload => app}/jobs/regular/narrative_init.rb (100%) rename plugins/discourse-narrative-bot/{autoload => app}/jobs/regular/narrative_timeout.rb (100%) rename plugins/discourse-narrative-bot/{autoload => app}/jobs/regular/send_default_welcome_message.rb (100%) delete mode 100644 plugins/discourse-narrative-bot/autoload/jobs/onceoff/discourse_narrative_bot/grant_badges.rb delete mode 100644 plugins/discourse-narrative-bot/autoload/jobs/onceoff/discourse_narrative_bot/remap_old_bot_images.rb create mode 100644 plugins/discourse-narrative-bot/config/routes.rb create mode 100644 plugins/discourse-narrative-bot/lib/discourse_narrative_bot/engine.rb create mode 100644 plugins/discourse-narrative-bot/lib/discourse_narrative_bot/store.rb delete mode 100644 plugins/discourse-narrative-bot/spec/jobs/onceoff/grant_badges_spec.rb delete mode 100644 plugins/discourse-narrative-bot/spec/jobs/onceoff/remap_old_bot_images_spec.rb create mode 100644 plugins/discourse-narrative-bot/spec/plugin_spec.rb diff --git a/config/environments/test.rb b/config/environments/test.rb index 587a79b8a04..d59858c23c9 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -95,11 +95,6 @@ Discourse::Application.configure do # Most existing tests were written assuming allow_uncategorized_topics # was enabled, so we should set it to true. s.set_regardless_of_locale(:allow_uncategorized_topics, true) - - # disable plugins - if ENV["LOAD_PLUGINS"] == "1" - s.set_regardless_of_locale(:discourse_narrative_bot_enabled, false) - end end SiteSetting.refresh! diff --git a/plugins/discourse-narrative-bot/app/controllers/discourse_narrative_bot/certificates_controller.rb b/plugins/discourse-narrative-bot/app/controllers/discourse_narrative_bot/certificates_controller.rb new file mode 100644 index 00000000000..21fb902bc9d --- /dev/null +++ b/plugins/discourse-narrative-bot/app/controllers/discourse_narrative_bot/certificates_controller.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +module DiscourseNarrativeBot + class CertificatesController < ::ApplicationController + requires_plugin DiscourseNarrativeBot::PLUGIN_NAME + layout false + skip_before_action :check_xhr + requires_login + + def generate + immutable_for(24.hours) + + %i[date user_id].each do |key| + unless params[key]&.present? + raise Discourse::InvalidParameters.new("#{key} must be present") + end + end + + if params[:user_id].to_i != current_user.id + rate_limiter = RateLimiter.new(current_user, "svg_certificate", 3, 1.minute) + else + rate_limiter = RateLimiter.new(current_user, "svg_certificate_self", 30, 10.minutes) + end + rate_limiter.performed! unless current_user.staff? + + user = User.find_by(id: params[:user_id]) + raise Discourse::NotFound if user.blank? + + hijack do + generator = CertificateGenerator.new(user, params[:date], avatar_url(user)) + + svg = params[:type] == "advanced" ? generator.advanced_user_track : generator.new_user_track + + respond_to { |format| format.svg { render inline: svg } } + end + end + + private + + def avatar_url(user) + UrlHelper.absolute(Discourse.base_path + user.avatar_template.gsub("{size}", "250")) + end + end +end diff --git a/plugins/discourse-narrative-bot/autoload/jobs/regular/bot_input.rb b/plugins/discourse-narrative-bot/app/jobs/regular/bot_input.rb similarity index 100% rename from plugins/discourse-narrative-bot/autoload/jobs/regular/bot_input.rb rename to plugins/discourse-narrative-bot/app/jobs/regular/bot_input.rb diff --git a/plugins/discourse-narrative-bot/autoload/jobs/regular/narrative_init.rb b/plugins/discourse-narrative-bot/app/jobs/regular/narrative_init.rb similarity index 100% rename from plugins/discourse-narrative-bot/autoload/jobs/regular/narrative_init.rb rename to plugins/discourse-narrative-bot/app/jobs/regular/narrative_init.rb diff --git a/plugins/discourse-narrative-bot/autoload/jobs/regular/narrative_timeout.rb b/plugins/discourse-narrative-bot/app/jobs/regular/narrative_timeout.rb similarity index 100% rename from plugins/discourse-narrative-bot/autoload/jobs/regular/narrative_timeout.rb rename to plugins/discourse-narrative-bot/app/jobs/regular/narrative_timeout.rb diff --git a/plugins/discourse-narrative-bot/autoload/jobs/regular/send_default_welcome_message.rb b/plugins/discourse-narrative-bot/app/jobs/regular/send_default_welcome_message.rb similarity index 100% rename from plugins/discourse-narrative-bot/autoload/jobs/regular/send_default_welcome_message.rb rename to plugins/discourse-narrative-bot/app/jobs/regular/send_default_welcome_message.rb diff --git a/plugins/discourse-narrative-bot/autoload/jobs/onceoff/discourse_narrative_bot/grant_badges.rb b/plugins/discourse-narrative-bot/autoload/jobs/onceoff/discourse_narrative_bot/grant_badges.rb deleted file mode 100644 index fe3b17e70be..00000000000 --- a/plugins/discourse-narrative-bot/autoload/jobs/onceoff/discourse_narrative_bot/grant_badges.rb +++ /dev/null @@ -1,33 +0,0 @@ -# frozen_string_literal: true - -module Jobs - module DiscourseNarrativeBot - class GrantBadges < ::Jobs::Onceoff - def execute_onceoff(args) - new_user_track_badge = - Badge.find_by(name: ::DiscourseNarrativeBot::NewUserNarrative.badge_name) - - advanced_user_track_badge = - Badge.find_by(name: ::DiscourseNarrativeBot::AdvancedUserNarrative.badge_name) - - PluginStoreRow - .where(plugin_name: ::DiscourseNarrativeBot::PLUGIN_NAME, type_name: "JSON") - .find_each do |row| - value = JSON.parse(row.value) - completed = value["completed"] - user = User.find_by(id: row.key) - - if user && completed - if completed.include?(::DiscourseNarrativeBot::NewUserNarrative.to_s) - BadgeGranter.grant(new_user_track_badge, user) - end - - if completed.include?(::DiscourseNarrativeBot::AdvancedUserNarrative.to_s) - BadgeGranter.grant(advanced_user_track_badge, user) - end - end - end - end - end - end -end diff --git a/plugins/discourse-narrative-bot/autoload/jobs/onceoff/discourse_narrative_bot/remap_old_bot_images.rb b/plugins/discourse-narrative-bot/autoload/jobs/onceoff/discourse_narrative_bot/remap_old_bot_images.rb deleted file mode 100644 index e20a05b1e7d..00000000000 --- a/plugins/discourse-narrative-bot/autoload/jobs/onceoff/discourse_narrative_bot/remap_old_bot_images.rb +++ /dev/null @@ -1,44 +0,0 @@ -# frozen_string_literal: true - -module Jobs - module DiscourseNarrativeBot - class RemapOldBotImages < ::Jobs::Onceoff - def execute_onceoff(args) - paths = %w[ - /images/font-awesome-link.png - /images/unicorn.png - /images/font-awesome-ellipsis.png - /images/font-awesome-bookmark.png - /images/font-awesome-smile.png - /images/font-awesome-flag.png - /images/font-awesome-search.png - /images/capybara-eating.gif - /images/font-awesome-pencil.png - /images/font-awesome-trash.png - /images/font-awesome-rotate-left.png - /images/font-awesome-gear.png - ] - - Post - .raw_match("/images/") - .where(user_id: -2) - .find_each do |post| - if ( - matches = - post.raw.scan(%r{(? "certificates#generate", :format => :svg +end + +Discourse::Application.routes.draw { mount ::DiscourseNarrativeBot::Engine, at: "/discobot" } diff --git a/plugins/discourse-narrative-bot/lib/discourse_narrative_bot/engine.rb b/plugins/discourse-narrative-bot/lib/discourse_narrative_bot/engine.rb new file mode 100644 index 00000000000..7d74cfb0bb4 --- /dev/null +++ b/plugins/discourse-narrative-bot/lib/discourse_narrative_bot/engine.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module DiscourseNarrativeBot + class Engine < ::Rails::Engine + engine_name PLUGIN_NAME + isolate_namespace DiscourseNarrativeBot + config.autoload_paths << File.join(config.root, "lib") + end +end diff --git a/plugins/discourse-narrative-bot/lib/discourse_narrative_bot/store.rb b/plugins/discourse-narrative-bot/lib/discourse_narrative_bot/store.rb new file mode 100644 index 00000000000..dab33725247 --- /dev/null +++ b/plugins/discourse-narrative-bot/lib/discourse_narrative_bot/store.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require_dependency "plugin_store" + +module DiscourseNarrativeBot + class Store + def self.set(key, value) + ::PluginStore.set(PLUGIN_NAME, key, value) + end + + def self.get(key) + ::PluginStore.get(PLUGIN_NAME, key) + end + + def self.remove(key) + ::PluginStore.remove(PLUGIN_NAME, key) + end + end +end diff --git a/plugins/discourse-narrative-bot/plugin.rb b/plugins/discourse-narrative-bot/plugin.rb index c2e758d44bd..c10959d90ad 100644 --- a/plugins/discourse-narrative-bot/plugin.rb +++ b/plugins/discourse-narrative-bot/plugin.rb @@ -9,22 +9,26 @@ enabled_site_setting :discourse_narrative_bot_enabled hide_plugin -if Rails.env == "development" - # workaround, teach reloader to reload jobs - # if we do not do this then - # - # 1. on reload rails goes and undefines Jobs::Base - # 2. as a side effect this undefines Jobs::BotInput - # 3. we have a post_edited hook that queues a job for bot input - # 4. if you are not running sidekiq in dev every time you save a post it will trigger it - # 5. but the constant can not be autoloaded - Rails.configuration.autoload_paths << File.expand_path("../autoload/jobs", __FILE__) -end - require_relative "lib/discourse_narrative_bot/welcome_post_type_site_setting" register_asset "stylesheets/discourse-narrative-bot.scss" +module ::DiscourseNarrativeBot + PLUGIN_NAME = "discourse-narrative-bot".freeze + BOT_USER_ID = -2 +end + +require_relative "lib/discourse_narrative_bot/engine" + after_initialize do + if Rails.env.test? + ::SiteSetting.defaults.tap do |s| + # disable plugins + if ENV["LOAD_PLUGINS"] == "1" + s.set_regardless_of_locale(:discourse_narrative_bot_enabled, false) + end + end + end + SeedFu.fixture_paths << Rails .root .join("plugins", "discourse-narrative-bot", "db", "fixtures") @@ -32,24 +36,6 @@ after_initialize do Mime::Type.register "image/svg+xml", :svg - require_relative "autoload/jobs/regular/bot_input" - require_relative "autoload/jobs/regular/narrative_timeout" - require_relative "autoload/jobs/regular/narrative_init" - require_relative "autoload/jobs/regular/send_default_welcome_message" - require_relative "autoload/jobs/onceoff/discourse_narrative_bot/grant_badges" - require_relative "autoload/jobs/onceoff/discourse_narrative_bot/remap_old_bot_images" - require_relative "lib/discourse_narrative_bot/actions" - require_relative "lib/discourse_narrative_bot/base" - require_relative "lib/discourse_narrative_bot/new_user_narrative" - require_relative "lib/discourse_narrative_bot/advanced_user_narrative" - require_relative "lib/discourse_narrative_bot/track_selector" - require_relative "lib/discourse_narrative_bot/certificate_generator" - require_relative "lib/discourse_narrative_bot/dice" - require_relative "lib/discourse_narrative_bot/quote_generator" - require_relative "lib/discourse_narrative_bot/magic_8_ball" - require_relative "lib/discourse_narrative_bot/welcome_post_type_site_setting" - require_relative "lib/discourse_narrative_bot/post_guardian_extension" - RailsMultisite::ConnectionManagement.safe_each_connection do if SiteSetting.discourse_narrative_bot_enabled # Disable welcome message because that is what the bot is supposed to replace. @@ -63,79 +49,6 @@ after_initialize do end end - require_dependency "plugin_store" - - module ::DiscourseNarrativeBot - PLUGIN_NAME = "discourse-narrative-bot".freeze - BOT_USER_ID = -2 - - class Engine < ::Rails::Engine - engine_name PLUGIN_NAME - isolate_namespace DiscourseNarrativeBot - end - - class Store - def self.set(key, value) - ::PluginStore.set(PLUGIN_NAME, key, value) - end - - def self.get(key) - ::PluginStore.get(PLUGIN_NAME, key) - end - - def self.remove(key) - ::PluginStore.remove(PLUGIN_NAME, key) - end - end - - class CertificatesController < ::ApplicationController - layout false - skip_before_action :check_xhr - requires_login - - def generate - immutable_for(24.hours) - - %i[date user_id].each do |key| - unless params[key]&.present? - raise Discourse::InvalidParameters.new("#{key} must be present") - end - end - - if params[:user_id].to_i != current_user.id - rate_limiter = RateLimiter.new(current_user, "svg_certificate", 3, 1.minute) - else - rate_limiter = RateLimiter.new(current_user, "svg_certificate_self", 30, 10.minutes) - end - rate_limiter.performed! unless current_user.staff? - - user = User.find_by(id: params[:user_id]) - raise Discourse::NotFound if user.blank? - - hijack do - generator = CertificateGenerator.new(user, params[:date], avatar_url(user)) - - svg = - params[:type] == "advanced" ? generator.advanced_user_track : generator.new_user_track - - respond_to { |format| format.svg { render inline: svg } } - end - end - - private - - def avatar_url(user) - UrlHelper.absolute(Discourse.base_path + user.avatar_template.gsub("{size}", "250")) - end - end - end - - DiscourseNarrativeBot::Engine.routes.draw do - get "/certificate" => "certificates#generate", :format => :svg - end - - Discourse::Application.routes.append { mount ::DiscourseNarrativeBot::Engine, at: "/discobot" } - self.add_model_callback(User, :after_destroy) { DiscourseNarrativeBot::Store.remove(self.id) } self.on(:user_created) do |user| diff --git a/plugins/discourse-narrative-bot/spec/jobs/onceoff/grant_badges_spec.rb b/plugins/discourse-narrative-bot/spec/jobs/onceoff/grant_badges_spec.rb deleted file mode 100644 index 3f7832e8d79..00000000000 --- a/plugins/discourse-narrative-bot/spec/jobs/onceoff/grant_badges_spec.rb +++ /dev/null @@ -1,29 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe Jobs::DiscourseNarrativeBot::GrantBadges do - let(:user) { Fabricate(:user) } - let(:other_user) { Fabricate(:user) } - - before do - DiscourseNarrativeBot::Store.set( - user.id, - completed: [ - DiscourseNarrativeBot::NewUserNarrative.to_s, - DiscourseNarrativeBot::AdvancedUserNarrative.to_s, - ], - ) - end - - it "should grant the right badges" do - described_class.new.execute_onceoff({}) - - expect(user.badges.count).to eq(2) - - expect(user.badges.map(&:name)).to contain_exactly( - DiscourseNarrativeBot::NewUserNarrative.badge_name, - DiscourseNarrativeBot::AdvancedUserNarrative.badge_name, - ) - - expect(other_user.badges.count).to eq(0) - end -end diff --git a/plugins/discourse-narrative-bot/spec/jobs/onceoff/remap_old_bot_images_spec.rb b/plugins/discourse-narrative-bot/spec/jobs/onceoff/remap_old_bot_images_spec.rb deleted file mode 100644 index 4a3abe96ad1..00000000000 --- a/plugins/discourse-narrative-bot/spec/jobs/onceoff/remap_old_bot_images_spec.rb +++ /dev/null @@ -1,43 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe Jobs::DiscourseNarrativeBot::RemapOldBotImages do - context "when bot's post contains an old link" do - let!(:post) do - Fabricate( - :post, - user: ::DiscourseNarrativeBot::Base.new.discobot_user, - raw: - 'If you’d like to learn more, select below and **bookmark this private message**. If you do, there may be a :gift: in your future!', - ) - end - - it "should remap the links correctly" do - expected_raw = - 'If you’d like to learn more, select below and **bookmark this private message**. If you do, there may be a :gift: in your future!' - - 2.times do - described_class.new.execute_onceoff({}) - expect(post.reload.raw).to eq(expected_raw) - end - end - end - - context "with subfolder" do - let!(:post) do - Fabricate( - :post, - user: ::DiscourseNarrativeBot::Base.new.discobot_user, - raw: - 'If you’d like to learn more, select below and **bookmark this private message**. If you do, there may be a :gift: in your future!', - ) - end - - it "should remap the links correctly" do - described_class.new.execute_onceoff({}) - - expect(post.reload.raw).to eq( - 'If you’d like to learn more, select below and **bookmark this private message**. If you do, there may be a :gift: in your future!', - ) - end - end -end diff --git a/plugins/discourse-narrative-bot/spec/plugin_spec.rb b/plugins/discourse-narrative-bot/spec/plugin_spec.rb new file mode 100644 index 00000000000..97e18d86fcf --- /dev/null +++ b/plugins/discourse-narrative-bot/spec/plugin_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +RSpec.describe "Plugin specs" do + let(:narrative_bot) { ::DiscourseNarrativeBot::Base.new } + let(:discobot_user) { narrative_bot.discobot_user } + + before { SiteSetting.discourse_narrative_bot_enabled = true } + it "should update bot's `UserProfile#bio_raw` when `default_locale` site setting is changed" do + expect(discobot_user.user_profile.bio_raw).to eq( + I18n.with_locale(:en) { I18n.t("discourse_narrative_bot.bio") }, + ) + + SiteSetting.default_locale = "zn_CN" + + expect(discobot_user.user_profile.bio_raw).to eq( + I18n.with_locale(:zh_CN) { I18n.t("discourse_narrative_bot.bio") }, + ) + end +end diff --git a/plugins/discourse-narrative-bot/spec/requests/discobot_certificate_spec.rb b/plugins/discourse-narrative-bot/spec/requests/discobot_certificate_spec.rb index 6508718696f..8d3f0908d93 100644 --- a/plugins/discourse-narrative-bot/spec/requests/discobot_certificate_spec.rb +++ b/plugins/discourse-narrative-bot/spec/requests/discobot_certificate_spec.rb @@ -2,9 +2,10 @@ RSpec.describe "Discobot Certificate" do let(:user) { Fabricate(:user, name: "Jeff Atwood") } - let(:params) { { date: Time.zone.now.strftime("%b %d %Y"), user_id: user.id } } + before { SiteSetting.discourse_narrative_bot_enabled = true } + describe "when viewing the certificate" do describe "when no logged in" do it "should return the right response" do