Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Scheduled publishing #134

Merged
merged 4 commits into from

2 participants

@vinayvinay

few more changes to get scheduled publishing working on publisher.

  1. adding #locked_for_edits? to be used in place of #published? when checking if the form needs to be disabled for edits. this takes care of the edition being scheduled_for_publishing?
  2. changing the publish_at timestamp validation sequence to incorporate it within the state machine so that it gets validated only when required, not on every save.
vinayvinay added some commits
@vinayvinay vinayvinay Cancel for scheduling should make publish_at nil
... which needed:

1. the validation for publish_at to be a future
time to be moved to the state_machine
2. publish_at to be editable when in state
scheduled_for_published.
d2e4af3
@vinayvinay vinayvinay Combining 2 tests, adding a new one for edge case 9b60093
@vinayvinay vinayvinay closed this
@vinayvinay vinayvinay reopened this
@alext alext merged commit c5f5c43 into master
@alext alext deleted the scheduled-publishing branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 6, 2014
  1. @vinayvinay

    Cancel for scheduling should make publish_at nil

    vinayvinay authored
    ... which needed:
    
    1. the validation for publish_at to be a future
    time to be moved to the state_machine
    2. publish_at to be editable when in state
    scheduled_for_published.
  2. @vinayvinay
Commits on Mar 7, 2014
  1. @vinayvinay
  2. @vinayvinay
