From a35009e339e98d250f97d059f62dd118e1ff56c3 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 30 Sep 2020 16:20:39 -0400 Subject: [PATCH] Remove redirect for listings#update (#10499) * Remove redirect for listings update * Quick refactor * Extract query to convenience method --- app/controllers/listings_controller.rb | 31 ++++++++++++++++++++------ spec/requests/listings_spec.rb | 9 ++++++++ 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/app/controllers/listings_controller.rb b/app/controllers/listings_controller.rb index 27b9ec0ed..c42699bc5 100644 --- a/app/controllers/listings_controller.rb +++ b/app/controllers/listings_controller.rb @@ -33,8 +33,7 @@ class ListingsController < ApplicationController # rubocop:enable Rails/LexicallyScopedActionFilter def index - published_listings = Listing.where(published: true) - @displayed_listing = published_listings.find_by(slug: params[:slug]) if params[:slug] + @displayed_listing = Listing.where(published: true).find_by(slug: params[:slug]) if params[:slug] if params[:view] == "moderate" not_found unless @displayed_listing @@ -43,10 +42,7 @@ class ListingsController < ApplicationController @listings = if params[:category].blank? - published_listings - .order(bumped_at: :desc) - .includes(:user, :organization, :taggings) - .limit(12) + listings_for_index_view else Listing.none end @@ -124,7 +120,19 @@ class ListingsController < ApplicationController end def process_after_update - redirect_to "/listings" + # The following sets variables used in the index view. We render the + # index view directly to avoid having to redirect. + # + # Redirects lead to a race condition where we redirect to a cached view + # after updating data and we don't bust the cache fast enough before + # hitting the view, therefore stale content ends up being served from + # cache. + # + # https://github.com/forem/forem/issues/10338#issuecomment-693401481 + @listings = listings_for_index_view + @listings_json = @listings.to_json(INDEX_JSON_OPTIONS) + + render :index end def process_after_unpublish @@ -134,4 +142,13 @@ class ListingsController < ApplicationController def check_limit rate_limit!(:listing_creation) end + + # This is a convenience method to query listings for use in the index view in + # the index action and process_after_update method + def listings_for_index_view + Listing.where(published: true) + .order(bumped_at: :desc) + .includes(:user, :organization, :taggings) + .limit(12) + end end diff --git a/spec/requests/listings_spec.rb b/spec/requests/listings_spec.rb index 272aaa252..e3dfffb39 100644 --- a/spec/requests/listings_spec.rb +++ b/spec/requests/listings_spec.rb @@ -521,6 +521,15 @@ RSpec.describe "/listings", type: :request do expect(listing.title).not_to eq("New title!") expect(listing.cached_tag_list).not_to include("hey") end + + it "doesn't redirect on a successful update" do + put "/listings/#{listing.id}", params: { + listing: { body_markdown: "hello new markdown", title: "New title!", tag_list: "new, tags, hey" } + } + + expect(response).not_to have_http_status(:redirect) + expect(response).to have_http_status(:ok) + end end end