Skip to content

Fix/localized slug uniqueness #62

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

Open
wants to merge 4 commits into
base: staging
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
27 changes: 8 additions & 19 deletions admin/src/components/PermalinkInput/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ const PermalinkInput = forwardRef((props, ref) => {
!isCreatingEntry && !hasDifferentRelationUID && targetRelationValue?.id === modifiedData.id;

const initialValue = initialData[name];
const locale = initialData.locale;
const initialRelationValue = getRelationValue(initialData, targetFieldConfig.targetRelation);
const initialAncestorsPath = getPermalinkAncestors(initialValue);
const initialSlug = getPermalinkSlug(initialValue);
Expand Down Expand Up @@ -140,10 +141,14 @@ const PermalinkInput = forwardRef((props, ref) => {

if (!newSlug) {
setIsLoading(false);

return;
}

const params = `${contentTypeUID}/${encodeURIComponent(newSlug)}`;
const params = `${contentTypeUID}/${encodeURIComponent(newSlug)}${
locale ? `/${locale}` : ''
}`;

const endpoint = getApiUrl(`${pluginId}/check-availability/${params}`);

const { data } = await fetchClient.get(endpoint);
Expand Down Expand Up @@ -220,6 +225,7 @@ const PermalinkInput = forwardRef((props, ref) => {
if (!targetRelationValue) {
removeAncestorsPath();
setIsLoading(false);

return;
}

Expand Down Expand Up @@ -322,6 +328,7 @@ const PermalinkInput = forwardRef((props, ref) => {
setIsOrphan(false);
setAncestorsPath(null);
setFieldError(null);

return;
}

Expand Down Expand Up @@ -451,24 +458,6 @@ const PermalinkInput = forwardRef((props, ref) => {
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [isCreatingEntry, isCustomized, debouncedTargetValue]);

/*
This use effect clashes with the check connection use effect,
effectively overwriting the ancestors path when changing locales.
I am leaving this here for now as I dont know what other potential
side effects this has, but it is not needed for the ancestors path
to be correctly set.
*/

// useEffect(() => {
// // This is required for scenarios like switching between locales to ensure
// // the field value updates with the locale change.
// const newAncestorsPath = getPermalinkAncestors(initialValue);
// const newSlug = getPermalinkSlug(initialValue);

// setFieldState(newAncestorsPath, newSlug, true);
// // eslint-disable-next-line react-hooks/exhaustive-deps
// }, [initialData.id]);

useEffect(() => {
// Remove ancestors path if we have selected the current entity as the parent.
if (selectedSelfRelation) {
Expand Down
2 changes: 1 addition & 1 deletion admin/src/contentManagerHooks/filter-permalink-columns.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const filterPermalinkColumns = ({ displayedHeaders, layout }) => {

return {
...header,
cellFormatter: (props) => {
cellFormatter(props) {
const value = props[header.name];
const ancestorsPath = getPermalinkAncestors(value);
const slug = getPermalinkSlug(value);
Expand Down
2 changes: 1 addition & 1 deletion admin/src/hooks/use-permalink.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const usePermalink = (uid, data, isCreatingEntry) => {
}

setUrl(url);
setCopy(state?.copy === false ? false : true);
setCopy(state?.copy !== false);
}, [contentTypes, data, uid, setCopy, setUrl, runHookWaterfall]);

useEffect(() => {
Expand Down
2 changes: 1 addition & 1 deletion server/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ module.exports = {
contentTypes: [],
lowercase: true,
},
validator: (config) => {
validator(config) {
if (!config.contentTypes) {
return;
}
Expand Down
6 changes: 3 additions & 3 deletions server/controllers/permalinks.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,16 @@ module.exports = {
},

async checkAvailability(ctx) {
const { uid, value } = ctx.request.params;
const { uid, value, locale } = ctx.request.params;
const decodedValue = decodeURIComponent(value);

// Validate UID.
await getService('validation').validateUIDInput(uid);

// Check availability and maybe provide a suggestion.
const { isAvailable, suggestion } = await getService('permalinks').getAvailability(
uid,
decodedValue
decodedValue,
locale
);

ctx.send({
Expand Down
8 changes: 7 additions & 1 deletion server/lifecycles/before-create-update.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ module.exports = async ({ strapi }) => {
const id = get(where, 'id', null);
const attr = layouts[uid];
const value = data[attr.name];
let locale = data.locale || undefined;

if (id && !locale && model.attributes.locale) {
const entity = await strapi.db.query(uid).findOne({ where: { id } });
locale = entity.locale;
}

await getService('validation').validateFormat(uid, value);
await getService('validation').validateConnection(uid, data, id);
Expand All @@ -27,7 +33,7 @@ module.exports = async ({ strapi }) => {
const { name } = layouts[uid];

return getService('validation')
.validateAvailability(uid, name, value, id)
.validateAvailability(uid, name, value, id, locale)
.then((available) => ({
uid,
available,
Expand Down
2 changes: 1 addition & 1 deletion server/routes/admin-api.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ module.exports = {
},
{
method: 'GET',
path: '/check-availability/:uid/:value',
path: '/check-availability/:uid/:value/:locale?',
handler: 'permalinks.checkAvailability',
config: {
policies: ['admin::isAuthenticatedAdmin'],
Expand Down
8 changes: 4 additions & 4 deletions server/services/permalinks.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,16 @@ module.exports = ({ strapi }) => ({
return path;
},

async getAvailability(uid, value) {
async getAvailability(uid, value, locale) {
const layouts = await getService('config').layouts();
const uids = await getService('config').uids(uid);

// Check availability in each related collection.
const promisedAvailables = await Promise.all(
uids.map((_uid) => {
const { name } = layouts[_uid];

return getService('validation')
.validateAvailability(_uid, name, value)
.validateAvailability(_uid, name, value, null, locale)
.then((available) => ({
uid: _uid,
available,
Expand Down Expand Up @@ -74,7 +73,8 @@ module.exports = ({ strapi }) => ({
return null;
}

let entity, ancestor;
let entity;
let ancestor;

// Either get the ancestor from the connecting relation or look it up in the database.
if (isConnecting) {
Expand Down
26 changes: 20 additions & 6 deletions server/services/validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,33 @@ module.exports = ({ strapi }) => ({
return false;
},

async validateAvailability(uid, name, value, id = null) {
let where = { [name]: value };
async validateAvailability(uid, name, value, id = null, locale = undefined) {
const model = strapi.db.metadata.get(uid);

const { pluginOptions } = model;

const { i18n } = pluginOptions;

const isLocalized = (i18n && i18n.localized) || false;

const where = { [name]: value };
// If `id` is not null, omit it from the results so we aren't comparing against itself.
if (id) {
where.id = {
$ne: id,
};
}

// takes the locale into consideration for uniqueness, if the locale is present, and the model is localized
if (locale && isLocalized) {
where.locale = {
$eq: locale,
};
}

const count = await strapi.db.query(uid).count({ where });

return count > 0 ? false : true;
return !(count > 0);
},

async validateConnection(uid, data, id = null) {
Expand Down Expand Up @@ -124,9 +138,9 @@ module.exports = ({ strapi }) => ({
const { attributes } = model;

// Ensure that exactly one permalink attribute is defined for this model.
const permalinkAttrs = Object.entries(attributes).filter(([, attr]) => {
return attr.customField === UID_PERMALINK_FIELD;
});
const permalinkAttrs = Object.entries(attributes).filter(
([, attr]) => attr.customField === UID_PERMALINK_FIELD
);

if (permalinkAttrs.length !== 1) {
throw new ValidationError(`Must have exactly one permalink attribute defined in ${uid}.`);
Expand Down
6 changes: 3 additions & 3 deletions server/utils/get-permalink-attr.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ const pluginId = require('./plugin-id');
const getPermalinkAttr = (uid) => {
const model = strapi.getModel(uid);

const permalinkAttr = Object.entries(model.attributes).find(([, attr]) => {
return attr.customField === UID_PERMALINK_FIELD;
});
const permalinkAttr = Object.entries(model.attributes).find(
([, attr]) => attr.customField === UID_PERMALINK_FIELD
);

if (!permalinkAttr) {
return null;
Expand Down
5 changes: 2 additions & 3 deletions server/utils/get-permalink-slug.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
'use strict';

const getPermalinkSlug = (path) => {
return path
const getPermalinkSlug = (path) =>
path
? path
.split('/')
.filter((i) => i)
.reverse()[0]
: '';
};

module.exports = getPermalinkSlug;