From 59d192a954854e1b2dd47d1582e699dbd48f8bee Mon Sep 17 00:00:00 2001
From: Sam Vevang <sam.vevang@gmail.com>
Date: Mon, 4 Nov 2024 17:17:40 -0600
Subject: [PATCH 01/16] Clarify naming and extract episode status instance
 methods

---
 app/models/apple/config.rb                             |  2 +-
 app/models/apple/episode_delivery_status.rb            |  8 ++++++++
 app/models/apple/publisher.rb                          |  2 +-
 app/models/concerns/apple_delivery.rb                  | 10 ++++------
 ...230920153421_add_needs_apple_delivery_to_episode.rb |  4 ++--
 test/factories/apple_episode_factory.rb                |  2 +-
 test/models/apple/episode_delivery_status_test.rb      |  2 +-
 7 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/app/models/apple/config.rb b/app/models/apple/config.rb
index a1a4a2db7..87ba16b27 100644
--- a/app/models/apple/config.rb
+++ b/app/models/apple/config.rb
@@ -41,7 +41,7 @@ def self.build_apple_config(podcast, key)
     def self.mark_as_delivered!(apple_publisher)
       apple_publisher.episodes_to_sync.each do |episode|
         if episode.podcast_container&.needs_delivery? == false
-          episode.feeder_episode.apple_has_delivery!
+          episode.feeder_episode.apple_mark_as_delivered!
         end
       end
     end
diff --git a/app/models/apple/episode_delivery_status.rb b/app/models/apple/episode_delivery_status.rb
index 5781c180c..c78e58a20 100644
--- a/app/models/apple/episode_delivery_status.rb
+++ b/app/models/apple/episode_delivery_status.rb
@@ -21,5 +21,13 @@ def increment_asset_wait
     def reset_asset_wait
       self.class.update_status(episode, asset_processing_attempts: 0)
     end
+
+    def mark_as_delivered!
+      self.class.update_status(episode, delivered: true)
+    end
+
+    def mark_as_not_delivered!
+      self.class.update_status(episode, delivered: false)
+    end
   end
 end
diff --git a/app/models/apple/publisher.rb b/app/models/apple/publisher.rb
index 65ee96901..81e7ef81a 100644
--- a/app/models/apple/publisher.rb
+++ b/app/models/apple/publisher.rb
@@ -367,7 +367,7 @@ def mark_delivery_files_uploaded!(eps)
         # update the feeder episode to indicate that delivery is no longer needed
         eps.each do |ep|
           Rails.logger.info("Marking episode as no longer needing delivery", {episode_id: ep.feeder_episode.id})
-          ep.feeder_episode.apple_has_delivery!
+          ep.feeder_episode.apple_mark_as_delivered!
         end
 
         Rails.logger.info("Updated remote container references for episodes.", {count: res.length})
diff --git a/app/models/concerns/apple_delivery.rb b/app/models/concerns/apple_delivery.rb
index 0083481b7..2be30403b 100644
--- a/app/models/concerns/apple_delivery.rb
+++ b/app/models/concerns/apple_delivery.rb
@@ -34,17 +34,15 @@ def apple_episode_delivery_status
   end
 
   def apple_needs_delivery?
-    return true if apple_episode_delivery_status.nil?
-
     apple_episode_delivery_status.delivered == false
   end
 
-  def apple_needs_delivery!
-    apple_update_delivery_status(delivered: false)
+  def apple_mark_as_not_delivered!
+    apple_episode_delivery_status.mark_as_not_delivered!
   end
 
-  def apple_has_delivery!
-    apple_update_delivery_status(delivered: true)
+  def apple_mark_as_delivered!
+    apple_episode_delivery_status.mark_as_delivered!
   end
 
   def measure_asset_processing_duration
diff --git a/db/migrate/20230920153421_add_needs_apple_delivery_to_episode.rb b/db/migrate/20230920153421_add_needs_apple_delivery_to_episode.rb
index 3b3d01d52..c61370edf 100644
--- a/db/migrate/20230920153421_add_needs_apple_delivery_to_episode.rb
+++ b/db/migrate/20230920153421_add_needs_apple_delivery_to_episode.rb
@@ -20,10 +20,10 @@ def change
         needs_delivery_episodes = []
         apple_episodes.each do |apple_episode|
           if (apple_episode.podcast_container.nil? || apple_episode.podcast_container.needs_delivery?) || apple_episode.apple_hosted_audio_asset_container_id.blank?
-            apple_episode.feeder_episode.apple_needs_delivery!
+            apple_episode.feeder_episode.apple_mark_as_not_delivered!
             needs_delivery_episodes << apple_episode.feeder_episode
           else
-            apple_episode.feeder_episode.apple_has_delivery!
+            apple_episode.feeder_episode.apple_mark_as_delivered!
           end
         end
 
diff --git a/test/factories/apple_episode_factory.rb b/test/factories/apple_episode_factory.rb
index ef8458758..1d2cb31e1 100644
--- a/test/factories/apple_episode_factory.rb
+++ b/test/factories/apple_episode_factory.rb
@@ -13,7 +13,7 @@
     factory :uploaded_apple_episode do
       feeder_episode do
         ep = create(:episode)
-        ep.apple_has_delivery!
+        ep.apple_mark_as_delivered!
         ep
       end
       transient do
diff --git a/test/models/apple/episode_delivery_status_test.rb b/test/models/apple/episode_delivery_status_test.rb
index 04cf8e3e0..ed1cc51cb 100644
--- a/test/models/apple/episode_delivery_status_test.rb
+++ b/test/models/apple/episode_delivery_status_test.rb
@@ -14,7 +14,7 @@ class Apple::EpisodeDeliveryStatusTest < ActiveSupport::TestCase
         episode.destroy
         assert_equal episode, delivery_status.episode
         assert_difference "Apple::EpisodeDeliveryStatus.count", +1 do
-          episode.apple_needs_delivery!
+          episode.apple_mark_as_not_delivered!
         end
         assert_equal episode, episode.apple_episode_delivery_statuses.first.episode
       end

