Merge branch 'main' into feat/apple_feed_subclass

app/controllers/episode_media_controller.rb

@@ -49,18 +49,22 @@ def update
         format.html { render :show, status: :unprocessable_entity }
       end
     end
+  rescue ActiveRecord::StaleObjectError
+    render :show, status: :conflict
   end
 
   private
 
   def set_episode
     @episode = Episode.find_by_guid!(params[:episode_id])
     @episode.strict_validations = true
+    @episode.locking_enabled = true
     @podcast = @episode.podcast
   end
 
   def episode_params
     nilify params.fetch(:episode, {}).permit(
+      :lock_version,
       :medium,
       :ad_breaks,
       contents_attributes: %i[id position original_url file_size _destroy _retry],

app/controllers/episodes_controller.rb

@@ -84,6 +84,8 @@ def update
         format.html { render :edit, status: :unprocessable_entity }
       end
     end
+  rescue ActiveRecord::StaleObjectError
+    render :edit, status: :conflict
   end
 
   # DELETE /episodes/1
@@ -108,6 +110,7 @@ def destroy
   def set_episode
     @episode = Episode.find_by_guid!(params[:id])
     @episode.strict_validations = true
+    @episode.locking_enabled = true
   end
 
   def set_podcast
@@ -136,6 +139,7 @@ def episodes_query
 
   def episode_params
     nilify(params.fetch(:episode, {}).permit(
+      :lock_version,
       :title,
       :clean_title,
       :subtitle,

app/controllers/feeds_controller.rb

@@ -59,6 +59,8 @@ def update
         end
       end
     end
+  rescue ActiveRecord::StaleObjectError
+    render :show, status: :conflict
   end
 
   # DELETE /feeds/1
@@ -94,6 +96,7 @@ def set_feeds
   # Use callbacks to share common setup or constraints between actions.
   def set_feed
     @feed = Feed.find(params[:id])
+    @feed.locking_enabled = true
   end
 
   # Only allow a list of trusted parameters through.
@@ -103,6 +106,7 @@ def feed_params
 
   def nilified_feed_params
     nilify params.fetch(:feed, {}).permit(
+      :lock_version,
       :file_name,
       :title,
       :subtitle,

app/controllers/podcast_engagement_controller.rb

@@ -17,13 +17,16 @@ def update
         format.html { render :show, status: :unprocessable_entity }
       end
     end
+  rescue ActiveRecord::StaleObjectError
+    render :show, status: :conflict
   end
 
   private
 
   # Use callbacks to share common setup or constraints between actions.
   def set_podcast
     @podcast = Podcast.find(params[:podcast_id])
+    @podcast.locking_enabled = true
     authorize @podcast
   end
 
@@ -32,6 +35,7 @@ def set_podcast
   ### TODO include params for socmed and podcast apps
   def podcast_engagement_params
     nilify params.fetch(:podcast, {}).permit(
+      :lock_version,
       :donation_url,
       :payment_pointer
     )

app/controllers/podcasts_controller.rb

@@ -76,6 +76,8 @@ def update
         format.html { render :edit, status: :unprocessable_entity }
       end
     end
+  rescue ActiveRecord::StaleObjectError
+    render :edit, status: :conflict
   end
 
   # DELETE /podcasts/1
@@ -98,6 +100,7 @@ def destroy
 
   def set_podcast
     @podcast = Podcast.find(params[:id])
+    @podcast.locking_enabled = true
   end
 
   def published_episodes(date_range)
@@ -107,6 +110,7 @@ def published_episodes(date_range)
 
   def podcast_params
     nilify params.fetch(:podcast, {}).permit(
+      :lock_version,
       :title,
       :prx_account_uri,
       :link,

app/javascript/controllers/toggle_value_controller.js

@@ -0,0 +1,12 @@
+import { Controller } from "@hotwired/stimulus"
+
+export default class extends Controller {
+  static targets = ["field"]
+  static values = { next: String }
+
+  toggle() {
+    const prev = this.fieldTarget.value
+    this.fieldTarget.value = this.nextValue
+    this.nextValue = prev
+  }
+}

app/models/apple/publisher.rb

@@ -252,14 +252,20 @@ def sync_episodes!(eps)
         poll_episodes!(eps)
 
         create_apple_episodes = eps.select(&:apple_new?)
-        # NOTE: We don't attempt to update the remote state of episodes. Once
-        # apple has parsed the feed, it will not allow changing any attributes.
+        Apple::Episode.create_episodes(api, create_apple_episodes)
+        Rails.logger.info("Created remote episodes", {count: create_apple_episodes.length})
+
+        # NOTE: We don't attempt to update the remote state of published episodes.
+        # Once apple has parsed the feed, it will not allow changing any attributes.
         #
         # It's assumed that the episodes are created solely by the PRX web UI (not
         # on Podcasts Connect).
-        Apple::Episode.create_episodes(api, create_apple_episodes)
 
-        Rails.logger.info("Created remote episodes", {count: create_apple_episodes.length})
+        # However, if the episode is drafting state,
+        # then we can try to update the episode attributes
+        draft_apple_episodes = eps.select(&:drafting?)
+        Apple::Episode.update_episodes(api, draft_apple_episodes)
+        Rails.logger.info("Updated remote episodes", {count: draft_apple_episodes.length})
 
         show.reload
       end
@@ -275,7 +281,7 @@ def poll_podcast_containers!(eps)
     def sync_podcast_containers!(eps)
       Rails.logger.tagged("##{__method__}") do
         # TODO: right now we only create one delivery per container,
-        # Apple RSS scaping means we don't need deliveries for freemium episode images
+        # Apple RSS scraping means we don't need deliveries for freemium episode images
         # But we do need asset deliveries for apple-only (non-rss) images
 
         Rails.logger.info("Starting podcast container sync")

app/models/application_record.rb

@@ -11,4 +11,14 @@ def self.alias_error_messages(to_field, from_field)
   def error_message_aliases
     @@error_message_aliases
   end
+
+  def locking_enabled?
+    !!(super && @locking_enabled)
+  end
+
+  attr_writer :locking_enabled
+
+  def stale?
+    !!try(:lock_version_changed?)
+  end
 end

app/views/episode_media/_form.html.erb

@@ -35,4 +35,5 @@
 
   <% end %>
 
+  <%= render "layouts/stale_record_modal", form: form, discard_path: episode_media_path(episode) %>
 <% end %>

app/views/episode_media/_form_status.html.erb

@@ -2,6 +2,7 @@
   <div class="card-header-primary">
     <h2 class="card-title h5"><%= t(".title") %></h2>
   </div>
+  <%= render "layouts/stale_record_field", form: form %>
   <div class="card-footer d-flex align-items-center">
 
     <p class="status-text flex-grow-1"><strong><%= t(".updated_at_hint") %></strong> <br><%= l(episode.updated_at) %></p>

app/views/episodes/_form.html.erb

@@ -22,4 +22,6 @@
   </div>
 
   <%= render "confirm_destroy", episode: episode %>
+
+  <%= render "layouts/stale_record_modal", form: form, discard_path: edit_episode_path(episode) if episode.persisted? %>
 <% end %>

app/views/episodes/_form_status.html.erb

@@ -45,6 +45,8 @@
     <% end %>
   </div>
 
+  <%= render "layouts/stale_record_field", form: form %>
+
   <div class="card-footer d-flex align-items-center justify-content-between">
     <% if episode.persisted? %>
       <p class="status-text"><strong><%= t(".updated_at_hint") %></strong> <br><%= local_time_ago(episode.updated_at) %></p>

app/views/feeds/_form.html.erb

@@ -30,4 +30,6 @@
   </div>
 
   <%= render "confirm_destroy", podcast: podcast, feed: feed %>
+
+  <%= render "layouts/stale_record_modal", form: form, discard_path: podcast_feed_path(podcast, feed) if feed.persisted? %>
 <% end %>

app/views/feeds/_form_status.html.erb

@@ -2,6 +2,9 @@
   <div class="card-header-primary">
     <h2 class="card-title h5"><%= t(".title") %></h2>
   </div>
+
+  <%= render "layouts/stale_record_field", form: form %>
+
   <div class="card-footer d-flex align-items-center">
     <% if feed.persisted? %>
       <p class="status-text flex-grow-1"><strong><%= t(".updated_at_hint") %></strong> <br><%= local_time_ago(feed.updated_at) %></p>

app/views/layouts/_stale_record_field.html.erb

@@ -0,0 +1,16 @@
+<% if form.object.stale? %>
+  <div class="card-footer" data-controller="toggle-value" data-toggle-value-next-value="<%= form.object.lock_version_was %>">
+    <div class="form-check">
+      <%= form.hidden_field :lock_version, data: {toggle_value_target: "field"} %>
+      <input id="stale_record_overwrite" class="form-check-input is-invalid" type="checkbox" data-action="toggle-value#toggle">
+      <div class="d-flex align-items-center">
+        <label for="stale_record_overwrite" class="form-check-label text-danger">
+          <b><%= t(".label") %></b>
+          <span class="text-danger ms-1"><%= t(".aside") %></span>
+        </label>
+      </div>
+    </div>
+  </div>
+<% else %>
+  <%= form.hidden_field :lock_version %>
+<% end %>

app/views/layouts/_stale_record_modal.html.erb

@@ -0,0 +1,21 @@
+<% if form.object.stale? %>
+  <div class="modal hide" id="stale-record" tabindex="-1" aria-labelledby="stale-record-title" aria-hidden="true">
+    <div class="modal-dialog modal-dialog-centered">
+      <div class="modal-content">
+        <div class="modal-header">
+          <h5 class="modal-title fw-bold" id="stale-record-title"><%= t(".title") %></h5>
+        </div>
+        <div class="modal-body">
+          <p><%= t ".body_html", model: form.object.model_name.human, ago: local_time_ago(form.object.updated_at) %></p>
+        </div>
+        <div class="modal-footer">
+          <%= link_to t(".discard"), discard_path, class: "btn btn-discard-changed", data: {action: "unsaved#discard", unsaved_target: "discard"} %>
+          <button type="button" class="btn btn-warning" data-bs-dismiss="modal"><%= t(".continue") %></button>
+        </div>
+      </div>
+    </div>
+  </div>
+
+  <%# click hidden button to immediately show modal %>
+  <button type="button" class="d-none" data-bs-toggle="modal" data-bs-target="#stale-record" data-controller="click" data-click-immediate-value="true"></button>
+<% end %>

app/views/podcast_engagement/_form.html.erb

@@ -20,4 +20,5 @@
     </div>
   </div>
 
+  <%= render "layouts/stale_record_modal", form: form, discard_path: podcast_engagement_path(podcast) %>
 <% end %>

app/views/podcast_engagement/_form_status.html.erb

@@ -2,6 +2,9 @@
   <div class="card-header-primary">
     <h2 class="card-title h5"><%= t(".title") %></h2>
   </div>
+
+  <%= render "layouts/stale_record_field", form: form %>
+
   <div class="card-footer d-flex align-items-center">
 
     <p class="status-text flex-grow-1"><strong><%= t(".updated_at_hint") %></strong> <br><%= local_time_ago(podcast.updated_at) %></p>

app/views/podcasts/_form.html.erb

@@ -19,4 +19,6 @@
   </div>
 
   <%= render "confirm_destroy", podcast: podcast %>
+
+  <%= render "layouts/stale_record_modal", form: form, discard_path: edit_podcast_path(podcast) if podcast.persisted? %>
 <% end %>

app/views/podcasts/_form_status.html.erb

@@ -2,6 +2,9 @@
   <div class="card-header-primary">
     <h2 class="card-title h5"><%= t(".title") %></h2>
   </div>
+
+  <%= render "layouts/stale_record_field", form: form %>
+
   <div class="card-footer d-flex align-items-center">
     <% if podcast.persisted? %>
       <p class="status-text flex-grow-1"><strong><%= t(".updated_at_hint") %></strong> <br><%= local_time_ago(podcast.updated_at) %></p>

config/locales/en.yml

@@ -918,6 +918,14 @@ en:
       privacy: Privacy
       status: Status
       terms: Terms
+    stale_record_field:
+      aside: (Not Recommended)
+      label: Overwrite
+    stale_record_modal:
+      body_html: Another user updated this %{model} <b>%{ago}</b>.</br></br>You can either discard your changes and make them again in a fresh form, or continue editing at your own risk.
+      continue: Continue Editing
+      discard: Discard changes
+      title: Warning! Another user has updated this form.
   podcasts:
     confirm_destroy:
       <<: *form_status

db/migrate/20240424214558_add_locking_columns.rb

@@ -0,0 +1,7 @@
+class AddLockingColumns < ActiveRecord::Migration[7.0]
+  def change
+    add_column :episodes, :lock_version, :integer, null: false, default: 0
+    add_column :feeds, :lock_version, :integer, null: false, default: 0
+    add_column :podcasts, :lock_version, :integer, null: false, default: 0
+  end
+end

test/controllers/episodes_controller_test.rb

@@ -71,6 +71,11 @@ class EpisodesControllerTest < ActionDispatch::IntegrationTest
     assert_response :unprocessable_entity
   end
 
+  test "optimistically locks updating episodes" do
+    patch episode_url(episode), params: {episode: {lock_version: episode.lock_version - 1}}
+    assert_response :conflict
+  end
+
   test "should destroy episode" do
     assert episode
 

test/controllers/feeds_controller_test.rb

@@ -71,6 +71,11 @@ class FeedsControllerTest < ActionDispatch::IntegrationTest
     assert_response :unprocessable_entity
   end
 
+  test "optimistically locks updating feeds" do
+    patch podcast_feed_url(podcast, feed), params: {feed: {lock_version: feed.lock_version - 1}}
+    assert_response :conflict
+  end
+
   test "should destroy feed" do
     assert podcast
     assert feed

test/controllers/podcasts_controller_test.rb

@@ -77,6 +77,11 @@ class PodcastsControllerTest < ActionDispatch::IntegrationTest
     assert_response :unprocessable_entity
   end
 
+  test "optimistically locks updating podcasts" do
+    patch podcast_url(podcast), params: {podcast: {lock_version: podcast.lock_version - 1}}
+    assert_response :conflict
+  end
+
   test "should destroy podcast" do
     assert podcast