Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UUID set as slug on failed validations on base column #1012

Open
brian-davis opened this issue Jun 27, 2023 · 1 comment
Open

UUID set as slug on failed validations on base column #1012

brian-davis opened this issue Jun 27, 2023 · 1 comment

Comments

@brian-davis
Copy link

I'm seeing this issue, when submitting a form for a model object which has validations defined on the base field. I've described my workaround below. I will also be submitting a PR.

Setup:

$ rails new friendly_id_debug
$ cd friendly_id_debug
$ rails g scaffold Post title:string body:text
$ rails db:migrate

In app/models/post.rb:

class Post < ApplicationRecord
  validates :title, presence: true
  validates :body, presence: true
end

In db/seeds.rb

10.times do |i|
  Post.create({
    title: "Post #{i + 1}",
    body: "words" * 100
  })
end

Finish:

$ rails db:migrate db:seed
$ rails s

test form validations as normal.

add friendly_id:

$ bundle add friendly_id
$ rails g migration AddSlugToPosts slug:uniq
$ rails generate friendly_id
$ rails db:migrate

in app/models/post.rb:

class Post < ApplicationRecord
  extend FriendlyId
  friendly_id :title, use: :slugged

  validates :title, presence: true
  validates :body, presence: true
end

And app/controllers/posts_controller.rb:

def set_post
  @post = Post.friendly.find(params[:id])
end

Update existing records:

>> Post.find_each(&:save)

Add update slug on title change:

In app/models/post.rb:

class Post < ApplicationRecord
  extend FriendlyId
  friendly_id :title, use: :slugged

  validates :title, presence: true
  validates :body, presence: true

  def should_generate_new_friendly_id?
    title_changed?
  end
end

Go back to edit, and try submitting with a blank field.

1 error prohibited this post from being saved:
Title can't be blank

But now open a chrome inpsector and look at the form:

<form action="/posts/88c1529f-c6d9-4a4a-8b27-45f259091baa" ...>
  ...
</form>

Submitting error:

ActiveRecord::RecordNotFound (can't find record with friendly id: "88c1529f-c6d9-4a4a-8b27-45f259091baa")

unit tests:

In test/models/post_test.rb add:

require "test_helper"

class PostTest < ActiveSupport::TestCase
  # normal use
  test "slug updates when title updates" do
    # set up
    post = Post.create({
      title: "My First Post",
      body: "My Deep Thoughts"
    })
    assert_equal("my-first-post", post.slug)

    # execute
    post.update(title: "Changed My Mind")

    # desired behavior
    assert_equal("changed-my-mind", post.slug)
  end
end

This should be green already. Add a red test:

  # edge case
  # validatition bug:
  # Expected ["my-second-post", "2d883d62-9c3e-4158-9daf-b34e655dcc3e"] to be nil.
  # This bug will break form behavior after an invalid submission
  test "slug unchanged when title update is invalid, in :valid? call" do
    # set up
    post = Post.create({
      title: "My Second Post",
      body: "More Deep Thoughts"
    })
    original_slug = post.slug.dup

    # execute
    post.assign_attributes({ title: "" }) # set .changes
    refute(post.valid?) # set .errors; falsy expected; trigger bug

    # desired behavior, slug not touched on failed validation for :title
    assert_nil(post.changes["slug"])
    assert_equal(original_slug, post.slug)
  end

debugging:

> friendly_id_config.query_field
=> "slug"
> friendly_id_config.base
=> :title
> errors.map(&:attribute)
=> [:title]

The fix. in app/models/post.rb, add:

  private def unset_slug_if_invalid
    if (
      errors.key?(friendly_id_config.base) ||
      errors.key?(friendly_id_config.slug_column.to_s)
    ) && attribute_changed?(friendly_id_config.slug_column.to_s)

      original_slug = changes[friendly_id_config.slug_column.to_s]&.first
      send "#{friendly_id_config.slug_column}=", original_slug
    end
  end

The base field (i.e. :title)should also be checked for errors.

with refactoring:

class Post < ApplicationRecord
  extend FriendlyId
  friendly_id :title

  validates :title, presence: true
  validates :body, presence: true
end

config in initializer:

  config.use :slugged
  config.use Module.new {
    def should_generate_new_friendly_id?
      slug.blank? || attribute_changed?(friendly_id_config.base.to_s)
    end

    private def unset_slug_if_invalid
      if (
        errors.key?(friendly_id_config.base) ||
        errors.key?(friendly_id_config.slug_column)
      ) && attribute_changed?(friendly_id_config.slug_column)

        original_slug = changes[friendly_id_config.slug_column]&.first
        send "#{friendly_id_config.slug_column}=", original_slug
      end
    end
  }
@brian-davis
Copy link
Author

demo app with workaround:
https://github.com/brian-davis/special-funicular

PR with test and fix:
#1013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant