Addon for numerically sequential slugs #644
Conversation
|
I'm on vacation without my computer so I can't give good feedback on this for at least a week, but I'm in favor of this feature and grateful that you worked on it. When we had this feature by default, I remember there bein so fairly complex code to pull the sequence off the end of the slug in order to increment it, and to extract the base for SQL queries. You should make sure to look through the git and issue history for this feature in version 4.x so we don't end up repeating the cycle of fixing bugs that were solved years before. Your suggestions for documentation sound perfect. Thanks again for taking this on! |
|
@tekin what a legend |
| # Return the unnumbered (shortest) slug first, followed by the numbered ones | ||
| # in ascending order. | ||
| def ordering_query | ||
| "LENGTH(#{slug_column}) ASC, #{slug_column} ASC" |
There was a problem hiding this comment.
does this work in sqlite3, postgresl, mysql etc?
There was a problem hiding this comment.
I've only tested it with MySQL myself, but according to the Travis build it does: https://travis-ci.org/norman/friendly_id/builds/50012697
|
Cheers guys. The code is mostly based on the previous slug generator code from v4, but there are a few bits I didn't port across because mainly because I couldn't get my head around the failing case they were trying to solve. I should have some time later in the week to go back through it and see if there are any edge cases that I've failed to cover. |
|
Thanks for your work @tekin |
tekin
force-pushed
the
sequential-slugs
branch
2 times, most recently
from
f39685f to
28327c5
Compare
This addon provides the pre-5.0 behaviour that generated numerically-sequenced slugs to resolve conflict resolution.
tekin
force-pushed
the
sequential-slugs
branch
from
28327c5 to
a9c04aa
Compare
|
I've increased the test coverage and made a change to ensure the slug column is correctly escaped in the queries used to generate sequential slugs. There is a particular bit of code from the previous implementation that I've not ported: https://github.com/norman/friendly_id/pull/244/files - I think I understand the issue this is trying to solve, but I've really struggled to replicate a failing test. This makes me reluctant to simply copy across the code that is supposedly doing the required escaping. |
|
Hi @norman anything I can do to help move this forward? I'm not entirely sure what the failing build is about. It appears unrelated. |
|
@tekin sorry for the delay, I'll try to make time for this soon and get it (and the other PRs that are accumulating) merged in. |
|
@norman Any idea when this will be merged? The build doesn't seem to fail from this PR. Thanks. |
1 similar comment
Fixes a bug where a slug conflicting with a slug that starts with a number would get incorrectly sequenced based on the number at the start of the slug, rather than as a fresh sequence.
|
Sorry again for my long absence. I'm working on this today and will get it merged in along with the other open PRs. |
|
@tekin I picked through the history of this functionality in version 4.0 and added in support to escape the '%' and '_' characters in LIKE queries, the 'LEN' operator for MSSQL and specifying |
|
Thanks @norman, really appreciate your time and effort in creating and maintaining the gem 🍰 |
|
This is now out in 5.2.0.beta.1. No timetable for stable release yet, but probably within a couple weeks. |
This provides an addon that enables sequential slugs for conflict resolution, mimicking the behaviour of friendly_id v4.
Well formed candidates combined with UUIDs as a fallback will almost always be the better solution for resolving slug conflicts. That said, there are circumstances when this is either not possible or undesirable. For example, either good candidates cannot be formed, or the existing behaviour needs to be maintained for consistency.
I have recently had to deal with this situation whilst upgrading a large Rails 3 application to Rails 4. In this case it's not possible to form appropriate candidates for many of the sluggable models, and UUIDs would make new URLs inconsistent and "ugly". Ultimately it's both cheaper and far easier for the business (in this case the UK government) to maintain the existing behaviour than it would be to retrospectively migrate to the candidate/UUID-based approach.
I have intentionally left out any documentation pending feedback. I'm assuming that if this is to make it into friendly_id, it would have to be made clear that it's meant as a last resort for those unable to use the preferred candidate/UUID-based approach.
/cc @parndt