-
-
Notifications
You must be signed in to change notification settings - Fork 592
Add treat_numeric_as_conflict option to prevent ambiguous numeric slugs
#1037
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
Add treat_numeric_as_conflict option to prevent ambiguous numeric slugs
#1037
Conversation
|
@parndt I know there's not a ton of active development on friendly_id now, but any concerns with something like this? Thanks in advance. |
|
Thanks @oehlschl I think this seems fine to support. Is it possible to add to the changelog and add some docs around it for those using slugged so that people know it's an option (without reading the source code)? |
|
Yes, will follow up with the additional changes tonight; thanks @parndt! |
|
@parndt I added Guide documentation, and this change to the changelog (under Unreleased). Also updated the Int casting (will comment inline); please let me know if you need anything else. Thanks! |
treat_numeric_as_conflict option to prevent ambiguous numeric slugs
|
@parndt please let me know if you need anything else here, thanks! |
|
Pardon the ping @parndt, but would you please cut a new release when you can? Thanks! |
|
@oehlschl I will work on it; I've been meaning to set up trusted publishing for a while.. |
|
@oehlschl after much ado with rubygems publishing, we have version 5.6.0! |
|
@oehlschl Thanks for this - we had some custom code to deal with this, which I can now drop! |
|
Glad to hear it! Thanks all. Upgrading the gem and removing some of our own custom code now too. |
Summary
We sometimes have issues where clients call our APIs and pass integer primary keys instead of slugs, but
Model.friendly.find(id)finds a record with the same numeric slug, because our slug candidates are dynamic and in some cases purely numeric.We'd like to prevent that, and so this PR adds a new configuration option
treat_numeric_as_conflictprevent purely numeric slugs and therefore resolve the ambiguity whereModel.friendly.find("123")could return either a record withslug="123"OR a record withid=123.When enabled, purely numeric slug candidates are treated as conflicts and resolved using UUID suffixing, ensuring all slugs contain non-numeric characters that eliminate lookup ambiguity.
Changes
treat_numeric_as_conflictconfiguration option toSlugged::ConfigurationSlugGeneratorto detect and reject purely numeric slugs when enabledpurely_numeric_slug?helper method usingInteger()validation (consistent withpotential_primary_key?)Usage
Testing Done
Deploy Notes