Bump friendly #5321
Merged
Bump friendly #5321
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Bumps [friendly_id](https://github.com/norman/friendly_id) from 5.2.4 to 5.3.0. - [Release notes](https://github.com/norman/friendly_id/releases) - [Changelog](https://github.com/norman/friendly_id/blob/master/Changelog.md) - [Commits](norman/friendly_id@5.2.4...5.3.0) Signed-off-by: dependabot-preview[bot] <[email protected]>
This commit re-applies the changes made in the commit here - 1f8dd08 It was reverted later on due to an unforeseen issue relating to duplicate slugs between PDF's and HTML attachments but the fix for that issue will be addressed in the next commit.
1pretz1
force-pushed
the
bump-friendly-id
branch
from
8db33e1 to
43962bf
Compare
gclssvglx
reviewed
Feb 6, 2020
Friendly by default uses the slug of a record to generate a route param when generating a link, if a slug isn't present then the default rails behaviour is used which uses the id. For example an attachment with a slug field set to 'number-1': Without friendly E.g link_to "Attachment", @attachment -> /attachment/1 With friendly E.g link_to "attachment", @attachment -> /attachment/number-1 We need to overide this behaviour as we were facing a production error which meant the slugs between attachments were the same leading to the wrong attachment being opened when visiting the edit page for the other attachment. The solution is now to use the attachment id's for routing like was implemented previously (before this gem bump) for non HTML attachments / foreign HTML attachments. We can do this via returning false for `should_generate_new_friendly_id?` which prevents slugs being created for certain attachments that were never meant to have them in the first place... problem solved! Also the friendly routes need to be set to false, this is to stop friendly from using slugs as part of route generation for file and external attachments. This shouldn't happen anyway as there shouldn't be slugs on these attachments but due to the data sync from production to integration some slugs obtain a value. Setting routes to false stops these being used.
Non HTML attachments shouldn't have slugs but this was allowed to happen in the initial bump of this gem. At the time we thought this wouldn't make a difference but a bug was introduced so this is a migration to revert all slugs to nil given they're not html attachments.
1pretz1
force-pushed
the
bump-friendly-id
branch
from
75b5410 to
3539b86
Compare
kevindew
approved these changes
Feb 6, 2020
| @@ -0,0 +1,4 @@ | |||
| Attachment | |||
| .where.not(type: "HtmlAttachment") | |||
| .where.not(slug: nil) | |||
There was a problem hiding this comment.
Minor - this can be where.not(type: "HtmlAttachment", slug: nil)
There was a problem hiding this comment.
Nice, I normally like to put my where clauses on separate lines to be super explicit as I think it reads better but thats just personal preference!
|
Tested on integration, all working as expected so going to merge! 🚀 |
ChrisBAshton
added a commit
that referenced
this pull request
Feb 5, 2025
This PR: - No longer sets `slug` to `nil` when the locale is non-English. Every locale is considered "sluggable". - This also means we no longer need the `sluggable_locale?`, `slug_eligible?` methods and ASCII checks. (The latter has caused a number of Zendesk tickets, whereby even English HTML Attachments have been given a content-ID based URL, because they contain a non-ASCII character like a fancy single quote). - `sluggable_string` doesn't appear to have been in use, so it was deleted. (`sluggable_string` is used in the Document class: `friendly_id :sluggable_string`. But the Attachment class uses `friendly_id :title`, so overriding the `sluggable_string` method had no effect). - Adds a test for friendly_id's native "uniqueness" feature (which we've opted into via `use: :scoped, scope: :attachable` on the Attachment class), so in theory we should be protected from clashes in the attachment slug namespace. Therefore the issue alluded to in #5321 (which reverts the last time this was attempted) should not happen again. - Adds a basic safeguard that the slug-generation results in a slug that contains at least 4 a-z or A-Z characters. This is to protect against generating a slug like `-`, which could happen from sluggifying a title like `{chinese}-{chinese}`. - That said, if we somehow DO get into a state whereby an attachment has what we'd call an invalid slug (<4 azAZ chars), we don't want to fix it if the attachment is already live. So we check for `safely_resluggable?`, introduced in #5942. Result: - Most HTML attachments should now have human-readable slugs. Only languages that are non-ASCII and cannot be converted to ASCII, such as Chinese, will continue to fall back to the content-ID based URLs. Trello: https://trello.com/c/YGsEG9YV/3418-improve-html-attachment-slug-creation
ChrisBAshton
added a commit
that referenced
this pull request
Feb 5, 2025
This PR: - No longer sets `slug` to `nil` when the locale is non-English. Every locale is considered "sluggable". - This also means we no longer need the `sluggable_locale?`, `slug_eligible?` methods and ASCII checks. (The latter has caused a number of Zendesk tickets, whereby even English HTML Attachments have been given a content-ID based URL, because they contain a non-ASCII character like a fancy single quote). - `sluggable_string` doesn't appear to have been in use, so it was deleted. (`sluggable_string` is used in the Document class: `friendly_id :sluggable_string`. But the Attachment class uses `friendly_id :title`, so overriding the `sluggable_string` method had no effect). - Adds a test for friendly_id's native "uniqueness" feature (which we've opted into via `use: :scoped, scope: :attachable` on the Attachment class), so in theory we should be protected from clashes in the attachment slug namespace. Therefore the issue alluded to in #5321 (which reverts the last time this was attempted) should not happen again. - Adds a basic safeguard that the slug-generation results in a slug that contains at least 4 a-z or A-Z characters. This is to protect against generating a slug like `-`, which could happen from sluggifying a title like `{chinese}-{chinese}`. - That said, if we somehow DO get into a state whereby an attachment has what we'd call an invalid slug (<4 azAZ chars), we don't want to fix it if the attachment is already live. So we check for `safely_resluggable?`, introduced in #5942. Result: - Most HTML attachments should now have human-readable slugs. Only languages that are non-ASCII and cannot be converted to ASCII, such as Chinese, will continue to fall back to the content-ID based URLs. Next steps: - Now that all Whitehall tables use the UTF8MB4 charset (#9767), we could potentially use CJK character sets natively in slugs - but it may not always show nicely in the browser. Compare `https://example.com/你好` with `https://example.com/%E4%BD%A0%E5%A5%BD`. - The option we'd likely want to go with is conversion to their Pinyin representations, e.g. `https://example.com/ni-hao`, probably using the `pinyin` or `ruby-pinyin` gem. Trello: https://trello.com/c/YGsEG9YV/3418-improve-html-attachment-slug-creation
ChrisBAshton
added a commit
that referenced
this pull request
Feb 5, 2025
This PR: - No longer sets `slug` to `nil` when the locale is non-English. Every locale is considered "sluggable". - This also means we no longer need the `sluggable_locale?`, `slug_eligible?` methods and ASCII checks. (The latter has caused a number of Zendesk tickets, whereby even English HTML Attachments have been given a content-ID based URL, because they contain a non-ASCII character like a fancy single quote). - `sluggable_string` doesn't appear to have been in use, so it was deleted. (`sluggable_string` is used in the Document class: `friendly_id :sluggable_string`. But the Attachment class uses `friendly_id :title`, so overriding the `sluggable_string` method had no effect). - Adds a test for friendly_id's native "uniqueness" feature (which we've opted into via `use: :scoped, scope: :attachable` on the Attachment class), so in theory we should be protected from clashes in the attachment slug namespace. Therefore the issue alluded to in #5321 (which reverts the last time this was attempted) should not happen again. - Adds a basic safeguard that the slug-generation results in a slug that contains at least 4 a-z or A-Z characters. This is to protect against generating a slug like `-`, which could happen from sluggifying a title like `{chinese}-{chinese}`. - That said, if we somehow DO get into a state whereby an attachment has what we'd call an invalid slug (<4 azAZ chars), we don't want to fix it if the attachment is already live. So we check for `safely_resluggable?`, introduced in #5942. Result: - Most HTML attachments should now have human-readable slugs. Only languages that are non-ASCII and cannot be converted to ASCII, such as Chinese, will continue to fall back to the content-ID based URLs. Next steps: - Now that all Whitehall tables use the UTF8MB4 charset (#9767), we could potentially use CJK character sets natively in slugs - but it may not always show nicely in the browser. Compare `https://example.com/你好` with `https://example.com/%E4%BD%A0%E5%A5%BD`. - The option we'd likely want to go with is conversion to their Pinyin representations, e.g. `https://example.com/ni-hao`, probably using the `pinyin` or `ruby-pinyin` gem. Trello: https://trello.com/c/YGsEG9YV/3418-improve-html-attachment-slug-creation
ChrisBAshton
added a commit
that referenced
this pull request
Feb 5, 2025
This PR: - No longer sets `slug` to `nil` when the locale is non-English. Every locale is considered "sluggable". - This also means we no longer need the `sluggable_locale?`, `slug_eligible?` methods and ASCII checks. (The latter has caused a number of Zendesk tickets, whereby even English HTML Attachments have been given a content-ID based URL, because they contain a non-ASCII character like a fancy single quote). - `sluggable_string` doesn't appear to have been in use, so it was deleted. (`sluggable_string` is used in the Document class: `friendly_id :sluggable_string`. But the Attachment class uses `friendly_id :title`, so overriding the `sluggable_string` method had no effect). - Adds a test for friendly_id's native "uniqueness" feature (which we've opted into via `use: :scoped, scope: :attachable` on the Attachment class), so in theory we should be protected from clashes in the attachment slug namespace. Therefore the issue alluded to in #5321 (which reverts the last time this was attempted) should not happen again. - Adds a basic safeguard that the slug-generation results in a slug that contains at least 4 a-z or A-Z characters. This is to protect against generating a slug like `-`, which could happen from sluggifying a title like `{chinese}-{chinese}`. - That said, if we somehow DO get into a state whereby an attachment has what we'd call an invalid slug (<4 azAZ chars), we don't want to fix it if the attachment is already live. So we check for `safely_resluggable?`, introduced in #5942. Result: - Most HTML attachments should now have human-readable slugs. Only languages that are non-ASCII and cannot be converted to ASCII, such as Chinese, will continue to fall back to the content-ID based URLs. Next steps: - Now that all Whitehall tables use the UTF8MB4 charset (#9767), we could potentially use CJK character sets natively in slugs - but it may not always show nicely in the browser. Compare `https://example.com/你好` with `https://example.com/%E4%BD%A0%E5%A5%BD`. - The option we'd likely want to go with is conversion to their Pinyin representations, e.g. `https://example.com/ni-hao`, probably using the `pinyin` or `ruby-pinyin` gem. Trello: https://trello.com/c/YGsEG9YV/3418-improve-html-attachment-slug-creation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR bumps friendly but adds some code so it behaves exactly the same as it did before, huraah...
Further details in the commits: ->
Trello:
https://trello.com/c/EiQSm4yx/1359-bump-friendlyid-to-version-530