Skip to content

add slug conflict handling logic #596

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.

Sign up for GitHub

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
dsurducan wants to merge 1 commit into
base: main
Choose a base branch
from
Open

add slug conflict handling logic #596

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
60 changes: 51 additions & 9 deletions packages/plugins/slugManager/manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { IAPI, BaseEntity, Meta } from "./api.ts";
import { IAPI, BaseEntity, Meta, API } from "./api.ts";

interface ProfileUpdate {
meta: Meta;
Expand All @@ -11,24 +11,35 @@ interface SlugFormatString {
fields?: never;
}

interface SlugConflictFormatString {
slugConflictFormat?: string;
fields?: never;
}

interface SlugFormatFunc {
slugFormat: (lang: string, profile: BaseEntity) => string;
fields: string[];
}

interface SlugConflictFormatFunc {
slugConflictFormat?: (lang: string, profile: BaseEntity) => string;
fields?: string[];
}

export type InternalSlugManagerConfig = {
searchIds?: string[];
entityTypes?: string[];
slugField?: string;
api: Pick<IAPI, "updateField" | "listLanguageProfiles" | "listEntities">;
} & (SlugFormatString | SlugFormatFunc);
} & (SlugFormatString | SlugFormatFunc) & (SlugConflictFormatString | SlugConflictFormatFunc);

