Merge pull request #2331 from pascalbetz/fix-2330

Update published_at only after page has been published.

app/models/alchemy/page.rb

@@ -457,14 +457,9 @@ def copy_children_to(new_parent)
       end
     end
 
-    # Creates a public version of the page.
-    #
-    # Sets the +published_at+ value to current time
-    #
-    # The +published_at+ attribute is used as +cache_key+.
+    # Creates a public version of the page in the background.
     #
     def publish!(current_time = Time.current)
-      update(published_at: current_time)
       PublishPageJob.perform_later(id, public_on: current_time)
     end
 

app/models/alchemy/page/publisher.rb

@@ -12,7 +12,9 @@ def initialize(page)
 
       # Copies all currently visible elements to the public version of page
       #
-      # Creates a new published version if none exists yet.
+      # Creates a new published version if none exists yet and updates
+      # the `published_at` timestamp of the page.
+      # `published_at` is used as a cache key.
       #
       # Sends a publish notification to all registered publish targets
       #
@@ -32,6 +34,7 @@ def publish!(public_on:)
               end
             end
           end
+          page.update(published_at: public_on)
         end
 
         Alchemy.publish_targets.each { |p| p.perform_later(page) }

spec/controllers/alchemy/admin/pages_controller_spec.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require "rails_helper"
+require "timecop"
 
 RSpec.describe Alchemy::Admin::PagesController do
   routes { Alchemy::Engine.routes }
@@ -50,8 +51,12 @@
     let(:page) { create(:alchemy_page) }
 
     it "publishes the page" do
-      expect_any_instance_of(Alchemy::Page).to receive(:publish!)
-      post :publish, params: { id: page }
+      current_time = Time.current
+      Timecop.freeze(current_time) do
+        expect {
+          post :publish, params: { id: page }
+        }.to have_enqueued_job(Alchemy::PublishPageJob).with(page.id, public_on: current_time)
+      end
     end
   end
 end

spec/models/alchemy/page/publisher_spec.rb

@@ -37,6 +37,14 @@
       expect { publish }.to change { page.versions.published.count }.by(1)
     end
 
+    it "updates the public_on timestamp" do
+      expect {
+        publish
+      }.to change {
+        page.reload.public_on
+      }.to(current_time)
+    end
+
     context "with elements" do
       include_context "with elements"
 

spec/models/alchemy/page_spec.rb

@@ -1561,30 +1561,10 @@ def copy_children_to(new_parent)
     end
 
     describe "#publish!" do
-      let(:current_time) { Time.current.change(usec: 0) }
-      let(:page) do
-        create(:alchemy_page,
-               public_on: public_on,
-               public_until: public_until,
-               published_at: published_at)
-      end
-      let(:published_at) { nil }
-      let(:public_on) { nil }
-      let(:public_until) { nil }
-
-      before do
-        allow(Time).to receive(:current).and_return(current_time)
-      end
-
-      it "enqueues publish page job" do
-        expect {
-          page.publish!
-        }.to have_enqueued_job(Alchemy::PublishPageJob)
-      end
+      let(:page) { create(:alchemy_page) }
 
-      it "sets published_at" do
-        page.publish!
-        expect(page.published_at).to eq(current_time)
+      it "enqueues a Alchemy::PublishPageJob" do
+        expect { page.publish! }.to have_enqueued_job(Alchemy::PublishPageJob)
       end
     end
 

spec/requests/alchemy/admin/pages_controller_spec.rb

@@ -634,10 +634,10 @@ module Alchemy
       describe "#publish" do
         let(:page) { create(:alchemy_page, published_at: 3.days.ago) }
 
-        it "should publish the page" do
+        it "published page in the background" do
           expect {
             post publish_admin_page_path(page)
-          }.to change { page.reload.published_at }
+          }.to have_enqueued_job(Alchemy::PublishPageJob)
         end
       end