fix(hub-common): ensure entity slug is unique and typekeyword is in s…

…ync (#1693)

packages/common/src/discussions/edit.ts

@@ -1,7 +1,8 @@
 import { IUserItemOptions, removeItem } from "@esri/arcgis-rest-portal";
-import { IHubDiscussion } from "../core/types";
+import { IHubDiscussion, IHubItemEntity } from "../core/types";
 import { createModel, getModel, updateModel } from "../models";
-import { constructSlug, getUniqueSlug, setSlugKeyword } from "../items/slugs";
+import { constructSlug } from "../items/slugs";
+import { ensureUniqueEntitySlug } from "../items/_internal/slugs";
 import {
   createSetting,
   removeSetting,
@@ -41,16 +42,8 @@ export async function createDiscussion(
   if (!discussion.slug) {
     discussion.slug = constructSlug(discussion.name, discussion.orgUrlKey);
   }
-  // Ensure slug is  unique
-  discussion.slug = await getUniqueSlug(
-    { slug: discussion.slug },
-    requestOptions
-  );
-  // add slug to keywords
-  discussion.typeKeywords = setSlugKeyword(
-    discussion.typeKeywords,
-    discussion.slug
-  );
+  // Ensure slug is unique
+  await ensureUniqueEntitySlug(discussion as IHubItemEntity, requestOptions);
   discussion.typeKeywords = setDiscussableKeyword(
     discussion.typeKeywords,
     discussion.isDiscussable
@@ -101,10 +94,7 @@ export async function updateDiscussion(
   requestOptions: IHubRequestOptions
 ): Promise<IHubDiscussion> {
   // verify that the slug is unique, excluding the current discussion
-  discussion.slug = await getUniqueSlug(
-    { slug: discussion.slug, existingId: discussion.id },
-    requestOptions
-  );
+  await ensureUniqueEntitySlug(discussion as IHubItemEntity, requestOptions);
   discussion.typeKeywords = setDiscussableKeyword(
     discussion.typeKeywords,
     discussion.isDiscussable

packages/common/src/initiative-templates/edit.ts

@@ -2,7 +2,7 @@ import { IUserRequestOptions } from "@esri/arcgis-rest-auth";
 
 // Note - we separate these imports so we can cleanly spy on things in tests
 import { createModel, getModel, updateModel } from "../models";
-import { constructSlug, getUniqueSlug, setSlugKeyword } from "../items/slugs";
+import { constructSlug } from "../items/slugs";
 import {
   IPortal,
   IUserItemOptions,
@@ -22,6 +22,8 @@ import { getPropertyMap } from "./_internal/getPropertyMap";
 import { cloneObject } from "../util";
 import { setDiscussableKeyword } from "../discussions";
 import { IModel } from "../types";
+import { ensureUniqueEntitySlug } from "../items/_internal/slugs";
+import { IHubItemEntity } from "../core";
 
 /**
  * @private
@@ -48,15 +50,10 @@ export async function createInitiativeTemplate(
   }
 
   // Ensure slug is unique
-  initiativeTemplate.slug = await getUniqueSlug(
-    { slug: initiativeTemplate.slug },
+  await ensureUniqueEntitySlug(
+    initiativeTemplate as IHubItemEntity,
     requestOptions
   );
-  // add slug to keywords
-  initiativeTemplate.typeKeywords = setSlugKeyword(
-    initiativeTemplate.typeKeywords,
-    initiativeTemplate.slug
-  );
   initiativeTemplate.typeKeywords = setDiscussableKeyword(
     initiativeTemplate.typeKeywords,
     initiativeTemplate.isDiscussable
@@ -115,8 +112,8 @@ export async function updateInitiativeTemplate(
   requestOptions: IUserRequestOptions
 ): Promise<IHubInitiativeTemplate> {
   // verify that the slug is unique, excluding the current initiative template
-  initiativeTemplate.slug = await getUniqueSlug(
-    { slug: initiativeTemplate.slug, existingId: initiativeTemplate.id },
+  await ensureUniqueEntitySlug(
+    initiativeTemplate as IHubItemEntity,
     requestOptions
   );
   // set discussable keyword

packages/common/src/initiatives/HubInitiatives.ts

@@ -7,12 +7,7 @@ import {
   getModel,
   updateModel,
 } from "../models";
-import {
-  constructSlug,
-  getItemBySlug,
-  getUniqueSlug,
-  setSlugKeyword,
-} from "../items/slugs";
+import { constructSlug, getItemBySlug } from "../items/slugs";
 
 import {
   isGuid,
@@ -38,7 +33,7 @@ import {
 import { IRequestOptions } from "@esri/arcgis-rest-request";
 
 import { PropertyMapper } from "../core/_internal/PropertyMapper";
-import { IEntityInfo, IHubInitiative } from "../core/types";
+import { IEntityInfo, IHubInitiative, IHubItemEntity } from "../core/types";
 import { IHubSearchResult } from "../search";
 import { parseInclude } from "../search/_internal/parseInclude";
 import { fetchItemEnrichments } from "../items/_enrichments";
@@ -62,6 +57,7 @@ import { createId } from "../util";
 import { IArcGISContext } from "../ArcGISContext";
 import { convertHubGroupToGroup } from "../groups/_internal/convertHubGroupToGroup";
 import { IHubGroup } from "../core/types/IHubGroup";
+import { ensureUniqueEntitySlug } from "../items/_internal/slugs";
 
 /**
  * @private
@@ -85,15 +81,7 @@ export async function createInitiative(
     initiative.slug = constructSlug(initiative.name, initiative.orgUrlKey);
   }
   // Ensure slug is  unique
-  initiative.slug = await getUniqueSlug(
-    { slug: initiative.slug },
-    requestOptions
-  );
-  // add slug to keywords
-  initiative.typeKeywords = setSlugKeyword(
-    initiative.typeKeywords,
-    initiative.slug
-  );
+  await ensureUniqueEntitySlug(initiative as IHubItemEntity, requestOptions);
   // add the status keyword
   initiative.typeKeywords = setEntityStatusKeyword(
     initiative.typeKeywords,
@@ -208,10 +196,7 @@ export async function updateInitiative(
   requestOptions: IUserRequestOptions
 ): Promise<IHubInitiative> {
   // verify that the slug is unique, excluding the current initiative
-  initiative.slug = await getUniqueSlug(
-    { slug: initiative.slug, existingId: initiative.id },
-    requestOptions
-  );
+  await ensureUniqueEntitySlug(initiative as IHubItemEntity, requestOptions);
   // update the status keyword
   initiative.typeKeywords = setEntityStatusKeyword(
     initiative.typeKeywords,

packages/common/src/items/_internal/slugs.ts

@@ -0,0 +1,17 @@
+import { IHubItemEntity } from "../../core";
+import { IHubRequestOptions } from "../../types";
+import { getUniqueSlug, setSlugKeyword } from "../slugs";
+
+// NOTE: this mutates the entity that is passed in
+export const ensureUniqueEntitySlug = async (
+  entity: IHubItemEntity,
+  requestOptions: IHubRequestOptions
+): Promise<IHubItemEntity> => {
+  // verify that the slug is unique
+  const { id: existingId, slug } = entity;
+  const slugInfo = existingId ? { slug, existingId } : { slug };
+  entity.slug = await getUniqueSlug(slugInfo, requestOptions);
+  // add slug to keywords
+  entity.typeKeywords = setSlugKeyword(entity.typeKeywords, entity.slug);
+  return entity;
+};

packages/common/src/pages/HubPages.ts

@@ -14,13 +14,8 @@ import { IRequestOptions } from "@esri/arcgis-rest-request";
 import { getItem, IItem } from "@esri/arcgis-rest-portal";
 import { cloneObject, unique } from "../util";
 import { mapBy, isGuid } from "../utils";
-import {
-  constructSlug,
-  getItemBySlug,
-  getUniqueSlug,
-  setSlugKeyword,
-} from "../items/slugs";
-import { IHubPage } from "../core";
+import { constructSlug, getItemBySlug } from "../items/slugs";
+import { IHubItemEntity, IHubPage } from "../core";
 import {
   createModel,
   fetchModelFromItem,
@@ -33,6 +28,7 @@ import { computeProps } from "./_internal/computeProps";
 import { IUserRequestOptions } from "@esri/arcgis-rest-auth";
 import { IUserItemOptions, removeItem } from "@esri/arcgis-rest-portal";
 import { DEFAULT_PAGE, DEFAULT_PAGE_MODEL } from "./defaults";
+import { ensureUniqueEntitySlug } from "../items/_internal/slugs";
 
 /**
  * @private
@@ -56,9 +52,7 @@ export async function createPage(
     page.slug = constructSlug(page.name, page.orgUrlKey);
   }
   // Ensure slug is  unique
-  page.slug = await getUniqueSlug({ slug: page.slug }, requestOptions);
-  // add slug and status to keywords
-  page.typeKeywords = setSlugKeyword(page.typeKeywords, page.slug);
+  await ensureUniqueEntitySlug(page as IHubItemEntity, requestOptions);
 
   // Map page object onto a default page Model
   const mapper = new PropertyMapper<Partial<IHubPage>, IModel>(
@@ -86,10 +80,7 @@ export async function updatePage(
   requestOptions: IUserRequestOptions
 ): Promise<IHubPage> {
   // verify that the slug is unique, excluding the current page
-  page.slug = await getUniqueSlug(
-    { slug: page.slug, existingId: page.id },
-    requestOptions
-  );
+  await ensureUniqueEntitySlug(page as IHubItemEntity, requestOptions);
 
   // get the backing item & data
   const model = await getModel(page.id, requestOptions);

packages/common/src/projects/edit.ts

@@ -9,7 +9,7 @@ import {
   removeItem,
 } from "@esri/arcgis-rest-portal";
 import { PropertyMapper } from "../core/_internal/PropertyMapper";
-import { IHubProject, IHubProjectEditor } from "../core/types";
+import { IHubItemEntity, IHubProject, IHubProjectEditor } from "../core/types";
 import { DEFAULT_PROJECT, DEFAULT_PROJECT_MODEL } from "./defaults";
 import { computeProps } from "./_internal/computeProps";
 import { getPropertyMap } from "./_internal/getPropertyMap";
@@ -19,6 +19,7 @@ import { IModel } from "../types";
 import { setEntityStatusKeyword } from "../utils/internal/setEntityStatusKeyword";
 import { editorToMetric } from "../core/schemas/internal/metrics/editorToMetric";
 import { setMetricAndDisplay } from "../core/schemas/internal/metrics/setMetricAndDisplay";
+import { ensureUniqueEntitySlug } from "../items/_internal/slugs";
 
 /**
  * @private
@@ -42,9 +43,8 @@ export async function createProject(
     project.slug = constructSlug(project.name, project.orgUrlKey);
   }
   // Ensure slug is  unique
-  project.slug = await getUniqueSlug({ slug: project.slug }, requestOptions);
-  // add slug and status to keywords
-  project.typeKeywords = setSlugKeyword(project.typeKeywords, project.slug);
+  await ensureUniqueEntitySlug(project as IHubItemEntity, requestOptions);
+  // add status to keywords
   project.typeKeywords = setEntityStatusKeyword(
     project.typeKeywords,
     project.status
@@ -121,10 +121,7 @@ export async function updateProject(
   requestOptions: IUserRequestOptions
 ): Promise<IHubProject> {
   // verify that the slug is unique, excluding the current project
-  project.slug = await getUniqueSlug(
-    { slug: project.slug, existingId: project.id },
-    requestOptions
-  );
+  await ensureUniqueEntitySlug(project as IHubItemEntity, requestOptions);
   // update the status keyword
   project.typeKeywords = setEntityStatusKeyword(
     project.typeKeywords,

packages/common/src/sites/HubSites.ts

@@ -15,7 +15,7 @@ import { applyPermissionMigration } from "./_internal/applyPermissionMigration";
 import { computeProps } from "./_internal/computeProps";
 import { getPropertyMap } from "./_internal/getPropertyMap";
 import { IHubSite } from "../core/types/IHubSite";
-import { constructSlug, getUniqueSlug, setSlugKeyword } from "../items/slugs";
+import { constructSlug } from "../items/slugs";
 import { slugify } from "../utils/slugify";
 import { ensureUniqueDomainName } from "./domains/ensure-unique-domain-name";
 import { stripProtocol } from "../urls/strip-protocol";
@@ -42,6 +42,8 @@ import { reflectCollectionsToSearchCategories } from "./_internal/reflectCollect
 import { convertCatalogToLegacyFormat } from "./_internal/convertCatalogToLegacyFormat";
 import { convertFeaturesToLegacyCapabilities } from "./_internal/capabilities/convertFeaturesToLegacyCapabilities";
 import { computeLinks } from "./_internal/computeLinks";
+import { ensureUniqueEntitySlug } from "../items/_internal/slugs";
+import { IHubItemEntity } from "../core";
 export const HUB_SITE_ITEM_TYPE = "Hub Site Application";
 export const ENTERPRISE_SITE_ITEM_TYPE = "Site Application";
 
@@ -228,9 +230,7 @@ export async function createSite(
     site.slug = constructSlug(site.name, site.orgUrlKey);
   }
   // Ensure slug is  unique
-  // site.slug = await getUniqueSlug({ slug: site.slug }, requestOptions);
-  // add slug to keywords
-  site.typeKeywords = setSlugKeyword(site.typeKeywords, site.slug);
+  await ensureUniqueEntitySlug(site as IHubItemEntity, requestOptions);
 
   if (!site.subdomain) {
     site.subdomain = slugify(site.name);
@@ -334,10 +334,7 @@ export async function updateSite(
   requestOptions: IHubRequestOptions
 ): Promise<IHubSite> {
   // verify that the slug is unique, excluding the current site
-  site.slug = await getUniqueSlug(
-    { slug: site.slug, existingId: site.id },
-    requestOptions
-  );
+  await ensureUniqueEntitySlug(site as IHubItemEntity, requestOptions);
   site.typeKeywords = setDiscussableKeyword(
     site.typeKeywords,
     site.isDiscussable

packages/common/src/templates/edit.ts

@@ -13,6 +13,7 @@ import {
   IUserItemOptions,
   removeItem,
 } from "@esri/arcgis-rest-portal";
+import { ensureUniqueEntitySlug } from "../items/_internal/slugs";
 
 /**
  * @private
@@ -47,10 +48,7 @@ export async function updateTemplate(
   requestOptions: IUserRequestOptions
 ): Promise<IHubTemplate> {
   // 1. Verify the slug is unique, excluding the current template
-  template.slug = await getUniqueSlug(
-    { slug: template.slug, existingId: template.id },
-    requestOptions
-  );
+  await ensureUniqueEntitySlug(template, requestOptions);
 
   // 2. Update relevant typeKeywords
   template.typeKeywords = setDiscussableKeyword(

packages/common/test/initiative-templates/edit.test.ts

@@ -171,7 +171,7 @@ describe("initiative template edit module:", () => {
       expect(chk.description).toBe("Some longer description");
       expect(chk.typeKeywords).toEqual([
         "Hub Initiative Template",
-        "slug|dcdev-wat-blarg",
+        "slug|dcdev-wat-blarg-1",
         "cannotDiscuss",
       ]);
       expect(chk.location).toEqual({

packages/common/test/projects/edit.test.ts

@@ -178,7 +178,7 @@ describe("project edit module:", () => {
       expect(chk.description).toBe("Some longer description");
       expect(chk.typeKeywords).toEqual([
         "Hub Project",
-        "slug|dcdev-wat-blarg",
+        "slug|dcdev-wat-blarg-1",
         "status|inProgress",
         "cannotDiscuss",
       ]);

packages/common/test/sites/HubSites.test.ts

@@ -523,6 +523,7 @@ describe("HubSites:", () => {
     let uniqueDomainSpy: jasmine.Spy;
     let createModelSpy: jasmine.Spy;
     let updateModelSpy: jasmine.Spy;
+    let ensureUniqueEntitySlugSpy: jasmine.Spy;
 
     let addDomainsSpy: jasmine.Spy;
     beforeEach(() => {
@@ -547,6 +548,12 @@ describe("HubSites:", () => {
         const newModel = commonModule.cloneObject(m);
         return Promise.resolve(newModel);
       });
+      ensureUniqueEntitySlugSpy = spyOn(
+        require("../../src/items/_internal/slugs"),
+        "ensureUniqueEntitySlug"
+      ).and.callFake((site: commonModule.IHubSite) => {
+        return Promise.resolve(site);
+      });
     });
     describe("online: ", () => {
       beforeEach(() => {
@@ -563,6 +570,7 @@ describe("HubSites:", () => {
 
         const chk = await commonModule.createSite(sparseSite, MOCK_HUB_REQOPTS);
 
+        expect(ensureUniqueEntitySlugSpy.calls.count()).toBe(1);
         expect(uniqueDomainSpy.calls.count()).toBe(1);
         expect(createModelSpy.calls.count()).toBe(1);
         expect(updateModelSpy.calls.count()).toBe(1);
@@ -621,6 +629,7 @@ describe("HubSites:", () => {
 
         const chk = await commonModule.createSite(site, MOCK_HUB_REQOPTS);
 
+        expect(ensureUniqueEntitySlugSpy.calls.count()).toBe(1);
         expect(uniqueDomainSpy.calls.count()).toBe(1);
         expect(createModelSpy.calls.count()).toBe(1);
         expect(updateModelSpy.calls.count()).toBe(1);
@@ -649,6 +658,7 @@ describe("HubSites:", () => {
           MOCK_ENTERPRISE_REQOPTS
         );
 
+        expect(ensureUniqueEntitySlugSpy.calls.count()).toBe(1);
         expect(uniqueDomainSpy.calls.count()).toBe(1);
         expect(createModelSpy.calls.count()).toBe(1);
         expect(updateModelSpy.calls.count()).toBe(1);