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.
This commit is contained in:
Alan Guo Xiang Tan 2024-03-06 11:14:53 +08:00 committed by GitHub
parent 6b46b9ab78
commit 3491642f98
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
16 changed files with 116 additions and 258 deletions

View File

@ -95,11 +95,6 @@ Discourse::Application.configure do
# Most existing tests were written assuming allow_uncategorized_topics # Most existing tests were written assuming allow_uncategorized_topics
# was enabled, so we should set it to true. # was enabled, so we should set it to true.
s.set_regardless_of_locale(:allow_uncategorized_topics, 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 end
SiteSetting.refresh! SiteSetting.refresh!

View File

@ -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

View File

@ -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

View File

@ -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{(?<!/plugins/discourse-narrative-bot)(#{paths.join("|")})})
).present?
new_raw = post.raw
matches.each do |match|
path = match.first
new_raw = new_raw.gsub(path, "/plugins/discourse-narrative-bot#{path}")
end
post.update_columns(raw: new_raw)
post.rebake!
end
end
end
end
end
end

View File

@ -0,0 +1,7 @@
# frozen_string_literal: true
DiscourseNarrativeBot::Engine.routes.draw do
get "/certificate" => "certificates#generate", :format => :svg
end
Discourse::Application.routes.draw { mount ::DiscourseNarrativeBot::Engine, at: "/discobot" }

View File

@ -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

View File

@ -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

View File

@ -9,22 +9,26 @@
enabled_site_setting :discourse_narrative_bot_enabled enabled_site_setting :discourse_narrative_bot_enabled
hide_plugin 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" require_relative "lib/discourse_narrative_bot/welcome_post_type_site_setting"
register_asset "stylesheets/discourse-narrative-bot.scss" 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 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 SeedFu.fixture_paths << Rails
.root .root
.join("plugins", "discourse-narrative-bot", "db", "fixtures") .join("plugins", "discourse-narrative-bot", "db", "fixtures")
@ -32,24 +36,6 @@ after_initialize do
Mime::Type.register "image/svg+xml", :svg 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 RailsMultisite::ConnectionManagement.safe_each_connection do
if SiteSetting.discourse_narrative_bot_enabled if SiteSetting.discourse_narrative_bot_enabled
# Disable welcome message because that is what the bot is supposed to replace. # Disable welcome message because that is what the bot is supposed to replace.
@ -63,79 +49,6 @@ after_initialize do
end end
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.add_model_callback(User, :after_destroy) { DiscourseNarrativeBot::Store.remove(self.id) }
self.on(:user_created) do |user| self.on(:user_created) do |user|

View File

@ -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

View File

@ -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 youd like to learn more, select <img src="/images/font-awesome-gear.png" width="16" height="16"> <img src="/images/font-awesome-ellipsis.png" width="16" height="16"> below and <img src="/images/font-awesome-bookmark.png" width="16" height="16"> **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 youd like to learn more, select <img src="/plugins/discourse-narrative-bot/images/font-awesome-gear.png" width="16" height="16"> <img src="/plugins/discourse-narrative-bot/images/font-awesome-ellipsis.png" width="16" height="16"> below and <img src="/plugins/discourse-narrative-bot/images/font-awesome-bookmark.png" width="16" height="16"> **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 youd like to learn more, select <img src="/community/images/font-awesome-ellipsis.png" width="16" height="16"> below and <img src="/community/images/font-awesome-bookmark.png" width="16" height="16"> **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 youd like to learn more, select <img src="/community/plugins/discourse-narrative-bot/images/font-awesome-ellipsis.png" width="16" height="16"> below and <img src="/community/plugins/discourse-narrative-bot/images/font-awesome-bookmark.png" width="16" height="16"> **bookmark this private message**. If you do, there may be a :gift: in your future!',
)
end
end
end

View File

@ -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

View File

@ -2,9 +2,10 @@
RSpec.describe "Discobot Certificate" do RSpec.describe "Discobot Certificate" do
let(:user) { Fabricate(:user, name: "Jeff Atwood") } let(:user) { Fabricate(:user, name: "Jeff Atwood") }
let(:params) { { date: Time.zone.now.strftime("%b %d %Y"), user_id: user.id } } 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 viewing the certificate" do
describe "when no logged in" do describe "when no logged in" do
it "should return the right response" do it "should return the right response" do