Apple::Config only belongs to the private feed #993
Conversation
| @@ -15,7 +15,8 @@ def perform(podcast, pub_item) | |||
| Rails.logger.info("Starting publishing pipeline via PublishFeedJob", {podcast_id: podcast.id, publishing_queue_item_id: pub_item.id}) | |||
|
|
|||
| PublishingPipelineState.start!(podcast) | |||
| podcast.feeds.each { |feed| publish_feed(podcast, feed) } | |||
| podcast.feeds.each { |feed| publish_apple(podcast, feed) } | |||
| podcast.feeds.each { |feed| publish_rss(podcast, feed) } | |||
There was a problem hiding this comment.
Okay, so here is kind of the crux of it.
It used to be the public/default feed that had a f.apple_config. But now it's tied to the private_feed.apple_config.
So - we can't rely on the podcast.feeds[0] (the default_feed) processing first, which blocks RSS until we publish to apple. Instead, we do all the apple publishing first, before moving onto RSS.
| end | ||
|
|
||
| def publish_apple(feed) | ||
| def publish_apple(podcast, feed) | ||
| return unless feed.publish_to_apple? | ||
| res = PublishAppleJob.perform_now(feed.apple_config) |
There was a problem hiding this comment.
Took me awhile to realize - this class is a job, but we only call it synchronously. So we block right here in this thread until Apple publishing works/fails.
| return false unless key&.valid? | ||
|
|
||
| public_feed.publish_to_apple? | ||
| !!key&.valid? && publish_enabled? |
There was a problem hiding this comment.
The logic was circular here - it called up to the public_feed, which called back down into this apple_config.publish_enabled field. Cleaned it up so this model gets to decide directly.
There was a problem hiding this comment.
I saw this, and almost changed it in my PR, but decided it was a bit out of scope - glad you saw it too!
| apple_config.present? && | ||
| apple_config.public_feed == self && | ||
| apple_config.publish_enabled? | ||
| !!apple_config&.publish_to_apple? |
There was a problem hiding this comment.
Note this method now returns true on the private feed, but not on the default/public feed.
| let(:private_feed) { create(:private_feed, podcast: podcast) } | ||
| let(:apple_config) { create(:apple_config, podcast: podcast, public_feed: feed, private_feed: private_feed) } | ||
| let(:apple_config) { create(:apple_config, feed: private_feed) } |
There was a problem hiding this comment.
So common pattern in the test refactor... the public :feed is now just the podcast.default_feed. And the :apple_config only gets the private feed association.
Similarly, some method args change from the public-feed to the private-feed, since now it's the private_feed.publish_apple? == true.
| @@ -256,7 +256,7 @@ | |||
|
|
|||
| it "returns an authed url if private" do | |||
| assert_equal apple_show.feed_published_url, | |||
| "https://p.prxu.org/#{public_feed.podcast.path}/#{public_feed.slug}/feed-rss.xml?auth=" + public_feed.tokens.first.token | |||
| "https://p.prxu.org/#{public_feed.podcast.path}/feed-rss.xml?auth=" + public_feed.tokens.first.token | |||
There was a problem hiding this comment.
This test was a bit atypical - IRL you'd never have a public_feed that wasn't the default_feed (with a nil slug).
| belongs_to :public_feed, class_name: "Feed" | ||
| belongs_to :private_feed, class_name: "Feed" | ||
| belongs_to :key, class_name: "Apple::Key", optional: true | ||
| belongs_to :feed |
There was a problem hiding this comment.
So the only feed associated is the private apple feed, and I assume when I get to the migrations you are renaming that as feed_id
|
|
||
| remove_column :apple_configs, :podcast_id | ||
| remove_column :apple_configs, :public_feed_id | ||
| rename_column :apple_configs, :private_feed_id, :feed_id |
Our Apple::Config schema is fighting a bit with the formbuilder/UI. Since it shows up on the private Feed form UI. But only the
public_feed has_one :apple_config. And there are 3 foreign keys to populate/validate.This simplifies down to a single assocation:
private_feed has_one :apple_config. And then I alias/delegate everything else when possible, to keep interfaces the same.Should definitely QA this one carefully. Because it makes me nervous!