From 9da9dd2774c218f983a922d5ffe0740700b15bc0 Mon Sep 17 00:00:00 2001
From: Sam Vevang <sam.vevang@gmail.com>
Date: Tue, 29 Oct 2024 14:13:16 -0500
Subject: [PATCH 02/16] Fix quirk on soft deleted dependent destroy

---
 app/models/concerns/apple_delivery.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/concerns/apple_delivery.rb b/app/models/concerns/apple_delivery.rb
index 2be30403b..261acdc1a 100644
--- a/app/models/concerns/apple_delivery.rb
+++ b/app/models/concerns/apple_delivery.rb
@@ -11,7 +11,7 @@ module AppleDelivery
       class_name: "Apple::PodcastDelivery"
     has_many :apple_podcast_delivery_files, through: :apple_podcast_deliveries, source: :podcast_delivery_files,
       class_name: "Apple::PodcastDeliveryFile"
-    has_many :apple_episode_delivery_statuses, -> { order(created_at: :desc) }, dependent: :destroy, class_name: "Apple::EpisodeDeliveryStatus"
+    has_many :apple_episode_delivery_statuses, -> { order(created_at: :desc) }, class_name: "Apple::EpisodeDeliveryStatus"
 
     alias_method :podcast_container, :apple_podcast_container
     alias_method :apple_status, :apple_episode_delivery_status

From 2aa0e4d07f1f0d42893e1ce6e02e91e91d7e0651 Mon Sep 17 00:00:00 2001
From: Sam Vevang <sam.vevang@gmail.com>
Date: Tue, 29 Oct 2024 14:13:38 -0500
Subject: [PATCH 03/16] Move in related methods

---
 app/models/concerns/apple_delivery.rb | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/app/models/concerns/apple_delivery.rb b/app/models/concerns/apple_delivery.rb
index 261acdc1a..fe09c9f2d 100644
--- a/app/models/concerns/apple_delivery.rb
+++ b/app/models/concerns/apple_delivery.rb
@@ -45,6 +45,26 @@ def apple_mark_as_delivered!
     apple_episode_delivery_status.mark_as_delivered!
   end
 
+  def apple_mark_as_uploaded!
+    apple_episode_delivery_status.mark_as_uploaded!
+  end
+
+  def apple_mark_as_not_uploaded!
+    apple_episode_delivery_status.mark_as_not_uploaded!
+  end
+
+  def apple_prepare_for_delivery!
+    # remove the previous delivery attempt (soft delete)
+    apple_podcast_deliveries.map(&:destroy)
+    apple_podcast_deliveries.reset
+    apple_podcast_delivery_files.reset
+    apple_podcast_container&.podcast_deliveries&.reset
+  end
+
+  def apple_mark_for_reupload!
+    apple_mark_as_not_delivered!
+  end
+
   def measure_asset_processing_duration
     statuses = apple_episode_delivery_statuses.to_a
 

From fd6c5cf9c98a38101749cb5ccf218d2563677b37 Mon Sep 17 00:00:00 2001
From: Sam Vevang <sam.vevang@gmail.com>
Date: Tue, 29 Oct 2024 14:21:27 -0500
Subject: [PATCH 04/16] Break up method refactor

---
 app/models/apple/publisher.rb | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/app/models/apple/publisher.rb b/app/models/apple/publisher.rb
index 81e7ef81a..4cfc6d192 100644
--- a/app/models/apple/publisher.rb
+++ b/app/models/apple/publisher.rb
@@ -143,9 +143,12 @@ def deliver_and_publish!(eps)
           sync_podcast_deliveries!(eps)
           sync_podcast_delivery_files!(eps)
 
-          # upload and mark as uploaded
+          # upload and mark as uploaded, then update the audio container reference
           execute_upload_operations!(eps)
           mark_delivery_files_uploaded!(eps)
+          update_audio_container_reference!(eps)
+
+          mark_as_delivered!(eps)
 
           wait_for_upload_processing(eps)
 
@@ -361,16 +364,24 @@ def mark_delivery_files_uploaded!(eps)
       Rails.logger.tagged("##{__method__}") do
         pdfs = eps.map(&:podcast_delivery_files).flatten
         ::Apple::PodcastDeliveryFile.mark_uploaded(api, pdfs)
+      end
+    end
 
+    def update_audio_container_reference!(eps)
+      Rails.logger.tagged("##{__method__}") do
         # link the podcast container with the audio to the episode
         res = Apple::Episode.update_audio_container_reference(api, eps)
-        # update the feeder episode to indicate that delivery is no longer needed
+
+        Rails.logger.info("Updated remote container references for episodes.", {count: res.length})
+      end
+    end
+
+    def mark_as_delivered!(eps)
+      Rails.logger.tagged("##{__method__}") do
         eps.each do |ep|
           Rails.logger.info("Marking episode as no longer needing delivery", {episode_id: ep.feeder_episode.id})
           ep.feeder_episode.apple_mark_as_delivered!
         end
-
-        Rails.logger.info("Updated remote container references for episodes.", {count: res.length})
       end
     end
 

From 7ea78961fb1d381fa03e77f8bc993e5195e66e7b Mon Sep 17 00:00:00 2001
From: Sam Vevang <sam.vevang@gmail.com>
Date: Tue, 29 Oct 2024 14:31:06 -0500
Subject: [PATCH 05/16] Add the uploaded state marker

---
 app/models/apple/episode_delivery_status.rb          | 8 ++++++++
 db/migrate/20241029181938_new_file_uploaded_state.rb | 5 +++++
 db/schema.rb                                         | 3 ++-
 3 files changed, 15 insertions(+), 1 deletion(-)
 create mode 100644 db/migrate/20241029181938_new_file_uploaded_state.rb

diff --git a/app/models/apple/episode_delivery_status.rb b/app/models/apple/episode_delivery_status.rb
index c78e58a20..42416239f 100644
--- a/app/models/apple/episode_delivery_status.rb
+++ b/app/models/apple/episode_delivery_status.rb
@@ -22,6 +22,14 @@ def reset_asset_wait
       self.class.update_status(episode, asset_processing_attempts: 0)
     end
 
+    def mark_as_uploaded!
+      self.class.update_status(episode, uploaded: true)
+    end
+
+    def mark_as_not_uploaded!
+      self.class.update_status(episode, uploaded: false)
+    end
+
     def mark_as_delivered!
       self.class.update_status(episode, delivered: true)
     end
