mirror of
https://github.com/discourse/discourse.git
synced 2026-08-08 17:53:55 +08:00
The discourse-rss-polling plugin stored the feed author as a plain `author` string with no link to the users table. When a user was renamed, the polling job's `User.find_by_username(old_name)` returned nil, the job bailed with no log, and the feed silently stopped importing posts. We've hit this repeatedly on enterprise sites, and every time the fix is the same manual cleanup in the admin UI. This commit moves the feed -> user association onto a proper `user_id` foreign key, so renames (and user deletion) no longer break polling. It also brings the plugin up to current Discourse conventions: a service object for the update action, an ApplicationSerializer for the admin list view, and a fabricator for specs. What's in this commit: * Migration adds `user_id` and an index, backfills from the legacy `author` string via a case-insensitive match, defaults the rest to the system user, drops the default on `author`, and marks the column readonly so it cannot drift again. Follow-up post-deploy migration will drop the column. * `RssFeed` gains `belongs_to :user`, delegates `author_username` to the user, ignores the legacy `author` column, and owns `#poll` (moved here from the deleted `FeedSetting` DTO). * `PollFeed` job now looks up the author by `user_id` first and keeps a short-lived `author_username` fallback for jobs enqueued before deploy. When the user cannot be resolved it falls back to the system user and logs a warning instead of silently returning. * `PollAllFeeds` scheduled job iterates `RssFeed.includes(:user)` directly — no finder indirection. * `DiscourseRssPolling::FeedSetting::Update` service replaces the controller's inline parse/resolve/assign/render logic. The contract validates presence of `feed_url` and `author_username` (previously a blank username was silently saved as `user_id: nil`), normalizes the tag-chooser's array-of-hashes shape, and produces the CSV the `tags` column expects. Unknown usernames surface as an `on_model_not_found` match with a proper i18n error in the toast. * `FeedSettingSerializer < ApplicationSerializer` replaces the `FeedSetting` DTO for the admin list view and reads `author_username` live from the user association on every request, so the UI never shows a stale name. * `FeedSettingFinder` is removed — `by_embed_url` was dead and `.all` is now a one-liner at its two call sites. Ref - t/182280
99 lines
2.9 KiB
Ruby
Vendored
99 lines
2.9 KiB
Ruby
Vendored
# frozen_string_literal: true
|
|
|
|
RSpec.describe DiscourseRssPolling::FeedSetting::Update do
|
|
describe described_class::Contract, type: :model do
|
|
it { is_expected.to validate_presence_of(:feed_url) }
|
|
it { is_expected.to validate_presence_of(:author_username) }
|
|
end
|
|
|
|
describe ".call" do
|
|
subject(:result) { described_class.call(params:, **dependencies) }
|
|
|
|
fab!(:user)
|
|
fab!(:category)
|
|
fab!(:tag_1, :tag)
|
|
fab!(:tag_2, :tag)
|
|
|
|
let(:params) do
|
|
{
|
|
id: nil,
|
|
feed_url: "https://blog.example.com/feed",
|
|
author_username: user.username,
|
|
discourse_category_id: category.id,
|
|
discourse_tags: [tag_1.name, tag_2.name],
|
|
feed_category_filter: "news",
|
|
}
|
|
end
|
|
let(:dependencies) { {} }
|
|
|
|
context "when the contract is invalid" do
|
|
before { params[:feed_url] = nil }
|
|
|
|
it { is_expected.to fail_a_contract }
|
|
end
|
|
|
|
context "when the author does not exist" do
|
|
before { params[:author_username] = "ghost" }
|
|
|
|
it { is_expected.to fail_to_find_a_model(:user) }
|
|
end
|
|
|
|
context "when everything is ok" do
|
|
context "when creating a new feed" do
|
|
it { is_expected.to run_successfully }
|
|
|
|
it "creates a new rss_feed with the provided attributes" do
|
|
expect { result }.to change { DiscourseRssPolling::RssFeed.count }.by(1)
|
|
|
|
feed = result[:rss_feed]
|
|
expect(feed).to have_attributes(
|
|
url: "https://blog.example.com/feed",
|
|
user_id: user.id,
|
|
category_id: category.id,
|
|
tags: "#{tag_1.name},#{tag_2.name}",
|
|
category_filter: "news",
|
|
)
|
|
end
|
|
end
|
|
|
|
context "when updating an existing feed" do
|
|
fab!(:rss_feed) { Fabricate(:rss_feed, url: "https://old.example.com/feed", user: user) }
|
|
|
|
before { params[:id] = rss_feed.id }
|
|
|
|
it { is_expected.to run_successfully }
|
|
|
|
it "updates the existing rss_feed" do
|
|
expect { result }.not_to change { DiscourseRssPolling::RssFeed.count }
|
|
expect(rss_feed.reload).to have_attributes(
|
|
url: "https://blog.example.com/feed",
|
|
user_id: user.id,
|
|
category_id: category.id,
|
|
tags: "#{tag_1.name},#{tag_2.name}",
|
|
category_filter: "news",
|
|
)
|
|
end
|
|
end
|
|
|
|
context "when discourse_tags is an array of hashes" do
|
|
before { params[:discourse_tags] = [{ "name" => tag_1.name }, { "name" => tag_2.name }] }
|
|
|
|
it { is_expected.to run_successfully }
|
|
|
|
it "normalizes tags into a comma-separated string" do
|
|
expect(result[:rss_feed].tags).to eq("#{tag_1.name},#{tag_2.name}")
|
|
end
|
|
end
|
|
|
|
context "when discourse_tags is blank" do
|
|
before { params[:discourse_tags] = nil }
|
|
|
|
it { is_expected.to run_successfully }
|
|
|
|
it "stores nil tags" do
|
|
expect(result[:rss_feed].tags).to be_nil
|
|
end
|
|
end
|
|
end
|
|
end
|
|
end
|