ensure slug history prefers the record that most recently used the slug

[mtuckergh]

Slugs can be reused by different model instances when the slug is reassigned directly, as in model_instance.slug = "value_already_in_history". This means you can have more than one instance of the slug value for the same type in the history.
Currently, in this scenario, the first model instance to use the slug value will win out in a history search, because the slug_table_record method relies on natural sorting (by id) and selects the first record.
It seems more logical to retrieve the model instance that most recently used the slug value, which would actually be the last record with the slug value in the slug history table.

This PR sets an explicit descending order by created_at on the history table search, so that the selected model is the last one to use the target slug value.

[mtuckergh]

Looks like new test fails on Travis CI. They are passing locally for me. 😞
Likely to do with sorting by created_at. Perhaps I should sort by ID, since it gives the same result and may be slightly more dependable since ID is guaranteed unique. Will post update shortly.

[coveralls]

Coverage decreased (-0.71%) to 95.98% when pulling f542b5a on godaddy:most_recently_used into a46bf38 on norman:5.0-stable.

[coveralls]

Coverage decreased (-0.71%) to 95.98% when pulling f542b5a on godaddy:most_recently_used into a46bf38 on norman:5.0-stable.

[coveralls]

Coverage decreased (-0.71%) to 95.98% when pulling f542b5a on godaddy:most_recently_used into a46bf38 on norman:5.0-stable.

[coveralls]

Coverage decreased (-0.71%) to 95.98% when pulling f542b5a on godaddy:most_recently_used into a46bf38 on norman:5.0-stable.

[coveralls]

Coverage decreased (-0.71%) to 95.98% when pulling f542b5a on godaddy:most_recently_used into a46bf38 on norman:5.0-stable.

[coveralls]

Coverage decreased (-0.71%) to 95.98% when pulling f542b5a on godaddy:most_recently_used into a46bf38 on norman:5.0-stable.

[coveralls]

Coverage decreased (-0.71%) to 95.98% when pulling f542b5a on godaddy:most_recently_used into a46bf38 on norman:5.0-stable.

[mtuckergh]

ok. build is happy now. not sure why coverage dropped (even by the factional percentage) since I only changed an existing line and added a spec for that change.

[mtuckergh]

@norman any thoughts on this?

[norman]

Makes perfect sense - thanks for your work on this.

[norman]

Thinking about this some more, it seems like a bug that a record can claim a deleted slug from another record's slug history, that's not intentional behavior at all. Once a slug is used by a record, no other record should be able to use it ever, unless it's deleted from this history.

[mtuckergh]

I have to disagree. Slug history is generally an SEO optimization. You only fall back to history when no records are actively using it. Staking claim on a name forever, simply by using it once, seems very narrowly focused. If another record decides to actively use a slug that only exists in history, it should be able to do so, I would think.
I feel like there could be a different mixin that enforces uniqueness in the history, thus enforcing the permanent ownership of a slug.
Am I missing the core intent of slug history?

[norman]

I think either design choice is certainly valid, they are different use cases. To reuse a slug you could force people to intentionally delete it from the history. That has good and bad parts. My sense of alarm comes from the fact that the details of how this works have slipped by me for quite some time now. Although I'm generally not in favor of adding more configuration options, perhaps this would be a reasonable place to introduce one in 5.2.0 (the version I'm intending to release this PR in). Or, if not a configuration option, then a method that could be overridden, and better documentation on how this feature is actually working.

[mtuckergh]

This does seem like a reasonable place to provide a configuration option, something like "allow_slug_reuse"; wouldn't add much in terms of complexity.
Since the gem has historically not enforced uniqueness in the history table, I would suggest the default behavior to be to allow reuse.

[norman]

This is now out in 5.2.0.beta.1. No timetable for stable release yet, but probably within a couple weeks. I'm still thinking about the discussion here and haven't taken action yet, but I'll likely include a configuration option in 5.2.0.

[ananyo2012]

@norman Is this feature available in 5.2.0.beta.1? I tried using it but it gave the same usual results. I was also finding a way to reuse old slugs. I also don't see the change suggested in the 5.0-stable branch where it was merged.

[JulienItard]

@norman Hello, is there any news about This does seem like a reasonable place to provide a configuration option, something like "allow_slug_reuse" ? thanks