This page is out of date. Refresh to see the latest.
View
9 app/models/edition.rb
@@ -47,7 +47,6 @@ class Edition
validates :title, presence: true
validates :version_number, presence: true
validates :panopticon_id, presence: true
- validate :publish_at_is_in_the_future
validates_with SafeHtml
before_save :check_for_archived_artefact
@@ -99,7 +98,7 @@ def in_progress_sibling
end
def can_create_new_edition?
- subsequent_siblings.in_progress.empty?
+ !scheduled_for_publishing? && subsequent_siblings.in_progress.empty?
end
def meta_data
@@ -284,10 +283,4 @@ def destroy_artefact
Artefact.find(self.panopticon_id).destroy
end
end
-
- private
-
- def publish_at_is_in_the_future
- errors.add(:publish_at, "can't be a time in the past") if publish_at.present? && publish_at < Time.zone.now
- end
end
View
40 app/models/workflow.rb
@@ -90,6 +90,7 @@ class CannotDeletePublishedPublication < RuntimeError; end
state :scheduled_for_publishing do
validates_presence_of :publish_at
+ validate :publish_at_is_in_the_future
end
end
end
@@ -211,24 +212,31 @@ def in_progress?
! ["archived", "published"].include? self.state
end
+ def locked_for_edits?
+ scheduled_for_publishing? || published?
+ end
+
+ def error_description
+ published? ? 'Published editions' : 'Editions scheduled for publishing'
+ end
+
private
+ def publish_at_is_in_the_future
+ errors.add(:publish_at, "can't be a time in the past") if publish_at.present? && publish_at < Time.zone.now
+ end
+
def not_editing_published_item
- if changed? and ! state_changed?
- if archived?
- errors.add(:base, "Archived editions can't be edited")
- end
- if scheduled_for_publishing? || published?
- changes_allowed_when_published = ["slug", "section",
- "department", "business_proposition"]
- illegal_changes = changes.keys - changes_allowed_when_published
- if illegal_changes.empty?
- # Allow it
- else
- edition_description = published? ? 'Published editions' : 'Editions scheduled for publishing'
- errors.add(:base, "#{edition_description} can't be edited")
- end
- end
- end
+ return if changes.none? || state_changed?
+
+ errors.add(:base, "Archived editions can't be edited") if archived?
+
+ return unless locked_for_edits?
+ errors.add(:base, "#{error_description} can't be edited") if disallowable_change?
+ end
+
+ def disallowable_change?
+ allowed_to_change = %w(slug section publish_at department business_proposition)
+ (changes.keys - allowed_to_change).present?
end
end
View
4 lib/govuk_content_models/test_helpers/factories.rb
@@ -86,6 +86,10 @@
state 'scheduled_for_publishing'
publish_at 1.day.from_now
end
+
+ trait :published do
+ state 'published'
+ end
end
factory :answer_edition, parent: :edition do
end
View
40 test/models/edition_scheduled_for_publishing_test.rb
@@ -6,7 +6,6 @@ class EditionScheduledForPublishingTest < ActiveSupport::TestCase
setup do
@edition = FactoryGirl.create(:edition, state: 'ready')
@edition.schedule_for_publishing
- @edition.reload
end
should "return an error" do
@@ -34,6 +33,14 @@ class EditionScheduledForPublishingTest < ActiveSupport::TestCase
assert_equal 'scheduled_for_publishing', @edition.state
end
end
+
+ should "not allow scheduling at a time in the past" do
+ edition = FactoryGirl.create(:edition, state: 'ready')
+
+ edition.schedule_for_publishing(1.hour.ago)
+
+ assert_includes edition.errors[:publish_at], "can't be a time in the past"
+ end
end
context "when scheduled_for_publishing" do
@@ -56,29 +63,38 @@ class EditionScheduledForPublishingTest < ActiveSupport::TestCase
end
should "return false for #can_destroy?" do
- edition = FactoryGirl.create(:edition, :scheduled_for_publishing)
+ edition = FactoryGirl.build(:edition, :scheduled_for_publishing)
refute edition.can_destroy?
end
+ should "return false for #can_create_new_edition?" do
+ edition = FactoryGirl.build(:edition, :scheduled_for_publishing)
+ refute edition.can_create_new_edition?
+ end
+
should "allow transition to published state" do
- edition = FactoryGirl.create(:edition, :scheduled_for_publishing)
+ edition = FactoryGirl.build(:edition, :scheduled_for_publishing)
assert edition.can_publish?
end
end
context "#cancel_scheduled_publishing" do
- setup do
- @edition = FactoryGirl.create(:edition, :scheduled_for_publishing)
- @edition.cancel_scheduled_publishing
- @edition.reload
- end
+ should "remove the publish_at stored against the edition and transition back to ready" do
+ edition = FactoryGirl.create(:edition, :scheduled_for_publishing)
+ edition.cancel_scheduled_publishing
+ edition.reload
- should "remove the publish_at stored against the edition" do
- assert_nil @edition.publish_at
+ assert_nil edition.publish_at
+ assert_equal 'ready', edition.state
end
- should "complete the transition back to ready" do
- assert_equal 'ready', @edition.state
+ should "work with editions that have passed publish_at time" do
+ edition = FactoryGirl.create(:edition, :scheduled_for_publishing)
+ edition.update_attribute :publish_at, 2.days.ago
+
+ edition.cancel_scheduled_publishing
+
+ assert_equal 'ready', edition.reload.state
end
end
end
View
9 test/models/edition_test.rb
@@ -46,15 +46,6 @@ def template_unpublished_answer(version_number = 1)
assert a.errors[:title].any?
end
- context "#publish_at" do
- should "not be a time in the past" do
- edition = FactoryGirl.build(:edition, publish_at: 1.minute.ago)
-
- refute edition.valid?
- assert_includes edition.errors[:publish_at], "can't be a time in the past"
- end
- end
-
test "it should give a friendly (legacy supporting) description of its format" do
a = LocalTransactionEdition.new
assert_equal "LocalTransaction", a.format
View
11 test/models/workflow_test.rb
@@ -69,6 +69,17 @@ def template_user_and_published_transaction
end
end
+ context "#locked_for_edit?" do
+ should "return true if edition is scheduled for publishing for published" do
+ assert FactoryGirl.build(:edition, :scheduled_for_publishing).locked_for_edits?
+ assert FactoryGirl.build(:edition, :published).locked_for_edits?
+ end
+
+ should "return false if in draft state" do
+ refute FactoryGirl.build(:edition, state: 'draft').locked_for_edits?
+ end
+ end
+
test "permits the creation of new editions" do
user, transaction = template_user_and_published_transaction
assert transaction.persisted?
Something went wrong with that request. Please try again.