export function createManager(config: InternalSlugManagerConfig) {
const {
searchIds = [],
slugField = "slug",
entityTypes = [],
slugFormat,
slugConflictFormat = undefined,
api,
fields,
} = config;
Expand Down Expand Up @@ -73,11 +84,13 @@ export function createManager(config: InternalSlugManagerConfig) {
const idToPrimaryLanguage = getEntityIdToPrimaryLanguageMap(
entitiesResponse.entities
);
const updates = getUpdates(
const updates = await getUpdates(
response.profileLists.flatMap((profile) => profile.profiles),
idToPrimaryLanguage,
slugField,
slugFormat
slugFormat,
api,
slugConflictFormat
);

const outputString = JSON.stringify({
Expand Down Expand Up @@ -119,25 +132,54 @@ export function createManager(config: InternalSlugManagerConfig) {
);
}

const slug =
let desiredSlug =
typeof slugFormat == "string" ? slugFormat : slugFormat(lang, profile);
return api.updateField(event.entityId, lang, slugField, slug);
const isDuplicate = await isSlugConfict(desiredSlug, api);
if (isDuplicate) {
desiredSlug = slugConflictFormat == null ? desiredSlug + "-[[entityId]]" : typeof slugConflictFormat == "string" ? slugConflictFormat : slugConflictFormat(lang, profile);
}
return api.updateField(event.entityId, lang, slugField, desiredSlug);
Comment on lines +135 to +141
Copy link

@coderabbitai coderabbitai bot Nov 10, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Update conflict detection call and fix default format.

Issues with the webhook conflict handling:

  1. The function call needs to be updated to match the corrected function signature (pass slugField and excludeEntityId).
  2. The default conflict format "-[[entityId]]" uses template syntax but won't be expanded. It should use the actual event.entityId value.
  3. Line 139 has a complex nested ternary that's hard to read.

Apply this diff:

   let desiredSlug =
     typeof slugFormat == "string" ? slugFormat : slugFormat(lang, profile);
-  const isDuplicate = await isSlugConfict(desiredSlug, api);
+  const isDuplicate = await isSlugConflict(desiredSlug, api, slugField, event.entityId);
   if (isDuplicate) {
-    desiredSlug = slugConflictFormat == null ? desiredSlug + "-[[entityId]]" : typeof slugConflictFormat == "string" ? slugConflictFormat : slugConflictFormat(lang, profile);
+    if (slugConflictFormat == null) {
+      desiredSlug = `${desiredSlug}-${event.entityId}`;
+    } else if (typeof slugConflictFormat === "string") {
+      desiredSlug = slugConflictFormat;
+    } else {
+      desiredSlug = slugConflictFormat(lang, profile);
+    }
   }
   return api.updateField(event.entityId, lang, slugField, desiredSlug);

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In packages/plugins/slugManager/manager.ts around lines 135 to 141, update the
conflict detection call and default conflict format: call
isSlugConfict(desiredSlug, api, slugField, event.entityId) (matching the new
signature that accepts slugField and excludeEntityId), replace the literal
"-[[entityId]]" with a string that concatenates the actual event.entityId (e.g.
`"-" + event.entityId`) so the ID is inserted, and simplify the nested ternary
by using a short if/else to set desiredSlug when isDuplicate (check if
slugConflictFormat is null, else if it's a string use it, else call
slugConflictFormat(lang, profile)).

}

return { connector, webhook };
}

export function getUpdates(
async function isSlugConfict(desiredSlug : string, api : Pick<IAPI, "updateField" | "listLanguageProfiles" | "listEntities">) {
const params = new URLSearchParams({
fields: ["meta", ...["slug"]].join(","),
/**
* Match entities that have the same slug.
*/
filter: JSON.stringify({
"slug": {
$contains: desiredSlug,
}
}),
});
const entitiesResponse = await api.listEntities(params);
if (entitiesResponse?.entities?.length > 1) {
return true;
}
return false;
}
Comment on lines +147 to +164
Copy link

@coderabbitai coderabbitai bot Nov 10, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix typo, hardcoded field, filter logic, and conflict detection.

This function has several critical issues:

  1. Typo: Function name should be isSlugConflict not isSlugConfict.
  2. Hardcoded field: Line 149 hardcodes "slug" but the field name is configurable via slugField parameter. This function should accept slugField as a parameter.
  3. Incorrect filter: $contains: desiredSlug (line 155) matches any slug containing the desired slug as a substring. For example, if desiredSlug is "product", it will match "my-product", "product-123", etc. Use exact match instead: $eq: desiredSlug.
  4. Wrong conflict logic: Checking > 1 (line 160) means a conflict is only detected when 2+ entities exist. However, if even 1 entity has the slug, that's a conflict for a new entity. The logic should be >= 1. Additionally, for updates to existing entities, you need to exclude the entity being updated from the conflict check (otherwise updating an entity would always detect itself as a conflict).

Apply this diff:

-async function isSlugConfict(desiredSlug : string, api : Pick<IAPI, "updateField" | "listLanguageProfiles" | "listEntities">) {
+async function isSlugConflict(
+  desiredSlug: string,
+  api: Pick<IAPI, "updateField" | "listLanguageProfiles" | "listEntities">,
+  slugField: string,
+  excludeEntityId?: string
+) {
   const params = new URLSearchParams({
-    fields: ["meta", ...["slug"]].join(","),
+    fields: ["meta", slugField].join(","),
     /**
      * Match entities that have the same slug.
      */
     filter: JSON.stringify({
-      "slug": {
-        $contains: desiredSlug,
+      [slugField]: {
+        $eq: desiredSlug,
       }
     }),
   });
-  const entitiesResponse = await api.listEntities(params);	
-  if (entitiesResponse?.entities?.length > 1) {
+  const entitiesResponse = await api.listEntities(params);
+  const conflictingEntities = excludeEntityId
+    ? entitiesResponse?.entities?.filter(e => e.meta.id !== excludeEntityId) || []
+    : entitiesResponse?.entities || [];
+  if (conflictingEntities.length > 0) {
     return true;
   }
   return false;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async function isSlugConfict(desiredSlug : string, api : Pick<IAPI, "updateField" | "listLanguageProfiles" | "listEntities">) {
const params = new URLSearchParams({
fields: ["meta", ...["slug"]].join(","),
/**
* Match entities that have the same slug.
*/
filter: JSON.stringify({
"slug": {
$contains: desiredSlug,
}
}),
});
const entitiesResponse = await api.listEntities(params);
if (entitiesResponse?.entities?.length > 1) {
return true;
}
return false;
}
async function isSlugConflict(
desiredSlug: string,
api: Pick<IAPI, "updateField" | "listLanguageProfiles" | "listEntities">,
slugField: string,
excludeEntityId?: string
) {
const params = new URLSearchParams({
fields: ["meta", slugField].join(","),
/**
* Match entities that have the same slug.
*/
filter: JSON.stringify({
[slugField]: {
$eq: desiredSlug,
}
}),
});
const entitiesResponse = await api.listEntities(params);
const conflictingEntities = excludeEntityId
? entitiesResponse?.entities?.filter(e => e.meta.id !== excludeEntityId) || []
: entitiesResponse?.entities || [];
if (conflictingEntities.length > 0) {
return true;
}
return false;
}
🤖 Prompt for AI Agents 
In packages/plugins/slugManager/manager.ts around lines 147 to 164, rename the
function to isSlugConflict and change its signature to accept slugField (string)
and an optional excludeEntityId (string | undefined) alongside the api; stop
hardcoding "slug" and use slugField in the fields param and filter key; use
exact match ($eq: desiredSlug) instead of $contains; and change conflict logic
to detect >= 1 match, excluding the current entity when excludeEntityId is
provided by adding an id $ne condition to the filter so updates don't count the
entity itself as a conflict.


export async function getUpdates(
profiles: BaseEntity[],
idToPrimaryLanguage: Record<string, string>,
slugField: string,
slugFormat: string | ((lang: string, profile: BaseEntity) => string)
slugFormat: string | ((lang: string, profile: BaseEntity) => string),
api: Pick<IAPI, "updateField" | "listLanguageProfiles" | "listEntities">,
slugConflictFormat?: string | ((lang: string, profile: BaseEntity) => string)
) {
const updates: ProfileUpdate[] = [];
for (const profile of profiles) {
const lang = profile.meta.language;
const desiredSlug =
let desiredSlug =
typeof slugFormat == "string" ? slugFormat : slugFormat(lang, profile);
const isDuplicate = await isSlugConfict(desiredSlug, api);
if (isDuplicate) {
desiredSlug = slugConflictFormat == null ? desiredSlug + "-[[entityId]]" : typeof slugConflictFormat == "string" ? slugConflictFormat : slugConflictFormat(lang, profile);
}
Comment on lines +177 to +182
Copy link

@coderabbitai coderabbitai bot Nov 10, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Update conflict detection call and fix default format.

Same issues as in the webhook flow:

  1. The conflict check needs to pass slugField and exclude the current entity ID.
  2. The default format uses template syntax that won't be expanded.
  3. The nested ternary is hard to read.

Apply this diff:

     let desiredSlug =
       typeof slugFormat == "string" ? slugFormat : slugFormat(lang, profile);
-    const isDuplicate = await isSlugConfict(desiredSlug, api);
+    const isDuplicate = await isSlugConflict(desiredSlug, api, slugField, profile.meta.id);
     if (isDuplicate) {
-      desiredSlug = slugConflictFormat == null ? desiredSlug + "-[[entityId]]" : typeof slugConflictFormat == "string" ? slugConflictFormat : slugConflictFormat(lang, profile);
+      if (slugConflictFormat == null) {
+        desiredSlug = `${desiredSlug}-${profile.meta.id}`;
+      } else if (typeof slugConflictFormat === "string") {
+        desiredSlug = slugConflictFormat;
+      } else {
+        desiredSlug = slugConflictFormat(lang, profile);
+      }
     }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
packages/plugins/slugManager/manager.ts lines ~177-182: the current block builds
desiredSlug with a nested ternary, calls isSlugConfict without passing slugField
or excluding the current entity, and uses a default suffix "-[[entityId]]" that
won't be expanded; replace with clearer imperative logic: determine desiredSlug
by calling slugFormat(lang, profile) when slugFormat is a function otherwise use
the string, call isSlugConflict(desiredSlug, api, slugField, { excludeEntityId:
profile.id }) (or equivalent args per function signature) to include slugField
and exclude the current entity, and if conflict detected set desiredSlug to
either the string slugConflictFormat or slugConflictFormat(lang, profile) when
it's a function, falling back to a default suffix that uses the project’s
templating token (e.g. "-{entityId}") instead of "-[[entityId]]"; rewrite the
nested ternary into straightforward if/else for readability.


updates.push({
[slugField]: desiredSlug,
Expand Down
28 changes: 16 additions & 12 deletions packages/plugins/slugManager/test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,23 +52,25 @@ const slugFormatFunc = (lang: string, profile: BaseEntity) => {
};
const slugFormatFuncFields = ["c_customField"];

Deno.test("getUpdates returns correct number of updates", () => {
const updates = getUpdates(
Deno.test("getUpdates returns correct number of updates", async () => {
const updates = await getUpdates(
profiles,
idToPrimaryLanguage,
slugField,
slugFormatString
slugFormatString,
baseMockAPI
);

assertEquals(updates.length, 3);
});

Deno.test("getUpdates correctly sets isAlternateProfile", () => {
const updates = getUpdates(
Deno.test("getUpdates correctly sets isAlternateProfile", async () => {
const updates = await getUpdates(
profiles,
idToPrimaryLanguage,
slugField,
slugFormatString
slugFormatString,
baseMockAPI
);

for (const update of updates) {
Expand All @@ -79,23 +81,25 @@ Deno.test("getUpdates correctly sets isAlternateProfile", () => {
}
});

Deno.test("getUpdates sets correct slug field", () => {
const updates = getUpdates(
Deno.test("getUpdates sets correct slug field", async () => {
const updates = await getUpdates(
profiles,
idToPrimaryLanguage,
slugField,
slugFormatString
slugFormatString,
baseMockAPI
);

updates.forEach((update) => assertExists(update[slugField]));
});

Deno.test("getUpdates respects slug format", () => {
const updates = getUpdates(
Deno.test("getUpdates respects slug format", async () => {
const updates = await getUpdates(
profiles,
idToPrimaryLanguage,
slugField,
slugFormatFunc
slugFormatFunc,
baseMockAPI
);

updates.forEach((update, idx) => {
Expand Down
Loading