From fdc017f04c1a1de2f3ce0ed0921d7c2f4bf186ca Mon Sep 17 00:00:00 2001 From: Tom Wayson Date: Tue, 22 Oct 2024 08:07:00 -0700 Subject: [PATCH] fix(hub-common): ensure entity slug is unique and typekeyword is in sync (#1693) --- packages/common/src/discussions/edit.ts | 22 +++++----------- .../common/src/initiative-templates/edit.ts | 17 ++++++------- .../common/src/initiatives/HubInitiatives.ts | 25 ++++--------------- packages/common/src/items/_internal/slugs.ts | 17 +++++++++++++ packages/common/src/pages/HubPages.ts | 19 ++++---------- packages/common/src/projects/edit.ts | 13 ++++------ packages/common/src/sites/HubSites.ts | 13 ++++------ packages/common/src/templates/edit.ts | 6 ++--- .../test/initiative-templates/edit.test.ts | 2 +- packages/common/test/projects/edit.test.ts | 2 +- packages/common/test/sites/HubSites.test.ts | 10 ++++++++ 11 files changed, 64 insertions(+), 82 deletions(-) create mode 100644 packages/common/src/items/_internal/slugs.ts diff --git a/packages/common/src/discussions/edit.ts b/packages/common/src/discussions/edit.ts index 6b64c91b08a..50e03241e79 100644 --- a/packages/common/src/discussions/edit.ts +++ b/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 { // 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 diff --git a/packages/common/src/initiative-templates/edit.ts b/packages/common/src/initiative-templates/edit.ts index 0e1d48280ce..61afa6139eb 100644 --- a/packages/common/src/initiative-templates/edit.ts +++ b/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 { // 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 diff --git a/packages/common/src/initiatives/HubInitiatives.ts b/packages/common/src/initiatives/HubInitiatives.ts index 552f837f131..51ebf7afb24 100644 --- a/packages/common/src/initiatives/HubInitiatives.ts +++ b/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 { // 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, diff --git a/packages/common/src/items/_internal/slugs.ts b/packages/common/src/items/_internal/slugs.ts new file mode 100644 index 00000000000..9a84be77489 --- /dev/null +++ b/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 => { + // 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; +}; diff --git a/packages/common/src/pages/HubPages.ts b/packages/common/src/pages/HubPages.ts index d80cd5bf494..9c11929505e 100644 --- a/packages/common/src/pages/HubPages.ts +++ b/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, IModel>( @@ -86,10 +80,7 @@ export async function updatePage( requestOptions: IUserRequestOptions ): Promise { // 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); diff --git a/packages/common/src/projects/edit.ts b/packages/common/src/projects/edit.ts index ed97a84609a..6e3a124e373 100644 --- a/packages/common/src/projects/edit.ts +++ b/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 { // 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, diff --git a/packages/common/src/sites/HubSites.ts b/packages/common/src/sites/HubSites.ts index d0e926da6d5..e558ac56a1b 100644 --- a/packages/common/src/sites/HubSites.ts +++ b/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 { // 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 diff --git a/packages/common/src/templates/edit.ts b/packages/common/src/templates/edit.ts index 59f147fea81..ed1ab7d517c 100644 --- a/packages/common/src/templates/edit.ts +++ b/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 { // 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( diff --git a/packages/common/test/initiative-templates/edit.test.ts b/packages/common/test/initiative-templates/edit.test.ts index d279897a151..f8440d81137 100644 --- a/packages/common/test/initiative-templates/edit.test.ts +++ b/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({ diff --git a/packages/common/test/projects/edit.test.ts b/packages/common/test/projects/edit.test.ts index d95cbfe394a..31bd0fcc800 100644 --- a/packages/common/test/projects/edit.test.ts +++ b/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", ]); diff --git a/packages/common/test/sites/HubSites.test.ts b/packages/common/test/sites/HubSites.test.ts index 4c1365f29b9..0f647c93874 100644 --- a/packages/common/test/sites/HubSites.test.ts +++ b/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);