diff --git a/db/migrate/20241029181938_new_file_uploaded_state.rb b/db/migrate/20241029181938_new_file_uploaded_state.rb
new file mode 100644
index 000000000..c5ca9b5e5
--- /dev/null
+++ b/db/migrate/20241029181938_new_file_uploaded_state.rb
@@ -0,0 +1,5 @@
+class NewFileUploadedState < ActiveRecord::Migration[7.2]
+  def change
+    add_column :apple_episode_delivery_statuses, :uploaded, :boolean, default: false
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 235f715d7..45e036f4a 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.2].define(version: 2024_10_02_215949) do
+ActiveRecord::Schema[7.2].define(version: 2024_10_29_181938) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
   enable_extension "uuid-ossp"
@@ -37,6 +37,7 @@
     t.integer "source_fetch_count", default: 0
     t.bigint "source_media_version_id"
     t.integer "asset_processing_attempts", default: 0, null: false
+    t.boolean "uploaded", default: false
     t.index ["episode_id", "created_at"], name: "index_apple_episode_delivery_statuses_on_episode_id_created_at", include: ["delivered", "id"]
     t.index ["episode_id"], name: "index_apple_episode_delivery_statuses_on_episode_id"
   end

From f2fc12904ff86630fe57cec240964435932d6d8a Mon Sep 17 00:00:00 2001
From: Sam Vevang <sam.vevang@gmail.com>
Date: Wed, 30 Oct 2024 09:40:26 -0500
Subject: [PATCH 06/16] Break out the upload flow

---
 app/models/apple/podcast_delivery.rb |  6 ----
 app/models/apple/publisher.rb        | 47 +++++++++++++++++-----------
 2 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/app/models/apple/podcast_delivery.rb b/app/models/apple/podcast_delivery.rb
index 9b083ae01..861bdc5d6 100644
--- a/app/models/apple/podcast_delivery.rb
+++ b/app/models/apple/podcast_delivery.rb
@@ -73,12 +73,6 @@ def self.create_podcast_deliveries(api, episodes)
       end
 
       # Don't create deliveries for containers that already have deliveries.
-      # An alternative workflow would be to swap out the existing delivery and
-      # upload different audio.
-      #
-      # The overall publishing workflow dependes on the assumption that there is
-      # a delivery present. If we don't create a delivery here, we short-circuit
-      # subsequent steps (no uploads, no audio linking).
       episodes = select_episodes_for_delivery(episodes)
       podcast_containers = episodes.map(&:podcast_container)
 
diff --git a/app/models/apple/publisher.rb b/app/models/apple/publisher.rb
index 4cfc6d192..a4e680ad1 100644
--- a/app/models/apple/publisher.rb
+++ b/app/models/apple/publisher.rb
@@ -131,30 +131,35 @@ def publish!
     def deliver_and_publish!(eps)
       Rails.logger.tagged("Apple::Publisher#deliver_and_publish!") do
         eps.each_slice(PUBLISH_CHUNK_LEN) do |eps|
-          # Soft delete any existing delivery and delivery files
-          prepare_for_delivery!(eps)
+          # First upload the audio files
+          eps.filter(&:apple_needs_upload?).tap do |eps|
+            # Soft delete any existing delivery and delivery files
+            prepare_for_delivery!(eps)
 
-          # only create if needed
-          sync_episodes!(eps)
-          sync_podcast_containers!(eps)
+            # only create if needed
+            sync_episodes!(eps)
+            sync_podcast_containers!(eps)
 
-          wait_for_versioned_source_metadata(eps)
+            wait_for_versioned_source_metadata(eps)
 
-          sync_podcast_deliveries!(eps)
-          sync_podcast_delivery_files!(eps)
+            sync_podcast_deliveries!(eps)
+            sync_podcast_delivery_files!(eps)
 
-          # upload and mark as uploaded, then update the audio container reference
-          execute_upload_operations!(eps)
-          mark_delivery_files_uploaded!(eps)
-          update_audio_container_reference!(eps)
+            # upload and mark as uploaded, then update the audio container reference
+            execute_upload_operations!(eps)
+            mark_delivery_files_uploaded!(eps)
+            update_audio_container_reference!(eps)
+            # finally mark as uploaded
+            mark_as_uploaded!(eps)
+          end
 
-          mark_as_delivered!(eps)
+          increment_asset_wait!(eps)
 
           wait_for_upload_processing(eps)
-
-          increment_asset_wait!(eps)
           wait_for_asset_state(eps)
 
+          mark_as_delivered!(eps)
+
           publish_drafting!(eps)
           reset_asset_wait!(eps)
 
@@ -234,9 +239,6 @@ def wait_for_upload_processing(eps)
 
     def increment_asset_wait!(eps)
       Rails.logger.tagged("##{__method__}") do
-        eps = eps.filter { |e| e.podcast_delivery_files.any?(&:api_marked_as_uploaded?) }
-
-        # Mark the episodes as waiting again for asset processing
         eps.each { |ep| ep.apple_episode_delivery_status.increment_asset_wait }
       end
     end
@@ -385,6 +387,15 @@ def mark_as_delivered!(eps)
       end
     end
 
+    def mark_as_uploaded!(eps)
+      Rails.logger.tagged("##{__method__}") do
+        eps.each do |ep|
+          Rails.logger.info("Marking episode as no longer needing delivery", {episode_id: ep.feeder_episode.id})
+          ep.feeder_episode.apple_mark_as_uploaded!
+        end
+      end
+    end
+
     def publish_drafting!(eps)
       Rails.logger.tagged("##{__method__}") do
         eps = eps.select { |ep| ep.drafting? && ep.container_upload_complete? }

From 7d7ead79f0cd4e831a19ce605be4c857935b3c1c Mon Sep 17 00:00:00 2001
From: Sam Vevang <sam.vevang@gmail.com>
Date: Thu, 31 Oct 2024 10:44:06 -0500
Subject: [PATCH 07/16] Split out coverage for update reference

