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

Use persisted value for permalink field if there are unpersisted changes #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Joerg-Seitz
Copy link

Problem

When saving a record fails due to a validation on a field used for a permalink and you send the user back to a form that is built with a pathhelper using said permalink functionality, the permalink is built with the in memory value instead of the persisted value and throws a routing error

Solution:

Always refer to the persisted value for building the permalink

@Joerg-Seitz Joerg-Seitz requested a review from averell23 January 15, 2025 16:51
Copy link
Member

@averell23 averell23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a quite obscure bug - just found a little AR magic to make it even easier...

Comment on lines +21 to +30
field_name = permalink_options[:with]

if self.send("#{field_name}_changed?")
self.send("#{field_name}_was")
else
self.send(field_name)
end
end
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def persisted_permalink_field
field_name = permalink_options[:with]
if self.send("#{field_name}_changed?")
self.send("#{field_name}_was")
else
self.send(field_name)
end
end
end

elsif(!permalink_options.empty?)
"#{original_to_param}-#{self.send(permalink_options[:with])}"
"#{original_to_param}-#{persisted_permalink_field}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently there is attribute_in_database which already does this

Suggested change
"#{original_to_param}-#{persisted_permalink_field}"
"#{original_to_param}-#{attribute_in_database(permalink_options[:with])}"

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

Successfully merging this pull request may close these issues.

2 participants