From 7243db889c33d318e03471045539dd2988cd910a Mon Sep 17 00:00:00 2001 From: Dave Bouwman Date: Thu, 8 Jun 2023 16:33:57 -0600 Subject: [PATCH] fix: address issue creating sites on portal (#1085) * fix: address issue creating sites on portal * docs: clean up comments --- packages/common/src/sites/HubSites.ts | 5 +- .../src/sites/domains/addSiteDomains.ts | 6 +- packages/common/test/sites/HubSites.test.ts | 203 +++++++++--------- .../test/sites/domains/addSiteDomains.test.ts | 7 +- packages/sites/src/create-site.ts | 3 +- 5 files changed, 121 insertions(+), 103 deletions(-) diff --git a/packages/common/src/sites/HubSites.ts b/packages/common/src/sites/HubSites.ts index 0f630c0974f..79c8c1e3637 100644 --- a/packages/common/src/sites/HubSites.ts +++ b/packages/common/src/sites/HubSites.ts @@ -273,14 +273,13 @@ export async function createSite( ); // Register as an app - // const registration = await registerSiteAsApplication(model, requestOptions); - // model.data.values.clientId = registration.client_id; // NOTE: Site registration needs to happen via the hub api domain api calls // See https://devtopia.esri.com/dc/hub/issues/6390 for info // Register domain and at the same time register the site as an application + // for portal, this will return a single entry with just the clientKey const domainResponses = await addSiteDomains(model, requestOptions); - model.data.values.clientId = domainResponses[0].client_id; + model.data.values.clientId = domainResponses[0].clientKey; // update the model const updatedModel = await updateModel( diff --git a/packages/common/src/sites/domains/addSiteDomains.ts b/packages/common/src/sites/domains/addSiteDomains.ts index 86f74540251..d35fff91fe2 100644 --- a/packages/common/src/sites/domains/addSiteDomains.ts +++ b/packages/common/src/sites/domains/addSiteDomains.ts @@ -5,6 +5,8 @@ import { addDomain } from "./add-domain"; /** * Given a Site Model, register the domains with the Domain Service. * + * For Portal, this will return a sparse entry that contains just the portal clientKey + * * This should only be used when creating a site. To update domains related * to a site, use the `addDomain` and `removeDomain` functions directly * @@ -16,7 +18,9 @@ export function addSiteDomains( hubRequestOptions: IHubRequestOptions ): Promise { if (hubRequestOptions.isPortal) { - return Promise.resolve([]); + // For enterprise we don't register the domain, but we do return a sparse entry + // that contains just the portal clientKey so that the caller can use it. + return Promise.resolve([{ clientKey: "arcgisonline" }]); } else { const props = ["defaultHostname", "customHostname"]; return Promise.all( diff --git a/packages/common/test/sites/HubSites.test.ts b/packages/common/test/sites/HubSites.test.ts index 120e38879a9..67b2cf05b18 100644 --- a/packages/common/test/sites/HubSites.test.ts +++ b/packages/common/test/sites/HubSites.test.ts @@ -348,114 +348,125 @@ describe("HubSites:", () => { const newModel = commonModule.cloneObject(m); return Promise.resolve(newModel); }); - - addDomainsSpy = spyOn( - require("../../src/sites/domains/addSiteDomains"), - "addSiteDomains" - ).and.returnValue(Promise.resolve([{ clientKey: "FAKE_CLIENT_KEY" }])); }); - it("works with a sparse IHubSite", async () => { - const sparseSite: Partial = { - name: "my site", - orgUrlKey: "dcdev", - }; - - const chk = await commonModule.createSite(sparseSite, MOCK_HUB_REQOPTS); - - expect(uniqueDomainSpy.calls.count()).toBe(1); - expect(createModelSpy.calls.count()).toBe(1); - expect(updateModelSpy.calls.count()).toBe(1); - - expect(addDomainsSpy.calls.count()).toBe(1); - - const modelToCreate = createModelSpy.calls.argsFor(0)[0]; - expect(modelToCreate.item.title).toBe("my site"); - expect(modelToCreate.item.type).toBe("Hub Site Application"); - expect(modelToCreate.item.properties.slug).toBe("dcdev|my-site"); - expect(modelToCreate.item.properties.orgUrlKey).toBe("org"); - - expect(chk.name).toBe("my site"); - expect(chk.url).toBe("https://my-site-org.hubqa.arcgis.com"); - }); - it("works with a full IHubSite", async () => { - const site: Partial = { - name: "Special Site", - slug: "CuStOm-Slug", - subdomain: "custom-subdomain", - customHostname: "site.myorg.com", - theme: { - fake: "theme", - }, - defaultExtent: { - xmax: 10, - ymax: 10, - xmin: 5, - ymin: 5, - spatialReference: { - wkid: 4326, + describe("online: ", () => { + beforeEach(() => { + addDomainsSpy = spyOn( + require("../../src/sites/domains/addSiteDomains"), + "addSiteDomains" + ).and.returnValue(Promise.resolve([{ clientKey: "FAKE_CLIENT_KEY" }])); + }); + it("works with a sparse IHubSite", async () => { + const sparseSite: Partial = { + name: "my site", + orgUrlKey: "dcdev", + }; + + const chk = await commonModule.createSite(sparseSite, MOCK_HUB_REQOPTS); + + expect(uniqueDomainSpy.calls.count()).toBe(1); + expect(createModelSpy.calls.count()).toBe(1); + expect(updateModelSpy.calls.count()).toBe(1); + + expect(addDomainsSpy.calls.count()).toBe(1); + + const modelToCreate = createModelSpy.calls.argsFor(0)[0]; + expect(modelToCreate.item.title).toBe("my site"); + expect(modelToCreate.item.type).toBe("Hub Site Application"); + expect(modelToCreate.item.properties.slug).toBe("dcdev|my-site"); + expect(modelToCreate.item.properties.orgUrlKey).toBe("org"); + const modelToUpdate = updateModelSpy.calls.argsFor(0)[0]; + expect(modelToUpdate.data.values.clientId).toBe("FAKE_CLIENT_KEY"); + + expect(chk.name).toBe("my site"); + expect(chk.url).toBe("https://my-site-org.hubqa.arcgis.com"); + }); + it("works with a full IHubSite", async () => { + const site: Partial = { + name: "Special Site", + slug: "CuStOm-Slug", + subdomain: "custom-subdomain", + customHostname: "site.myorg.com", + theme: { + fake: "theme", }, - }, - culture: "fr-ca", - map: { - basemaps: { - primary: { - fake: "basemap", + defaultExtent: { + xmax: 10, + ymax: 10, + xmin: 5, + ymin: 5, + spatialReference: { + wkid: 4326, }, }, - }, - orgUrlKey: "dcdev", - layout: { - sections: [{}], - header: { - component: { - settings: { - title: "Title that is already set", + culture: "fr-ca", + map: { + basemaps: { + primary: { + fake: "basemap", }, }, }, - }, - }; + orgUrlKey: "dcdev", + layout: { + sections: [{}], + header: { + component: { + settings: { + title: "Title that is already set", + }, + }, + }, + }, + }; - const chk = await commonModule.createSite(site, MOCK_HUB_REQOPTS); + const chk = await commonModule.createSite(site, MOCK_HUB_REQOPTS); - expect(uniqueDomainSpy.calls.count()).toBe(1); - expect(createModelSpy.calls.count()).toBe(1); - expect(updateModelSpy.calls.count()).toBe(1); + expect(uniqueDomainSpy.calls.count()).toBe(1); + expect(createModelSpy.calls.count()).toBe(1); + expect(updateModelSpy.calls.count()).toBe(1); - expect(addDomainsSpy.calls.count()).toBe(1); + expect(addDomainsSpy.calls.count()).toBe(1); - expect(chk.name).toBe("Special Site"); - expect(chk.url).toBe("https://site.myorg.com"); - expect(chk.culture).toBe("fr-ca"); - expect(chk.theme).toEqual({ fake: "theme" }); - expect(chk.defaultExtent.xmax).toBe(10); - expect(chk.map.basemaps.primary).toEqual({ fake: "basemap" }); + const modelToUpdate = updateModelSpy.calls.argsFor(0)[0]; + expect(modelToUpdate.data.values.clientId).toBe("FAKE_CLIENT_KEY"); + expect(chk.name).toBe("Special Site"); + expect(chk.url).toBe("https://site.myorg.com"); + expect(chk.culture).toBe("fr-ca"); + expect(chk.theme).toEqual({ fake: "theme" }); + expect(chk.defaultExtent.xmax).toBe(10); + expect(chk.map.basemaps.primary).toEqual({ fake: "basemap" }); + }); }); - it("works in portal", async () => { - const sparseSite: Partial = { - name: "my site", - orgUrlKey: "dcdev", - }; - - const chk = await commonModule.createSite( - sparseSite, - MOCK_ENTERPRISE_REQOPTS - ); - - expect(uniqueDomainSpy.calls.count()).toBe(1); - expect(createModelSpy.calls.count()).toBe(1); - expect(updateModelSpy.calls.count()).toBe(1); - - expect(addDomainsSpy.calls.count()).toBe(1); - - const modelToCreate = createModelSpy.calls.argsFor(0)[0]; - expect(modelToCreate.item.title).toBe("my site"); - expect(modelToCreate.item.type).toBe("Site Application"); - expect(modelToCreate.item.properties.slug).toBe("dcdev|my-site"); - expect(modelToCreate.item.properties.orgUrlKey).toBe("org"); - expect(chk.url).toBe("https://my-server.com/portal/apps/sites/#/my-site"); - expect(chk.typeKeywords).toContain(`hubsubdomain|${chk.subdomain}`); - expect(chk.subdomain).toBe(`my-site`); + describe("portal:", () => { + it("works in portal", async () => { + const sparseSite: Partial = { + name: "my site", + orgUrlKey: "dcdev", + }; + + const chk = await commonModule.createSite( + sparseSite, + MOCK_ENTERPRISE_REQOPTS + ); + + expect(uniqueDomainSpy.calls.count()).toBe(1); + expect(createModelSpy.calls.count()).toBe(1); + expect(updateModelSpy.calls.count()).toBe(1); + + const modelToCreate = createModelSpy.calls.argsFor(0)[0]; + expect(modelToCreate.item.title).toBe("my site"); + expect(modelToCreate.item.type).toBe("Site Application"); + expect(modelToCreate.item.properties.slug).toBe("dcdev|my-site"); + expect(modelToCreate.item.properties.orgUrlKey).toBe("org"); + const modelToUpdate = updateModelSpy.calls.argsFor(0)[0]; + expect(modelToUpdate.data.values.clientId).toBe("arcgisonline"); + expect(chk.url).toBe( + "https://my-server.com/portal/apps/sites/#/my-site" + ); + expect(chk.typeKeywords).toContain(`hubsubdomain|${chk.subdomain}`); + expect(chk.subdomain).toBe(`my-site`); + }); }); }); describe("enrichments:", () => { diff --git a/packages/common/test/sites/domains/addSiteDomains.test.ts b/packages/common/test/sites/domains/addSiteDomains.test.ts index 61531b35dcf..c4c175aaf87 100644 --- a/packages/common/test/sites/domains/addSiteDomains.test.ts +++ b/packages/common/test/sites/domains/addSiteDomains.test.ts @@ -75,7 +75,12 @@ describe("addSiteDomains", () => { }, } as unknown as IHubRequestOptions; - await addSiteDomains(model, ro); + const result = await addSiteDomains(model, ro); + + expect(result[0].clientKey).toBe( + "arcgisonline", + "returns portal client key" + ); expect(addSpy.calls.count()).toBe(0); }); diff --git a/packages/sites/src/create-site.ts b/packages/sites/src/create-site.ts index 61574ba1a82..b9ee18f10a7 100644 --- a/packages/sites/src/create-site.ts +++ b/packages/sites/src/create-site.ts @@ -64,9 +64,8 @@ export function createSite( }) .then((domainResponses) => { // client id will be the same for all domain resonses so we can just grab the first one - // no guard clause because the only way we don't get a response is if addSiteDomains throws - // which would not hit this code model.data.values.clientId = domainResponses[0].clientKey; + // If we have a dcat section, hoist it out as it may contain complex adlib // templates that are needed at run-time // If we have data.values.dcatConfig, yank it off b/c that may have adlib template stuff in it