---
 test/factories/apple_episode_api_response.rb |  3 ++-
 test/factories/apple_episode_factory.rb      |  6 +++++-
 test/models/apple/publisher_test.rb          | 17 +++++++++++++++++
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/test/factories/apple_episode_api_response.rb b/test/factories/apple_episode_api_response.rb
index cf46ca04f..1021120e8 100644
--- a/test/factories/apple_episode_api_response.rb
+++ b/test/factories/apple_episode_api_response.rb
@@ -15,13 +15,14 @@
 
     after(:build) do |response_container, evaluator|
       response_container["api_response"] =
-        {"request_metadata" => {"apple_episode_id" => evaluator.apple_episode_id, "item_guid" => evaluator.item_guid},
+        {"request_metadata" => {"apple_episode_id" => evaluator.apple_episode_id, "guid" => evaluator.item_guid},
          "api_url" => evaluator.api_url,
          "api_parameters" => {},
          "api_response" => {"ok" => evaluator.ok,
                             "err" => evaluator.err,
                             "val" => {"data" => {"id" => "123",
                                                  "attributes" => {
+                                                   "appleHostedAudioAssetContainerId" => nil,
                                                    "appleHostedAudioAssetVendorId" => evaluator.apple_hosted_audio_asset_container_id,
                                                    "publishingState" => evaluator.publishing_state,
                                                    "guid" => evaluator.item_guid,
diff --git a/test/factories/apple_episode_factory.rb b/test/factories/apple_episode_factory.rb
index 1d2cb31e1..97052d6ca 100644
--- a/test/factories/apple_episode_factory.rb
+++ b/test/factories/apple_episode_factory.rb
@@ -6,7 +6,8 @@
     # set up transient api_response
     transient do
       feeder_episode { create(:episode) }
-      api_response { build(:apple_episode_api_response) }
+      api_response { build(:apple_episode_api_response, item_guid: feeder_episode.item_guid) }
+      apple_hosted_audio_asset_container_id { "456" }
     end
 
     # set a complete episode factory varient
@@ -17,9 +18,12 @@
         ep
       end
       transient do
+        apple_hosted_audio_asset_container_id { "456" }
         api_response do
           build(:apple_episode_api_response,
             publishing_state: "PUBLISH",
+            item_guid: feeder_episode.item_guid,
+            apple_hosted_audio_asset_container_id: apple_hosted_audio_asset_container_id,
             apple_hosted_audio_state: Apple::Episode::AUDIO_ASSET_SUCCESS)
         end
       end
diff --git a/test/models/apple/publisher_test.rb b/test/models/apple/publisher_test.rb
index 20ef7da77..3be35700f 100644
--- a/test/models/apple/publisher_test.rb
+++ b/test/models/apple/publisher_test.rb
@@ -527,4 +527,21 @@
       end
     end
   end
+
+  describe "#update_audio_container_reference!" do
+    let(:episode) { build(:uploaded_apple_episode, show: apple_publisher.show, apple_hosted_audio_asset_container_id: nil) }
+
+    it "updates container references for episodes" do
+      assert episode.has_unlinked_container?
+
+      mock_result = episode.apple_sync_log.api_response.deep_dup
+      mock_result["api_response"]["val"]["data"]["attributes"]["appleHostedAudioAssetContainerId"] = "456"
+
+      apple_publisher.api.stub(:bridge_remote_and_retry, [[mock_result], []]) do
+        apple_publisher.update_audio_container_reference!([episode])
+      end
+
+      refute episode.has_unlinked_container?
+    end
+  end
 end

From 0a94b59a7850378af96317c0287639e9a3f24de4 Mon Sep 17 00:00:00 2001
From: Sam Vevang <sam.vevang@gmail.com>
Date: Thu, 31 Oct 2024 10:46:47 -0500
Subject: [PATCH 08/16] fixup! Add the uploaded state marker

---
 test/models/apple/publisher_test.rb | 36 +++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/test/models/apple/publisher_test.rb b/test/models/apple/publisher_test.rb
index 3be35700f..7ed6019d6 100644
--- a/test/models/apple/publisher_test.rb
+++ b/test/models/apple/publisher_test.rb
@@ -528,6 +528,42 @@
     end
   end
 
+  describe "#mark_as_uploaded!" do
+    let(:episode1) { build(:uploaded_apple_episode, show: apple_publisher.show) }
+    let(:episode2) { build(:uploaded_apple_episode, show: apple_publisher.show) }
+    let(:episodes) { [episode1, episode2] }
+
+    it "marks episodes as uploaded" do
+      episodes.each do |ep|
+        refute ep.delivery_status.uploaded
+      end
+
+      apple_publisher.mark_as_uploaded!(episodes)
+
+      episodes.each do |ep|
+        assert ep.delivery_status.uploaded
+      end
+    end
+  end
+
+  describe "#mark_as_delivered!" do
+    let(:episode1) { build(:apple_episode, show: apple_publisher.show) }
+    let(:episode2) { build(:apple_episode, show: apple_publisher.show) }
+    let(:episodes) { [episode1, episode2] }
+
+    it "marks episodes as delivered" do
+      episodes.each do |ep|
+        refute ep.delivery_status.delivered
+      end
+
+      apple_publisher.mark_as_delivered!(episodes)
+
+      episodes.each do |ep|
+        assert ep.delivery_status.delivered
+      end
+    end
+  end
+
   describe "#update_audio_container_reference!" do
     let(:episode) { build(:uploaded_apple_episode, show: apple_publisher.show, apple_hosted_audio_asset_container_id: nil) }
 

From f9793b27783b70753e9b3fee0b0d215f34f81162 Mon Sep 17 00:00:00 2001
From: Sam Vevang <sam.vevang@gmail.com>
Date: Thu, 31 Oct 2024 11:08:44 -0500
Subject: [PATCH 09/16] Break out methods for testing

---
 app/models/apple/episode.rb                 | 18 +++++--
 app/models/apple/episode_delivery_status.rb |  6 ++-
 app/models/apple/publisher.rb               | 59 ++++++++++++---------
 app/models/concerns/apple_delivery.rb       |  4 ++
 test/models/apple/episode_test.rb           | 17 +-----
 test/models/apple/publisher_test.rb         | 44 ++++++++++++---
 6 files changed, 96 insertions(+), 52 deletions(-)

diff --git a/app/models/apple/episode.rb b/app/models/apple/episode.rb
index 9d8a2c801..b90668707 100644
--- a/app/models/apple/episode.rb
+++ b/app/models/apple/episode.rb
@@ -15,10 +15,8 @@ class Episode
     EPISODE_ASSET_WAIT_TIMEOUT = 15.minutes.freeze
     EPISODE_ASSET_WAIT_INTERVAL = 10.seconds.freeze
 
-    # Cleans up old delivery/delivery files iff the episode is to be delivered
+    # Cleans up old delivery/delivery files iff the episode is to be uploaded
     def self.prepare_for_delivery(episodes)
-      episodes = episodes.select { |ep| ep.needs_delivery? }
-
       episodes.map do |ep|
         Rails.logger.info("Preparing episode #{ep.feeder_id} for delivery", {episode_id: ep.feeder_id})
         ep.feeder_episode.apple_prepare_for_delivery!
@@ -567,5 +565,19 @@ def apple_episode_delivery_statuses
     alias_method :delivery_files, :podcast_delivery_files
     alias_method :delivery_status, :apple_episode_delivery_status
     alias_method :delivery_statuses, :apple_episode_delivery_statuses
+    alias_method :apple_status, :apple_episode_delivery_status
+
+    # Delegate methods to feeder_episode
+    def method_missing(method_name, *arguments, &block)
+      if feeder_episode.respond_to?(method_name)
+        feeder_episode.send(method_name, *arguments, &block)
+      else
+        super
+      end
+    end
+
+    def respond_to_missing?(method_name, include_private = false)
+      feeder_episode.respond_to?(method_name) || super
+    end
   end
 end
diff --git a/app/models/apple/episode_delivery_status.rb b/app/models/apple/episode_delivery_status.rb
index 42416239f..eef9e84bb 100644
--- a/app/models/apple/episode_delivery_status.rb
+++ b/app/models/apple/episode_delivery_status.rb
@@ -30,12 +30,14 @@ def mark_as_not_uploaded!
       self.class.update_status(episode, uploaded: false)
     end
 
+    # Whether the media file has been uploaded to Apple
+    # is a subset of whether the episode has been delivered
     def mark_as_delivered!
-      self.class.update_status(episode, delivered: true)
+      self.class.update_status(episode, delivered: true, uploaded: true)
     end
 
     def mark_as_not_delivered!
-      self.class.update_status(episode, delivered: false)
+      self.class.update_status(episode, delivered: false, uploaded: false)
     end
   end
 end
diff --git a/app/models/apple/publisher.rb b/app/models/apple/publisher.rb
index a4e680ad1..43b30e540 100644
--- a/app/models/apple/publisher.rb
+++ b/app/models/apple/publisher.rb
@@ -131,41 +131,49 @@ def publish!
     def deliver_and_publish!(eps)
       Rails.logger.tagged("Apple::Publisher#deliver_and_publish!") do
         eps.each_slice(PUBLISH_CHUNK_LEN) do |eps|
-          # First upload the audio files
           eps.filter(&:apple_needs_upload?).tap do |eps|
-            # Soft delete any existing delivery and delivery files
-            prepare_for_delivery!(eps)
+            upload_media!(eps)
+          end
 
-            # only create if needed
-            sync_episodes!(eps)
-            sync_podcast_containers!(eps)
+          process_and_deliver!(eps)
 
-            wait_for_versioned_source_metadata(eps)
+          raise_delivery_processing_errors(eps)
+        end
+      end
+    end
 
-            sync_podcast_deliveries!(eps)
-            sync_podcast_delivery_files!(eps)
+    def upload_media!(eps)
+      # Soft delete any existing delivery and delivery files
+      prepare_for_delivery!(eps)
 
-            # upload and mark as uploaded, then update the audio container reference
-            execute_upload_operations!(eps)
-            mark_delivery_files_uploaded!(eps)
-            update_audio_container_reference!(eps)
-            # finally mark as uploaded
-            mark_as_uploaded!(eps)
-          end
+      # only create if needed
+      sync_episodes!(eps)
+      sync_podcast_containers!(eps)
 
-          increment_asset_wait!(eps)
+      wait_for_versioned_source_metadata(eps)
 
-          wait_for_upload_processing(eps)
-          wait_for_asset_state(eps)
+      sync_podcast_deliveries!(eps)
+      sync_podcast_delivery_files!(eps)
 
-          mark_as_delivered!(eps)
+      # upload and mark as uploaded, then update the audio container reference
+      execute_upload_operations!(eps)
+      mark_delivery_files_uploaded!(eps)
+      update_audio_container_reference!(eps)
 
-          publish_drafting!(eps)
-          reset_asset_wait!(eps)
+      # finally mark the episode as uploaded
+      mark_as_uploaded!(eps)
+    end
 
-          raise_delivery_processing_errors(eps)
-        end
-      end
+    def process_and_deliver!(eps)
+      increment_asset_wait!(eps)
+
+      wait_for_upload_processing(eps)
+      wait_for_asset_state(eps)
+
+      mark_as_delivered!(eps)
+
+      publish_drafting!(eps)
+      reset_asset_wait!(eps)
     end
 
     def prepare_for_delivery!(eps)
@@ -239,6 +247,7 @@ def wait_for_upload_processing(eps)
 
     def increment_asset_wait!(eps)
       Rails.logger.tagged("##{__method__}") do
+        eps = eps.filter { |e| e.feeder_episode.apple_status.uploaded? }
         eps.each { |ep| ep.apple_episode_delivery_status.increment_asset_wait }
       end
     end
diff --git a/app/models/concerns/apple_delivery.rb b/app/models/concerns/apple_delivery.rb
index fe09c9f2d..02d3afbe3 100644
--- a/app/models/concerns/apple_delivery.rb
+++ b/app/models/concerns/apple_delivery.rb
@@ -37,6 +37,10 @@ def apple_needs_delivery?
     apple_episode_delivery_status.delivered == false
   end
 
+  def apple_needs_upload?
+    apple_episode_delivery_status.uploaded == false
+  end
+
   def apple_mark_as_not_delivered!
     apple_episode_delivery_status.mark_as_not_delivered!
   end
diff --git a/test/models/apple/episode_test.rb b/test/models/apple/episode_test.rb
index 30e8d8feb..2cefff029 100644
--- a/test/models/apple/episode_test.rb
+++ b/test/models/apple/episode_test.rb
@@ -274,22 +274,7 @@
 
   describe ".prepare_for_delivery" do
     it "should filter for episodes that need delivery" do
-      mock = Minitest::Mock.new
-      mock.expect(:call, true, [])
-
-      apple_episode.feeder_episode.stub(:apple_prepare_for_delivery!, mock) do
-        apple_episode.stub(:needs_delivery?, true) do
-          assert_equal [apple_episode], Apple::Episode.prepare_for_delivery([apple_episode])
-        end
-      end
-
-      mock.verify
-    end
-
-    it "should reject delivered episodes" do
-      apple_episode.stub(:needs_delivery?, false) do
-        assert_equal [], Apple::Episode.prepare_for_delivery([apple_episode])
-      end
+      assert_equal [apple_episode], Apple::Episode.prepare_for_delivery([apple_episode])
     end
 
     describe "soft deleting the delivery files" do
diff --git a/test/models/apple/publisher_test.rb b/test/models/apple/publisher_test.rb
index 7ed6019d6..390dfe60c 100644
--- a/test/models/apple/publisher_test.rb
+++ b/test/models/apple/publisher_test.rb
@@ -413,6 +413,7 @@
     it "should increment asset wait count for each episode" do
       episodes.each do |ep|
         assert_equal 0, ep.apple_episode_delivery_status.asset_processing_attempts
+        ep.feeder_episode.apple_mark_as_uploaded!
       end
 
       apple_publisher.increment_asset_wait!(episodes)
@@ -422,15 +423,12 @@
       end
     end
 
-    it "should only increment the episodes that are still waiting" do
+    it "only increments the episodes that are still waiting" do
       assert 1, episode1.podcast_delivery_files.length
       assert 1, episode2.podcast_delivery_files.length
 
-      episode2.podcast_delivery_files.first.stub(:api_marked_as_uploaded?, false) do
-        episode1.podcast_delivery_files.first.stub(:api_marked_as_uploaded?, true) do
-          apple_publisher.increment_asset_wait!(episodes)
-        end
-      end
+      episode1.feeder_episode.apple_mark_as_uploaded!
+      apple_publisher.increment_asset_wait!(episodes)
 
       assert_equal [1, 0], [episode1, episode2].map { |ep| ep.apple_episode_delivery_status.asset_processing_attempts }
     end
@@ -580,4 +578,38 @@
       refute episode.has_unlinked_container?
     end
   end
+
+  describe "#deliver_and_publish!" do
+    let(:episode) { build(:uploaded_apple_episode, show: apple_publisher.show) }
+
+    it "skips upload for already uploaded episodes" do
+      episode.feeder_episode.apple_mark_as_uploaded!
+
+      mock = Minitest::Mock.new
+      episode.feeder_episode.stub(:apple_prepare_for_delivery!, ->(*) { raise "Should not be called" }) do
+        mock.expect(:call, nil, [[]])
+        apple_publisher.stub(:upload_media!, mock) do
+          apple_publisher.stub(:process_and_deliver!, ->(*) {}) do
+            apple_publisher.deliver_and_publish!([episode])
+          end
+        end
+      end
+
+      assert mock.verify
+    end
+
+    it "processes uploads for non-uploaded episodes" do
+      refute episode.delivery_status.uploaded
+
+      mock = Minitest::Mock.new
+      mock.expect(:call, nil, [[episode]])
+      apple_publisher.stub(:upload_media!, mock) do
+        apple_publisher.stub(:process_and_deliver!, ->(*) {}) do
+          apple_publisher.deliver_and_publish!([episode])
+        end
+      end
+
+      mock.verify
+    end
+  end
 end

From a6dfab1e70a1d01381e93941c8af45f3bdd83f5b Mon Sep 17 00:00:00 2001
From: Sam Vevang <sam.vevang@gmail.com>
Date: Tue, 5 Nov 2024 09:28:34 -0600
Subject: [PATCH 10/16] Fix return value

---
 app/models/concerns/apple_delivery.rb       |  2 +-
 test/models/concerns/apple_delivery_test.rb | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/app/models/concerns/apple_delivery.rb b/app/models/concerns/apple_delivery.rb
index 02d3afbe3..fd99e1f8f 100644
--- a/app/models/concerns/apple_delivery.rb
+++ b/app/models/concerns/apple_delivery.rb
@@ -18,7 +18,7 @@ module AppleDelivery
   end
 
   def publish_to_apple?
-    podcast.apple_config&.publish_to_apple?
+    podcast.apple_config&.publish_to_apple? || false
   end
 
   def apple_update_delivery_status(attrs)
diff --git a/test/models/concerns/apple_delivery_test.rb b/test/models/concerns/apple_delivery_test.rb
index 285fdf858..c077587eb 100644
--- a/test/models/concerns/apple_delivery_test.rb
+++ b/test/models/concerns/apple_delivery_test.rb
@@ -75,4 +75,24 @@ class AppleDeliveryTest < ActiveSupport::TestCase
       assert_equal episode.apple_episode_delivery_statuses.last, result
     end
   end
+
+  describe "#publish_to_apple?" do
+    let(:episode) { create(:episode) }
+
+    it "returns false when podcast has no apple config" do
+      refute episode.publish_to_apple?
+    end
+
+    it "returns false when apple config exists but publishing disabled" do
+      create(:apple_config, feed: create(:private_feed, podcast: episode.podcast), publish_enabled: false)
+      refute episode.publish_to_apple?
+    end
+
+    it "returns true when apple config exists and publishing enabled" do
+      assert episode.publish_to_apple? == false
+      create(:apple_config, feed: create(:private_feed, podcast: episode.podcast), publish_enabled: true)
+      episode.podcast.reload
+      assert episode.publish_to_apple?
+    end
+  end
 end

From 7ba1c29fa18fa6bdfd8be6c772490852aa39f19b Mon Sep 17 00:00:00 2001
From: Sam Vevang <sam.vevang@gmail.com>
Date: Tue, 5 Nov 2024 09:29:04 -0600
Subject: [PATCH 11/16] Remove dupe methods

---
 app/models/concerns/apple_delivery.rb       | 12 -------
 test/models/concerns/apple_delivery_test.rb | 35 +++++++++++++++++++--
 2 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/app/models/concerns/apple_delivery.rb b/app/models/concerns/apple_delivery.rb
index fd99e1f8f..0ed1730a5 100644
--- a/app/models/concerns/apple_delivery.rb
+++ b/app/models/concerns/apple_delivery.rb
@@ -81,18 +81,6 @@ def measure_asset_processing_duration
     Time.now - start_status.created_at
   end
 
-  def apple_prepare_for_delivery!
-    # remove the previous delivery attempt (soft delete)
-    apple_podcast_deliveries.map(&:destroy)
-    apple_podcast_deliveries.reset
-    apple_podcast_delivery_files.reset
-    apple_podcast_container&.podcast_deliveries&.reset
-  end
-
-  def apple_mark_for_reupload!
-    apple_needs_delivery!
-  end
-
   def apple_episode
     return nil if !persisted? || !publish_to_apple?
 
diff --git a/test/models/concerns/apple_delivery_test.rb b/test/models/concerns/apple_delivery_test.rb
index c077587eb..239603576 100644
--- a/test/models/concerns/apple_delivery_test.rb
+++ b/test/models/concerns/apple_delivery_test.rb
@@ -18,16 +18,45 @@ class AppleDeliveryTest < ActiveSupport::TestCase
     end
 
     it "can be set to false" do
-      episode.apple_has_delivery!
+      episode.apple_mark_as_delivered!
       refute episode.apple_needs_delivery?
     end
 
     it "can be set to true" do
-      episode.apple_has_delivery!
+      episode.apple_mark_as_delivered!
       refute episode.apple_needs_delivery?
 
       # now set it to true
-      episode.apple_needs_delivery!
+      episode.apple_mark_as_not_delivered!
+      assert episode.apple_needs_delivery?
+    end
+  end
+
+  describe "#apple_mark_as_delivered!" do
+    let(:episode) { create(:episode) }
+
+    it "supercedes the uploaded status" do
+      episode.apple_mark_as_not_delivered!
+
+      assert episode.apple_needs_upload?
+      assert episode.apple_needs_delivery?
+
+      episode.apple_mark_as_delivered!
+
+      refute episode.apple_needs_upload?
+      refute episode.apple_needs_delivery?
+    end
+  end
+
+  describe "#apple_mark_as_uploaded!" do
+    it "sets the uploaded status" do
+      episode.apple_mark_as_uploaded!
+      assert episode.apple_episode_delivery_status.uploaded
+      refute episode.apple_needs_upload?
+    end
+
+    it "does not interact with the delivery status" do
+      episode.apple_mark_as_uploaded!
       assert episode.apple_needs_delivery?
     end
   end

From 53a95a065000d38aee3391f23352f031239f58bc Mon Sep 17 00:00:00 2001
From: Sam Vevang <sam.vevang@gmail.com>
Date: Tue, 5 Nov 2024 09:29:34 -0600
Subject: [PATCH 12/16] Coverage for apple_prepare_for_delivery

---
 test/models/concerns/apple_delivery_test.rb | 25 +++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/test/models/concerns/apple_delivery_test.rb b/test/models/concerns/apple_delivery_test.rb
index 239603576..51bea7100 100644
--- a/test/models/concerns/apple_delivery_test.rb
+++ b/test/models/concerns/apple_delivery_test.rb
@@ -124,4 +124,29 @@ class AppleDeliveryTest < ActiveSupport::TestCase
       assert episode.publish_to_apple?
     end
   end
+
+  describe "#apple_prepare_for_delivery!" do
+    let(:episode) { create(:episode) }
+    let(:container) { create(:apple_podcast_container, episode: episode) }
+    let(:delivery) { create(:apple_podcast_delivery, episode: episode, podcast_container: container) }
+    let(:delivery_file) { create(:apple_podcast_delivery_file, episode: episode, podcast_delivery: delivery) }
+
+    before do
+      delivery_file # Create the delivery file
+    end
+
+    it "soft deletes existing deliveries" do
+      assert_equal 1, episode.apple_podcast_deliveries.count
+      episode.apple_prepare_for_delivery!
+      assert_equal 0, episode.apple_podcast_deliveries.count
+      assert_equal 1, episode.apple_podcast_deliveries.with_deleted.count
+    end
+
+    it "resets associations" do
+      episode.apple_prepare_for_delivery!
+      refute episode.apple_podcast_deliveries.loaded?
+      refute episode.apple_podcast_delivery_files.loaded?
+      refute episode.apple_podcast_container.podcast_deliveries.loaded?
+    end
+  end
 end

From 3cc10c8efec75a3df249ae493329e2f40f917022 Mon Sep 17 00:00:00 2001
From: Sam Vevang <sam.vevang@gmail.com>
Date: Tue, 5 Nov 2024 09:40:48 -0600
Subject: [PATCH 13/16] Bifucated uploaded/delivered states match

---
 db/migrate/20241029181938_new_file_uploaded_state.rb | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/db/migrate/20241029181938_new_file_uploaded_state.rb b/db/migrate/20241029181938_new_file_uploaded_state.rb
index c5ca9b5e5..593f6593d 100644
--- a/db/migrate/20241029181938_new_file_uploaded_state.rb
+++ b/db/migrate/20241029181938_new_file_uploaded_state.rb
@@ -1,5 +1,12 @@
 class NewFileUploadedState < ActiveRecord::Migration[7.2]
   def change
     add_column :apple_episode_delivery_statuses, :uploaded, :boolean, default: false
+
+    # Set the new column to match the delivered column
+    execute(<<~SQL
+      UPDATE apple_episode_delivery_statuses
+      SET uploaded = delivered
+    SQL
+           )
   end
 end

From ab2a038fa19ccd2682fa018aca4cb707d2fea4a5 Mon Sep 17 00:00:00 2001
From: Sam Vevang <sam.vevang@gmail.com>
Date: Tue, 26 Nov 2024 09:09:55 -0600
Subject: [PATCH 14/16] Fixup factory initialization ep uploaded state

---
 test/factories/apple_episode_factory.rb | 21 ++++++++++++++++-----
 test/models/apple/publisher_test.rb     | 14 +++++++++-----
 2 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/test/factories/apple_episode_factory.rb b/test/factories/apple_episode_factory.rb
index 97052d6ca..889c2b34a 100644
--- a/test/factories/apple_episode_factory.rb
+++ b/test/factories/apple_episode_factory.rb
@@ -14,7 +14,6 @@
     factory :uploaded_apple_episode do
       feeder_episode do
         ep = create(:episode)
-        ep.apple_mark_as_delivered!
         ep
       end
       transient do
@@ -36,11 +35,23 @@
           api_marked_as_uploaded: true,
           upload_operations_complete: true)
 
-        create(:content, episode: apple_episode.feeder_episode, position: 1, status: "complete")
-        create(:content, episode: apple_episode.feeder_episode, position: 2, status: "complete")
-        v1 = apple_episode.feeder_episode.cut_media_version!
+        feeder_episode = apple_episode.feeder_episode
 
-        apple_episode.delivery_status.update!(delivered: true, source_media_version_id: v1.id)
+        # The content model calls Episode#publish!
+        # and that triggers a call to Episode#apple_mark_for_reupload!
+        # This modifies state to indicate that the episode needs to be reuploaded
+        create(:content, episode: feeder_episode, position: 1, status: "complete")
+        create(:content, episode: feeder_episode, position: 2, status: "complete")
+        v1 = feeder_episode.cut_media_version!
+
+        # Now model the case where the episode is uploaded.
+        # First we've gathered file metadata from the CDN
+        feeder_episode.apple_update_delivery_status(source_size: 1.megabyte,
+          source_url: "https://cdn.example.com/episode.mp3",
+          source_media_version_id: v1.id)
+
+        # Then we've uploaded as delivered (and necessarily uploaded)
+        feeder_episode.apple_mark_as_delivered!
       end
     end
 
diff --git a/test/models/apple/publisher_test.rb b/test/models/apple/publisher_test.rb
index 390dfe60c..b1149152a 100644
--- a/test/models/apple/publisher_test.rb
+++ b/test/models/apple/publisher_test.rb
@@ -406,8 +406,8 @@
   end
 
   describe "#increment_asset_wait!" do
-    let(:episode1) { build(:uploaded_apple_episode, show: apple_publisher.show) }
-    let(:episode2) { build(:uploaded_apple_episode, show: apple_publisher.show) }
+    let(:episode1) { build(:apple_episode, show: apple_publisher.show) }
+    let(:episode2) { build(:apple_episode, show: apple_publisher.show) }
     let(:episodes) { [episode1, episode2] }
 
     it "should increment asset wait count for each episode" do
@@ -527,19 +527,21 @@
   end
 
   describe "#mark_as_uploaded!" do
-    let(:episode1) { build(:uploaded_apple_episode, show: apple_publisher.show) }
-    let(:episode2) { build(:uploaded_apple_episode, show: apple_publisher.show) }
+    let(:episode1) { build(:apple_episode, show: apple_publisher.show) }
+    let(:episode2) { build(:apple_episode, show: apple_publisher.show) }
     let(:episodes) { [episode1, episode2] }
 
     it "marks episodes as uploaded" do
       episodes.each do |ep|
         refute ep.delivery_status.uploaded
+        refute ep.delivery_status.delivered
       end
 
       apple_publisher.mark_as_uploaded!(episodes)
 
       episodes.each do |ep|
         assert ep.delivery_status.uploaded
+        refute ep.delivery_status.delivered
       end
     end
   end
@@ -551,12 +553,14 @@
 
     it "marks episodes as delivered" do
       episodes.each do |ep|
+        refute ep.delivery_status.uploaded
         refute ep.delivery_status.delivered
       end
 
       apple_publisher.mark_as_delivered!(episodes)
 
       episodes.each do |ep|
+        assert ep.delivery_status.uploaded
         assert ep.delivery_status.delivered
       end
     end
@@ -580,7 +584,7 @@
   end
 
   describe "#deliver_and_publish!" do
-    let(:episode) { build(:uploaded_apple_episode, show: apple_publisher.show) }
+    let(:episode) { build(:apple_episode, show: apple_publisher.show) }
 
     it "skips upload for already uploaded episodes" do
       episode.feeder_episode.apple_mark_as_uploaded!

From e31a4339900143b782a9e99779312a645837a0e2 Mon Sep 17 00:00:00 2001
From: Sam Vevang <sam.vevang@gmail.com>
Date: Tue, 26 Nov 2024 09:17:32 -0600
Subject: [PATCH 15/16] All on one line

---
 test/factories/apple_episode_factory.rb | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/test/factories/apple_episode_factory.rb b/test/factories/apple_episode_factory.rb
index 889c2b34a..9d79b8cea 100644
--- a/test/factories/apple_episode_factory.rb
+++ b/test/factories/apple_episode_factory.rb
@@ -12,10 +12,7 @@
 
     # set a complete episode factory varient
     factory :uploaded_apple_episode do
-      feeder_episode do
-        ep = create(:episode)
-        ep
-      end
+      feeder_episode { create(:episode) }
       transient do
         apple_hosted_audio_asset_container_id { "456" }
         api_response do

From a1b644c842e5d6c48298f18fd5e4711112ad3c9f Mon Sep 17 00:00:00 2001
From: Sam Vevang <sam.vevang@gmail.com>
Date: Tue, 26 Nov 2024 09:17:40 -0600
Subject: [PATCH 16/16] Fixup words

---
 test/factories/apple_episode_factory.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/factories/apple_episode_factory.rb b/test/factories/apple_episode_factory.rb
index 9d79b8cea..c932ae860 100644
--- a/test/factories/apple_episode_factory.rb
+++ b/test/factories/apple_episode_factory.rb
@@ -47,7 +47,7 @@
           source_url: "https://cdn.example.com/episode.mp3",
           source_media_version_id: v1.id)
 
-        # Then we've uploaded as delivered (and necessarily uploaded)
+        # Then we've delivered (and necessarily uploaded)
         feeder_episode.apple_mark_as_delivered!
       end
     end