-
Notifications
You must be signed in to change notification settings - Fork 615
PRO-8154: Strip Latin accents #5137
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
base: main
Are you sure you want to change the base?
Changes from all commits
537dd1a
a2c5a4e
8aa5d8c
9f3ad2f
bfd97a8
3f48b5b
5e81886
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -70,7 +70,9 @@ module.exports = { | |
| i18n: { | ||
| ns: 'apostrophe', | ||
| browser: true | ||
| } | ||
| }, | ||
| // If true, slugifying will strip accents from Latin characters | ||
| stripUrlAccents: false | ||
| }, | ||
| async init(self) { | ||
| self.defaultNamespace = 'default'; | ||
|
|
@@ -677,7 +679,8 @@ module.exports = { | |
| debug: self.debug, | ||
| show: self.show, | ||
| action: self.action, | ||
| crossDomainClipboard: req.session && req.session.aposCrossDomainClipboard | ||
| crossDomainClipboard: req.session && req.session.aposCrossDomainClipboard, | ||
| stripUrlAccents: self.options.stripUrlAccents | ||
myovchev marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }; | ||
| if (req.session && req.session.aposCrossDomainClipboard) { | ||
| req.session.aposCrossDomainClipboard = null; | ||
|
|
@@ -734,6 +737,9 @@ module.exports = { | |
| } | ||
| return locale; | ||
| }, | ||
| shouldStripAccents(req) { | ||
| return self.options.stripUrlAccents === true; | ||
| }, | ||
| addLocalizeModal() { | ||
| self.apos.modal.add( | ||
| `${self.__meta.name}:localize`, | ||
|
|
@@ -757,7 +763,7 @@ module.exports = { | |
| } | ||
| req.baseUrlWithPrefix = `${req.baseUrl}${self.apos.prefix}`; | ||
| req.absoluteUrl = req.baseUrlWithPrefix + req.url; | ||
| req.prefix = `${req.baseUrlWithPrefix}${self.locales[req.locale].prefix || ''}`; | ||
| req.prefix = `${req.baseUrlWithPrefix}${self.locales[req.locale]?.prefix || ''}`; | ||
myovchev marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (!req.baseUrl) { | ||
| // Always set for bc, but in the absence of locale hostnames we | ||
| // set it later so it is not part of req.prefix | ||
|
|
@@ -1251,6 +1257,54 @@ module.exports = { | |
| console.log(`Due to conflicts, kept ${kept} documents from ${keep}`); | ||
| } | ||
| } | ||
| }, | ||
| 'strip-slug-accents': { | ||
| usage: 'Remove Latin accent characters from all document slugs and attachment names. Usage: node app @apostrophecms/i18n:strip-slug-accents', | ||
| async task() { | ||
| let docChanged = 0; | ||
| let attachmentChanged = 0; | ||
|
|
||
| await self.apos.migration.eachDoc({}, 5, async doc => { | ||
| const slug = doc.slug; | ||
| const req = self.apos.task.getAdminReq({ | ||
| locale: doc.aposLocale?.split(':')[0] || self.defaultLocale | ||
| }); | ||
| if (!self.shouldStripAccents(req)) { | ||
| return; | ||
| } | ||
|
|
||
| doc.slug = _.deburr(doc.slug); | ||
| if (slug !== doc.slug) { | ||
| const manager = self.apos.doc.getManager(doc.type); | ||
| if (!manager) { | ||
| return; | ||
| } | ||
| await manager.update(req, doc, { permissions: false }); | ||
| docChanged++; | ||
| self.apos.util.log(`Updated doc [${req.locale}] "${slug}" -> "${doc.slug}"`); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. During my testing I saw this There are too many changes I think. Something seems off.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is definitely off. This is very very off
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Switched to deburr instead of slugify. Could you please retest with the DB that showed those results? |
||
| } | ||
| }); | ||
|
|
||
| const req = self.apos.task.getAdminReq(); | ||
| await self.apos.attachment.each({}, 10, async (attachment) => { | ||
| if (!self.shouldStripAccents(req)) { | ||
| return; | ||
| } | ||
| const slug = _.deburr(attachment.name); | ||
| if (slug !== attachment.name) { | ||
| await self.apos.attachment.db.updateOne( | ||
myovchev marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { _id: attachment._id }, | ||
| { $set: { name: slug } } | ||
| ); | ||
| attachmentChanged++; | ||
| self.apos.util.log(`Updated attachment "${attachment.name}" -> "${slug}"`); | ||
| } | ||
| }); | ||
|
|
||
| self.apos.util.log( | ||
| `Updated ${docChanged} document slug(s) and ${attachmentChanged} attachment name(s).` | ||
| ); | ||
| } | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |
| // errors. | ||
| import { klona } from 'klona'; | ||
| import sluggo from 'sluggo'; | ||
| import { deburr } from 'lodash'; | ||
| import AposInputMixin from 'Modules/@apostrophecms/schema/mixins/AposInputMixin'; | ||
| import { debounceAsync } from 'Modules/@apostrophecms/ui/utils'; | ||
|
|
||
|
|
@@ -48,6 +49,9 @@ export default { | |
| }, | ||
| localePrefix() { | ||
| return this.field.page && apos.i18n.locales[apos.i18n.locale].prefix; | ||
| }, | ||
| stripAccents() { | ||
| return apos.i18n.stripUrlAccents === true; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Having this here will make it difficult to have this feature enabled/disabled per locales.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is one of the main showstoppers for per locale feature, which we don't implement with the current PR. |
||
| } | ||
| }, | ||
| watch: { | ||
|
|
@@ -192,6 +196,9 @@ export default { | |
| } | ||
|
|
||
| let slug = sluggo(s, options); | ||
| if (this.stripAccents) { | ||
| slug = deburr(slug); | ||
| } | ||
| if (preserveDash) { | ||
| slug += '-'; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -291,8 +291,18 @@ module.exports = { | |
| // ONE punctuation character normally forbidden in slugs may | ||
| // optionally be permitted by specifying it via options.allow. | ||
| // The separator may be changed via options.separator. | ||
| // By default, the i18n.options.stripUrlAccents option is honored; | ||
| // having stripAccents passed as an option takes precedence. | ||
| slugify(s, options) { | ||
| return require('sluggo')(s, options); | ||
| const { stripAccents, ...opts } = options || {}; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This new option is a challenge because we need to update all the usages of the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The accent cleaning behavior can't break BC because it's a new feature. This code is fully BC. |
||
| const slug = require('sluggo')(s, opts); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moving the require is out of context for this PR. (Many other utils does it). |
||
| const shouldStripAccents = (typeof stripAccents !== 'undefined') | ||
| ? stripAccents | ||
| : self.apos.i18n.options.stripUrlAccents; | ||
| if (shouldStripAccents) { | ||
| return _.deburr(slug); | ||
| } | ||
| return slug; | ||
| }, | ||
| // Returns a string that, when used for indexes, behaves | ||
| // similarly to MySQL's default behavior for sorting, plus a little | ||
|
|
@@ -938,7 +948,8 @@ module.exports = { | |
| // ONE punctuation character normally forbidden in slugs may | ||
| // optionally be permitted by specifying it via options.allow. | ||
| // The separator may be changed via options.separator. | ||
|
|
||
| // By default, the i18n.options.stripUrlAccents option is honored; | ||
| // having stripAccents passed as an option takes precedence. | ||
| slugify: function(string, options) { | ||
| return self.slugify(string, options); | ||
| }, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.