Merge pull request #1011 from PRX/revert-1006-feat/optimistic_locking

Revert "Optimistic locking"
PRX · Apr 29, 2024 · d3a83e2 · d3a83e2
2 parents c746de4 + d8fb7e1
commit d3a83e2
Show file tree

Hide file tree

Showing 25 changed files with 1 addition and 133 deletions.
diff --git a/app/controllers/episode_media_controller.rb b/app/controllers/episode_media_controller.rb
@@ -49,22 +49,18 @@ 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],

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

diff --git a/app/controllers/feeds_controller.rb b/app/controllers/feeds_controller.rb
@@ -59,8 +59,6 @@ def update
         end
       end
     end
-  rescue ActiveRecord::StaleObjectError
-    render :show, status: :conflict
   end
 
   # DELETE /feeds/1
@@ -96,7 +94,6 @@ 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.
@@ -106,7 +103,6 @@ def feed_params
 
   def nilified_feed_params
     nilify params.fetch(:feed, {}).permit(
-      :lock_version,
       :file_name,
       :title,
       :subtitle,

diff --git a/app/controllers/podcast_engagement_controller.rb b/app/controllers/podcast_engagement_controller.rb
@@ -17,16 +17,13 @@ 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
 
@@ -35,7 +32,6 @@ 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
     )

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

diff --git a/app/javascript/controllers/toggle_value_controller.js b/app/javascript/controllers/toggle_value_controller.js
diff --git a/app/models/application_record.rb b/app/models/application_record.rb
@@ -11,14 +11,4 @@ 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?) || false
-  end
 end
diff --git a/app/views/episode_media/_form.html.erb b/app/views/episode_media/_form.html.erb
@@ -35,5 +35,4 @@
 
   <% end %>
 
-  <%= render "layouts/stale_record_modal", form: form, discard_path: episode_media_path(episode) %>
 <% end %>
diff --git a/app/views/episode_media/_form_status.html.erb b/app/views/episode_media/_form_status.html.erb
@@ -2,7 +2,6 @@
   <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>

diff --git a/app/views/episodes/_form.html.erb b/app/views/episodes/_form.html.erb
@@ -22,6 +22,4 @@
   </div>
 
   <%= render "confirm_destroy", episode: episode %>
-
-  <%= render "layouts/stale_record_modal", form: form, discard_path: edit_episode_path(episode) if episode.persisted? %>
 <% end %>
diff --git a/app/views/episodes/_form_status.html.erb b/app/views/episodes/_form_status.html.erb
@@ -45,8 +45,6 @@
     <% 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>

diff --git a/app/views/feeds/_form.html.erb b/app/views/feeds/_form.html.erb
@@ -30,6 +30,4 @@
   </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 %>
diff --git a/app/views/feeds/_form_status.html.erb b/app/views/feeds/_form_status.html.erb
@@ -2,9 +2,6 @@
   <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>

diff --git a/app/views/layouts/_stale_record_field.html.erb b/app/views/layouts/_stale_record_field.html.erb
diff --git a/app/views/layouts/_stale_record_modal.html.erb b/app/views/layouts/_stale_record_modal.html.erb
diff --git a/app/views/podcast_engagement/_form.html.erb b/app/views/podcast_engagement/_form.html.erb
@@ -20,5 +20,4 @@
     </div>
   </div>
 
-  <%= render "layouts/stale_record_modal", form: form, discard_path: podcast_engagement_path(podcast) %>
 <% end %>
diff --git a/app/views/podcast_engagement/_form_status.html.erb b/app/views/podcast_engagement/_form_status.html.erb
@@ -2,9 +2,6 @@
   <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>

diff --git a/app/views/podcasts/_form.html.erb b/app/views/podcasts/_form.html.erb
@@ -19,6 +19,4 @@
   </div>
 
   <%= render "confirm_destroy", podcast: podcast %>
-
-  <%= render "layouts/stale_record_modal", form: form, discard_path: edit_podcast_path(podcast) if podcast.persisted? %>
 <% end %>
diff --git a/app/views/podcasts/_form_status.html.erb b/app/views/podcasts/_form_status.html.erb
@@ -2,9 +2,6 @@
   <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>

diff --git a/config/locales/en.yml b/config/locales/en.yml
@@ -918,14 +918,6 @@ 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

diff --git a/db/migrate/20240424214558_add_locking_columns.rb b/db/migrate/20240424214558_add_locking_columns.rb
diff --git a/db/schema.rb b/db/schema.rb
diff --git a/test/controllers/episodes_controller_test.rb b/test/controllers/episodes_controller_test.rb
@@ -71,11 +71,6 @@ 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
 

diff --git a/test/controllers/feeds_controller_test.rb b/test/controllers/feeds_controller_test.rb
@@ -71,11 +71,6 @@ 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

diff --git a/test/controllers/podcasts_controller_test.rb b/test/controllers/podcasts_controller_test.rb
@@ -77,11 +77,6 @@ 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