Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Adds

* Adds `stripUrlAccents` option in `@apostrophecms/i18n` module to globally control whether accents are stripped from URLs. When set to `true`, all URLs (slugs) will have accents from Latin characters removed on document creation and updates. No existing documents are modified automatically; this only affects new or updated documents. A new task `node app @apostrophecms/i18n:strip-slug-accents` is provided to update existing document slugs and attachment `name` props in the database when needed.
* Translation strings added for the layout- and layout-column-widgets.

### Changes
Expand Down
5 changes: 4 additions & 1 deletion modules/@apostrophecms/attachment/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,10 @@ module.exports = {
_id: options.attachmentId ?? self.apos.util.generateId(),
group: group.name,
createdAt: new Date(),
name: self.apos.util.slugify(path.basename(fileName, path.extname(fileName))),
name: self.apos.util.slugify(
path.basename(fileName, path.extname(fileName)),
{ stripAccents: self.apos.i18n.shouldStripAccents(req) }
),
title: self.apos.util.sortify(
path.basename(fileName, path.extname(fileName))
),
Expand Down
18 changes: 15 additions & 3 deletions modules/@apostrophecms/doc/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,10 @@ module.exports = {
'@apostrophecms/doc-type:beforePublish': {
testPermissions(req, info) {
if (info.options.permissions !== false) {
if (!self.apos.permission.can(req, info.options.autopublishing ? 'edit' : 'publish', info.draft)) {
if (!self.apos.permission.can(
req,
info.options.autopublishing ? 'edit' : 'publish', info.draft
)) {
throw self.apos.error('forbidden');
}
}
Expand All @@ -214,10 +217,19 @@ module.exports = {
'@apostrophecms/doc-type:beforeSave': {
ensureSlugSortifyAndUpdatedAt(req, doc, options) {
const manager = self.getManager(doc.type);
manager.ensureSlug(doc);
manager.ensureSlug(
doc,
{
stripAccents: self.apos.i18n.shouldStripAccents(req)
}
);
_.each(manager.schema, function (field) {
if (field.sortify) {
doc[field.name + 'Sortified'] = self.apos.util.sortify(doc[field.name] ? doc[field.name] : '');
doc[field.name + 'Sortified'] = self.apos.util.sortify(
doc[field.name]
? doc[field.name]
: ''
);
}
});
if (options.setUpdatedAtAndBy !== false) {
Expand Down
59 changes: 56 additions & 3 deletions modules/@apostrophecms/i18n/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
};
if (req.session && req.session.aposCrossDomainClipboard) {
req.session.aposCrossDomainClipboard = null;
Expand Down Expand Up @@ -734,6 +737,9 @@ module.exports = {
}
return locale;
},
shouldStripAccents(req) {
return self.options.stripUrlAccents === true;
},
addLocalizeModal() {
self.apos.modal.add(
`${self.__meta.name}:localize`,
Expand All @@ -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 || ''}`;
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
Expand Down Expand Up @@ -1251,6 +1257,53 @@ 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] || 'en'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forcing the locale to en seems wrong. We have i18n defaultLocale for that.
If you're using doc.aposLocale you can also pass it as query: { aposLocale: doc.aposLocale } and let the i18n middleware do its magic.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aposLocale can be undefined. I'll switch to default locale though.

});
if (!self.shouldStripAccents(req)) {
return;
}

doc.slug = self.apos.util.slugify(doc.slug, {
stripAccents: true,
allow: '/'
});
if (slug !== doc.slug) {
await self.apos.doc.update(req, doc, { permissins: false });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a typo with permissions.

Updating using self.apos.doc.update means there are no redirects created. The old URLs give me 404s now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, I'll switch to manager update.

docChanged++;
self.apos.util.log(`Updated doc [${req.locale}] "${slug}" -> "${doc.slug}"`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During my testing I saw this

Updated doc [en] "/@copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-template-02" -> "/@template-13"
Updated doc [en] "/@copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-template-02" -> "/@template-09"
Updated doc [en] "/@copy-of-copy-of-copy-of-copy-of-template-02" -> "/@template-06"
Updated doc [en] "/@copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-template-02" -> "/@template-13"
Updated doc [en] "/@copy-of-copy-of-copy-of-copy-of-template-02" -> "/@template-06"
Updated doc [en] "/@copy-of-copy-of-copy-of-template-02" -> "/@template-05"
Updated doc [en] "/@copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-template-02" -> "/@template-11"
Updated doc [en] "/@copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-template-02" -> "/@template-09"
Updated doc [en] "/@copy-of-copy-of-template-02" -> "/@template-04"
Updated doc [en] "/@copy-of-copy-of-copy-of-template-02" -> "/@template-05"
Updated doc [en] "/@copy-of-template-02" -> "/@template-03"
Updated doc [en] "/@copy-of-copy-of-template-02" -> "/@template-04"
Updated doc [en] "/@template-02" -> "/@template-02"
Updated doc [en] "/@copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-template-02" -> "/@template-11"
Updated doc [en] "/@template-02" -> "/@template-02"
Updated doc [en] "/@copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-template-02" -> "/@template-10"
Updated doc [en] "/@copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-template-02" -> "/@template-10"
Updated doc [en] "@template-article-03" -> "@template-article-03"
Updated doc [en] "/@copy-of-template-02" -> "/@template-03"
Updated doc [en] "/@copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-template-02" -> "/@template-12"
Updated doc [fr] "c-est-l-été-déjà" -> "c-est-l-ete-deja"
Updated doc [en] "/@copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-template-02" -> "/@template-12"
Updated doc [en] "/@copy-of-copy-of-copy-of-copy-of-copy-of-template-02" -> "/@template-07"
Updated doc [fr] "c-est-l-été-déjà" -> "c-est-l-ete-deja"
Updated doc [en] "/@copy-of-copy-of-copy-of-copy-of-copy-of-template-02" -> "/@template-07"
Updated doc [en] "@template-article-03" -> "@template-article-03"
Updated doc [en] "/@copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-template-02" -> "/@template-08"
Updated doc [en] "/@copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-template-02" -> "/@template-08"
Updated 28 document slug(s) and 0 attachment name(s).

There are too many changes I think. Something seems off.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is definitely off. This is very very off "/@copy-of-copy-of-copy-of-copy-of-copy-of-copy-of-template-02" -> "/@template-08" (completely different slugs). I'll be investigating

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 = self.apos.util.slugify(attachment.name, { stripAccents: true });
if (slug !== attachment.name) {
await self.apos.attachment.db.updateOne(
{ _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).`
);
}
}
};
}
Expand Down
4 changes: 2 additions & 2 deletions modules/@apostrophecms/page-type/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -468,14 +468,14 @@ module.exports = {
},
// If the page does not yet have a slug, add one based on the
// title; throw an error if there is no title
ensureSlug(page) {
ensureSlug(page, { stripAccents } = {}) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this scales well. We should work towards ensureSlug and slugify keeping the same signature somehow and fetching the stripAccents from the i18n locales option in my opinion. If we don't do that, it means that we have to change multiple external/pro modules to support the new feature, and apostrophe users will have to do the same. There will always be a piece of code somewhere where stripAccents is not passed and it will lead to inconsistent behavior that will be hard to track.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ensureSlug is internal method. It's called internally via event.
Utils should not fetch/know what i18n options are, so slugify is what it is - an util with an option.
Having an option for ensureSlug is a clear contract. This is not a public API so no BC is required.

I reverted option per locale because of many and bad showstoppers. I explained above that the there was an agreement to go with the simple "rule them all" option.

if (!page.slug || (!page.slug.match(/^\//))) {
if (page.title) {
// Parent-based slug would be better, but this is not an
// async function and callers will typically have done
// that already, so skip the overhead. This is just a fallback
// for naive use of the APIs
page.slug = '/' + self.apos.util.slugify(page.title);
page.slug = '/' + self.apos.util.slugify(page.title, { stripAccents });
} else {
throw self.apos.error('invalid', 'Page has neither a slug beginning with / or a title, giving up');
}
Expand Down
4 changes: 2 additions & 2 deletions modules/@apostrophecms/piece-type/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -1146,10 +1146,10 @@ module.exports = {
},
// If the piece does not yet have a slug, add one based on the
// title; throw an error if there is no title
ensureSlug(piece) {
ensureSlug(piece, { stripAccents } = {}) {
if (!piece.slug || piece.slug === 'none') {
if (piece.title) {
piece.slug = self.apos.util.slugify(piece.title);
piece.slug = self.apos.util.slugify(piece.title, { stripAccents });
} else if (piece.slug !== 'none') {
throw self.apos.error(
'invalid',
Expand Down
1 change: 1 addition & 0 deletions modules/@apostrophecms/schema/lib/addFieldTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ module.exports = (self) => {
// (no slashes at all)
convert (req, field, data, destination) {
const options = self.getSlugFieldOptions(field, data);
options.stripAccents = self.apos.i18n.shouldStripAccents(req);

destination[field.name] = self.apos.util.slugify(
self.apos.launder.string(data[field.name]),
Expand Down
7 changes: 7 additions & 0 deletions modules/@apostrophecms/schema/ui/apos/logic/AposInputSlug.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -48,6 +49,9 @@ export default {
},
localePrefix() {
return this.field.page && apos.i18n.locales[apos.i18n.locale].prefix;
},
stripAccents() {
return apos.i18n.stripUrlAccents === true;
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

@myovchev myovchev Nov 6, 2025

Choose a reason for hiding this comment

The 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: {
Expand Down Expand Up @@ -192,6 +196,9 @@ export default {
}

let slug = sluggo(s, options);
if (this.stripAccents) {
slug = deburr(slug);
}
if (preserveDash) {
slug += '-';
}
Expand Down
12 changes: 10 additions & 2 deletions modules/@apostrophecms/util/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,15 @@ 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, accents are preserved. To strip accents,
// set options.stripAccents to true.
slugify(s, options) {
return require('sluggo')(s, options);
const { stripAccents, ...opts } = options || {};
Copy link
Contributor

Choose a reason for hiding this comment

The 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 slugify method, in our modules but our clients needs to do the same. There is a default value, but this is some kind of a breaking change, mainly because it requires changes to the code for the behavior to remain consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. We should put that require statement at the top.
  2. Is the order of operations important here? slugify then strip accents vs strip accents then slugify.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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).
The current order of operation makes sense. You don't want to change the behavior of sluggo with pre-filtering the input, but to filter the output of the sluggo.

if (stripAccents) {
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
Expand Down Expand Up @@ -938,7 +945,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, accents are preserved. To strip accents,
// set options.stripAccents to true.
slugify: function(string, options) {
return self.slugify(string, options);
},
Expand Down
121 changes: 121 additions & 0 deletions test/docs.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ describe('Docs', function() {

afterEach(async function () {
await apos.doc.db.deleteMany({ type: 'test-people' });
await apos.doc.db.deleteMany({ type: 'test-page' });
await apos.lock.db.deleteMany({});
});

Expand Down Expand Up @@ -1036,6 +1037,126 @@ describe('Docs', function() {
assert(timestamps.doc === timestamps.expected);
});

it('should preserve latin accents by default (piece)', async function () {
const req = apos.task.getReq();
const object = {
title: 'C\'est déjà l\'été',
visibility: 'public',
type: 'test-people',
firstName: 'Janis',
lastName: 'Joplin',
age: 27,
alive: false,
updatedAt: '2018-08-29T12:57:03.685Z',
cacheInvalidatedAt: '2019-08-29T12:57:03.685Z'
};

await apos.doc.insert(req, object);

const doc = await apos.doc.db.findOne({ title: 'C\'est déjà l\'été' });
assert.equal(doc.slug, 'c-est-déjà-l-été');
});

it('should remove latin accents when configured to do so (piece)', async function () {
const req = apos.task.getReq();
const object = {
title: 'C\'est déjà l\'été',
visibility: 'public',
type: 'test-people',
firstName: 'Janis',
lastName: 'Joplin',
age: 27,
alive: false,
updatedAt: '2018-08-29T12:57:03.685Z',
cacheInvalidatedAt: '2019-08-29T12:57:03.685Z'
};
const originalSetting = apos.i18n.options.stripUrlAccents;
apos.i18n.options.stripUrlAccents = true;

await apos.doc.insert(req, object);

const doc = await apos.doc.db.findOne({ title: 'C\'est déjà l\'été' });
apos.i18n.options.stripUrlAccents = originalSetting;

assert.equal(doc.slug, 'c-est-deja-l-ete');
});

it('should remove latin accents when converting schema fields (piece)', async function () {
const req = apos.task.getReq();
const input = {
title: 'C\'est déjà l\'été',
slug: 'c-est-déjà-l-été',
visibility: 'public',
type: 'test-people',
firstName: 'Janis',
lastName: 'Joplin',
age: 27,
alive: false,
updatedAt: '2018-08-29T12:57:03.685Z',
cacheInvalidatedAt: '2019-08-29T12:57:03.685Z'
};
const originalSetting = apos.i18n.options.stripUrlAccents;
apos.i18n.options.stripUrlAccents = true;

const manager = apos.doc.getManager('test-people');
const page = manager.newInstance();
await manager.convert(req, input, page);
apos.i18n.options.stripUrlAccents = originalSetting;

assert.equal(page.slug, 'c-est-deja-l-ete');
});

it('should preserve latin accents by default (page)', async function () {
const req = apos.task.getReq();
const object = {
title: 'C\'est déjà l\'été',
visibility: 'public',
type: 'test-page'
};

await apos.doc.insert(req, object);

const doc = await apos.doc.db.findOne({ title: 'C\'est déjà l\'été' });
assert.equal(doc.slug, '/c-est-déjà-l-été');
});

it('should remove latin accents when configured to do so (page)', async function () {
const req = apos.task.getReq();
const object = {
title: 'C\'est déjà l\'été',
visibility: 'public',
type: 'test-page'
};
const originalSetting = apos.i18n.options.stripUrlAccents;
apos.i18n.options.stripUrlAccents = true;

await apos.doc.insert(req, object);

const doc = await apos.doc.db.findOne({ title: 'C\'est déjà l\'été' });
apos.i18n.options.stripUrlAccents = originalSetting;

assert.equal(doc.slug, '/c-est-deja-l-ete');
});

it('should remove latin accents when converting schema fields (page)', async function () {
const req = apos.task.getReq();
const input = {
title: 'C\'est déjà l\'été',
slug: '/c-est-déjà-l-été',
visibility: 'public',
type: 'test-page'
};
const originalSetting = apos.i18n.options.stripUrlAccents;
apos.i18n.options.stripUrlAccents = true;

const manager = apos.doc.getManager('test-page');
const page = manager.newInstance();
await manager.convert(req, input, page);
apos.i18n.options.stripUrlAccents = originalSetting;

assert.equal(page.slug, '/c-est-deja-l-ete');
});

/// ///
// CACHING
/// ///
Expand Down
Loading