From afe343ecbb9d7043281d66cb28fa6b39b80d3d57 Mon Sep 17 00:00:00 2001 From: Kane <918780+kanejamison@users.noreply.github.com> Date: Fri, 28 Aug 2026 08:53:15 -0700 Subject: [PATCH] Remove raising status validator; rely on inclusion validation validate_status_value raised ArgumentError inside a validation callback, so a tampered form/API status value produced a 500 instead of a validation error. The inclusion validation on :status already covers the same check, so the custom validator is deleted and tests now assert validation errors instead of raises. Closes #56 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01MSumqtaUuj2UhU4Wtm1C1H --- lib/bunko/models/post_methods/publishable.rb | 9 ----- test/models/post_publishing_test.rb | 36 ++++++++++---------- 2 files changed, 18 insertions(+), 27 deletions(-) diff --git a/lib/bunko/models/post_methods/publishable.rb b/lib/bunko/models/post_methods/publishable.rb index c58dbd7..9723b87 100644 --- a/lib/bunko/models/post_methods/publishable.rb +++ b/lib/bunko/models/post_methods/publishable.rb @@ -15,7 +15,6 @@ module Publishable # Callbacks before_validation :set_published_at, if: :should_set_published_at? - validate :validate_status_value # Scopes scope :published, -> { where(status: "published").where("published_at <= ?", Time.current).order(published_at: :desc) } @@ -37,14 +36,6 @@ def should_set_published_at? def set_published_at self.published_at = Time.current end - - def validate_status_value - return if status.blank? - - unless Bunko.configuration.valid_statuses.include?(status) - raise ArgumentError, "#{status} is not a valid status" - end - end end end end diff --git a/test/models/post_publishing_test.rb b/test/models/post_publishing_test.rb index 583f18c..9905b7c 100644 --- a/test/models/post_publishing_test.rb +++ b/test/models/post_publishing_test.rb @@ -68,9 +68,12 @@ class PostPublishingTest < ActiveSupport::TestCase post_type: @blog_type ) - assert_raises(ArgumentError) do + assert_raises(ActiveRecord::RecordInvalid) do post.update!(status: "invalid_status") end + + refute post.update(status: "invalid_status") + assert_includes post.errors[:status], "invalid_status is not a valid status" end test "posts with published status but future published_at are treated as scheduled" do @@ -109,28 +112,25 @@ class PostPublishingTest < ActiveSupport::TestCase refute post.send(:should_set_published_at?) end - test "validate_status_value raises error for invalid status" do - post = Post.new(status: "invalid", post_type: @blog_type) - - error = assert_raises(ArgumentError) do - post.send(:validate_status_value) - end + test "invalid status adds a validation error instead of raising" do + post = Post.new(title: "Test", content: "Content", status: "invalid", post_type: @blog_type) - assert_match(/invalid is not a valid status/, error.message) + refute post.valid? + assert_includes post.errors[:status], "invalid is not a valid status" end - test "validate_status_value does not raise error for valid status" do - post = Post.new(status: "published", post_type: @blog_type) - assert_nothing_raised do - post.send(:validate_status_value) - end + test "valid status produces no status errors" do + post = Post.new(title: "Test", content: "Content", status: "published", post_type: @blog_type) + + post.valid? + assert_empty post.errors[:status] end - test "validate_status_value returns early if status is blank" do - post = Post.new(post_type: @blog_type) - assert_nothing_raised do - post.send(:validate_status_value) - end + test "blank status adds a presence error instead of raising" do + post = Post.new(title: "Test", content: "Content", status: nil, post_type: @blog_type) + + refute post.valid? + assert_includes post.errors[:status], "can't be blank" end test "scheduled? returns true for published posts with future published_at" do