diff --git a/packages/common/src/discussions/_internal/DiscussionBusinessRules.ts b/packages/common/src/discussions/_internal/DiscussionBusinessRules.ts index 42f139388c5..f0e52da3ead 100644 --- a/packages/common/src/discussions/_internal/DiscussionBusinessRules.ts +++ b/packages/common/src/discussions/_internal/DiscussionBusinessRules.ts @@ -106,10 +106,6 @@ export const DiscussionPermissionPolicies: IPermissionPolicy[] = [ }, { permission: "hub:discussion:workspace:catalog:events", - dependencies: [ - "hub:discussion:workspace:catalog", - "hub:event", - "hub:feature:catalogs:edit:advanced", - ], + dependencies: ["hub:discussion:workspace:catalog", "hub:event"], }, ]; diff --git a/packages/common/src/events/_internal/EventBusinessRules.ts b/packages/common/src/events/_internal/EventBusinessRules.ts index 98a4c69c6ca..57748ec8e90 100644 --- a/packages/common/src/events/_internal/EventBusinessRules.ts +++ b/packages/common/src/events/_internal/EventBusinessRules.ts @@ -105,11 +105,7 @@ export const EventPermissionPolicies: IPermissionPolicy[] = [ }, { permission: "hub:event:workspace:catalog:events", - dependencies: [ - "hub:event:workspace:catalog", - "hub:event", - "hub:feature:catalogs:edit:advanced", - ], + dependencies: ["hub:event:workspace:catalog", "hub:event"], }, { permission: "hub:event:manage", diff --git a/packages/common/src/initiatives/_internal/InitiativeBusinessRules.ts b/packages/common/src/initiatives/_internal/InitiativeBusinessRules.ts index cf127959bbc..9b941883af5 100644 --- a/packages/common/src/initiatives/_internal/InitiativeBusinessRules.ts +++ b/packages/common/src/initiatives/_internal/InitiativeBusinessRules.ts @@ -165,11 +165,7 @@ export const InitiativePermissionPolicies: IPermissionPolicy[] = [ }, { permission: "hub:initiative:workspace:catalog:events", - dependencies: [ - "hub:initiative:workspace:catalog", - "hub:event", - "hub:feature:catalogs:edit:advanced", - ], + dependencies: ["hub:initiative:workspace:catalog", "hub:event"], }, { permission: "hub:initiative:manage", diff --git a/packages/common/src/permissions/HubPermissionPolicies.ts b/packages/common/src/permissions/HubPermissionPolicies.ts index bd8e18c5e0f..c2a15247f14 100644 --- a/packages/common/src/permissions/HubPermissionPolicies.ts +++ b/packages/common/src/permissions/HubPermissionPolicies.ts @@ -216,39 +216,18 @@ const SystemPermissionPolicies: IPermissionPolicy[] = [ environments: ["qaext"], availability: ["alpha"], }, - /** - * Gates advanced editing (e.g. adding new collections, adding - * additional scope filters, etc.) in the catalog configuration - * experince. - * - * TODO: Remove the site entity assertion once all catalog - * configuration features are supported by sites - */ - { - permission: "hub:feature:catalogs:edit:advanced", - entityEdit: true, - licenses: ["hub-premium"], - assertions: [ - { - property: "entity:type", - type: "neq", - value: "Hub Site Application", - }, - ], - }, /** * Gates catalog & collection appearance editing * in the catalog configuration experience. * * TODO: remove environment & availability gating once * we are ready to release catalog appearance - * configuration. Alternatively, we can remove it entirely - * in favor of the "hub:feature:caatalogs:edit:advanced" - * permission + * configuration. */ { permission: "hub:feature:catalogs:edit:appearance", - dependencies: ["hub:feature:catalogs:edit:advanced"], + entityEdit: true, + licenses: ["hub-premium"], environments: ["qaext"], availability: ["alpha"], }, diff --git a/packages/common/src/permissions/_internal/constants.ts b/packages/common/src/permissions/_internal/constants.ts index 68be489ee48..566ef3b3a2b 100644 --- a/packages/common/src/permissions/_internal/constants.ts +++ b/packages/common/src/permissions/_internal/constants.ts @@ -31,8 +31,6 @@ export const SystemPermissions = [ "hub:feature:newentityview", "hub:feature:history", "hub:feature:catalogs", // DEPRECATED - TODO: remove at next major version - /** remove once sites support all catalog configuration features */ - "hub:feature:catalogs:edit:advanced", "hub:feature:catalogs:edit:appearance", "hub:feature:gallery-card:enterprise", "hub:feature:inline-workspace", diff --git a/packages/common/src/projects/_internal/ProjectBusinessRules.ts b/packages/common/src/projects/_internal/ProjectBusinessRules.ts index 66358b5593a..ae12f69636c 100644 --- a/packages/common/src/projects/_internal/ProjectBusinessRules.ts +++ b/packages/common/src/projects/_internal/ProjectBusinessRules.ts @@ -136,11 +136,7 @@ export const ProjectPermissionPolicies: IPermissionPolicy[] = [ }, { permission: "hub:project:workspace:catalog:events", - dependencies: [ - "hub:project:workspace:catalog", - "hub:event", - "hub:feature:catalogs:edit:advanced", - ], + dependencies: ["hub:project:workspace:catalog", "hub:event"], }, { permission: "hub:project:manage", diff --git a/packages/common/src/sites/HubSites.ts b/packages/common/src/sites/HubSites.ts index 8b434a7c287..d2405a1dfc6 100644 --- a/packages/common/src/sites/HubSites.ts +++ b/packages/common/src/sites/HubSites.ts @@ -24,10 +24,7 @@ import { IHubRequestOptions, IModel } from "../hub-types"; import { removeDomainsBySiteId } from "./domains/remove-domains-by-site-id"; import { IHubSearchResult } from "../search/types/IHubSearchResult"; import { reflectCollectionsToSearchCategories } from "./_internal/reflectCollectionsToSearchCategories"; -import { - catalogToLegacy, - convertCatalogToLegacyFormat, -} from "./_internal/convertCatalogToLegacyFormat"; +import { convertCatalogToLegacyFormat } from "./_internal/convertCatalogToLegacyFormat"; import { convertFeaturesToLegacyCapabilities } from "./_internal/capabilities/convertFeaturesToLegacyCapabilities"; import { computeLinks } from "./_internal/computeLinks"; import { handleSubdomainChange } from "./_internal/subdomains"; @@ -126,9 +123,7 @@ const DEFAULT_SITE_MODEL: IModel = { url: "", } as IItem, data: { - catalog: { - groups: [], - }, + catalogV2: { schemaVersion: 0 }, feeds: {}, values: { title: "", @@ -281,15 +276,12 @@ export async function createSite( ); let model = mapper.entityToStore(site, cloneObject(DEFAULT_SITE_MODEL)); - // At this point `data.catalog` may have become a full IHubCatalog object due to an in-memory - // migration. However, we can't persist an IHubCatalog in `data.catalog` without breaking - // the application, since most of the app relies on the old catalog structure. As such, - // we convert any changes made to the catalog scope into the old format and merge the changes - // with the existing structure on the most current model. - // TODO: Remove once the application is plumbed to work off an IHubCatalog - if (getProp(model, "data.catalog.scopes")) { - model.data.catalog = catalogToLegacy(model.data.catalog); - } + // Remove default collections and set the v2 catalog + const catalogV2 = cloneObject(site.catalog); + catalogV2.collections = []; + model.data.catalogV2 = catalogV2; + // TODO: Remove once all sites are fully on catalogV2 + model.data.useCatalogV2 = true; // create the backing item model = await createModel( @@ -313,7 +305,7 @@ export async function createSite( ); // convert the model into a IHubSite and return - return mapper.storeToEntity(updatedModel, {}) as IHubSite; + return convertModelToSite(updatedModel, requestOptions); } /** @@ -376,20 +368,30 @@ export async function updateSite( await handleDomainChanges(modelToUpdate, currentModel, requestOptions); } - // Persist the new catalog to new property until the app is fully migrated + // Persist the v2 catalog modelToUpdate.data.catalogV2 = site.catalog; - // Because some old (but critical) application code still uses `data.values.searchCategories` - // as the source of truth for collection display configuration, we port all display changes - // in `data.catalog.collections` to the search category format. - // TODO: Remove once the app no longer relies on `data.values.searchCategories` - modelToUpdate = reflectCollectionsToSearchCategories(modelToUpdate); - // At this point `data.catalog` has become a full IHubCatalog object due to an in-memory - // migration. However, we can't persist an IHubCatalog in `data.catalog` without breaking - // the application, since most of the app relies on the old catalog structure. As such, - // we convert any changes made to the catalog scope into the old format and merge the changes - // with the existing structure on the most current model. - // TODO: Remove once the application is plumbed to work off an IHubCatalog - modelToUpdate = convertCatalogToLegacyFormat(modelToUpdate, currentModel); + + // Perform backwards compatibility steps if the site is still using the v1 catalog + if (site.isCatalogV1Enabled) { + // Because some old (but critical) application code still uses `data.values.searchCategories` + // as the source of truth for collection display configuration, we port all display changes + // in `data.catalog.collections` to the search category format. + // TODO: Remove once the app no longer relies on `data.values.searchCategories` + modelToUpdate = reflectCollectionsToSearchCategories(modelToUpdate); + // We can't persist an IHubCatalog in `data.catalog` without breaking + // the application, since most of the app relies on the old catalog structure. As such, + // we convert any changes made to the catalog scope into the old format and merge the changes + // with the existing structure on the most current model. + // TODO: Remove once the application is fully migrated to always use IHubCatalog + modelToUpdate = convertCatalogToLegacyFormat(modelToUpdate, currentModel); + } else { + // ensure the old catalog is removed if the site is not using v1 + delete modelToUpdate.data.catalog; + delete (modelToUpdate.data.values as Record) + .searchCategories; + // set the temporary property to indicate the site is fully upgraded to catalogV2 + modelToUpdate.data.useCatalogV2 = true; + } /** * Site capabilities are currently saved as an array on the * site.data.values.capabilities. We want to migragte these diff --git a/packages/common/src/sites/_internal/SiteBusinessRules.ts b/packages/common/src/sites/_internal/SiteBusinessRules.ts index 96a26e0705c..89c2bdccf68 100644 --- a/packages/common/src/sites/_internal/SiteBusinessRules.ts +++ b/packages/common/src/sites/_internal/SiteBusinessRules.ts @@ -139,7 +139,13 @@ export const SitesPermissionPolicies: IPermissionPolicy[] = [ { permission: "hub:site:workspace:catalog:upgrade", dependencies: ["hub:site:workspace:catalog"], - availability: ["flag"], + assertions: [ + { + property: "entity:isCatalogV1Enabled", + type: "eq", + value: true, + }, + ], }, { permission: "hub:site:workspace:pages", diff --git a/packages/common/src/sites/_internal/getPropertyMap.ts b/packages/common/src/sites/_internal/getPropertyMap.ts index ead1030b783..b7a3d6dac1d 100644 --- a/packages/common/src/sites/_internal/getPropertyMap.ts +++ b/packages/common/src/sites/_internal/getPropertyMap.ts @@ -64,7 +64,6 @@ export function getPropertyMap(): IPropertyMap[] { entityKey: "isHubHome", storeKey: "item.properties.isHubHome", }); - map.push({ entityKey: "catalog", storeKey: "data.catalog" }); map.push({ entityKey: "features", diff --git a/packages/common/src/sites/_internal/removeCatalogV1FromUpgradedSite.ts b/packages/common/src/sites/_internal/removeCatalogV1FromUpgradedSite.ts new file mode 100644 index 00000000000..26d43736e3b --- /dev/null +++ b/packages/common/src/sites/_internal/removeCatalogV1FromUpgradedSite.ts @@ -0,0 +1,23 @@ +import { IModel } from "../../hub-types"; +import { cloneObject } from "../../util"; + +/** + * Removes the `catalog` property from upgraded site models. We use site.data.useCatalogV2 + * (a temporary property) to determine if the upgrade has been completed. + * + * We need this temporary migration because earlier versions of the site schema upgrade + * force data.catalog to be present, even if the site is using catalogV2. + * + * TODO: Remove this function once we force all sites to use catalogV2. + * @param model + * @returns + */ +export function removeCatalogV1FromUpgradedSite(model: IModel): IModel { + const clone = cloneObject(model); + + if (clone.data.useCatalogV2) { + delete clone.data.catalog; + } + + return clone; +} diff --git a/packages/common/src/sites/defaults.ts b/packages/common/src/sites/defaults.ts index b4f414e296d..8e78f584f51 100644 --- a/packages/common/src/sites/defaults.ts +++ b/packages/common/src/sites/defaults.ts @@ -66,7 +66,7 @@ export const DEFAULT_SITE_MODEL: IModel = { }, }, data: { - catalog: { schemaVersion: 0 }, + catalogV2: { schemaVersion: 0 }, layout: {}, }, } as unknown as IModel; diff --git a/packages/common/src/sites/upgrade-site-schema.ts b/packages/common/src/sites/upgrade-site-schema.ts index 139981bcc64..ac5228ff92c 100644 --- a/packages/common/src/sites/upgrade-site-schema.ts +++ b/packages/common/src/sites/upgrade-site-schema.ts @@ -16,6 +16,7 @@ import { migrateWebMappingApplicationSites } from "./_internal/migrateWebMapping import { _migrateLinkUnderlinesCapability } from "./_internal/_migrate-link-underlines-capability"; import { _migrateToV2Catalog } from "./_internal/_migrate-to-v2-catalog"; import { ensureLowercaseOrgUrlKeySlugAndKeyword } from "./_internal/ensureLowercaseOrgUrlKeySlugAndKeyword"; +import { removeCatalogV1FromUpgradedSite } from "./_internal/removeCatalogV1FromUpgradedSite"; /** * Upgrades the schema upgrades @@ -47,6 +48,8 @@ export function upgradeSiteSchema(model: IModel): IModel { model = migrateWebMappingApplicationSites(model); model = _migrateLinkUnderlinesCapability(model); model = ensureLowercaseOrgUrlKeySlugAndKeyword(model); + // TODO: Remove this once all sites are fully on catalogV2 + model = removeCatalogV1FromUpgradedSite(model); return model; } diff --git a/packages/common/test/sites/HubSites.test.ts b/packages/common/test/sites/HubSites.test.ts index b57206f3eb4..cd6d490d0e3 100644 --- a/packages/common/test/sites/HubSites.test.ts +++ b/packages/common/test/sites/HubSites.test.ts @@ -58,6 +58,7 @@ const SITE_DATA = { primary: {}, }, }, + searchCategories: [{ key: "components.search.category_tabs.data" }], }, }; const SITE_MODEL = { @@ -486,8 +487,9 @@ describe("HubSites:", () => { const modelToUpdate = updateModelSpy.calls.argsFor(0)[0]; expect(modelToUpdate.item.title).toBe(updatedSite.name); }); - it("reflects collection changes to searchCategories", async () => { + it("if isCatalogV1Enabled, reflects collection changes to searchCategories", async () => { const updatedSite = cloneObject(SITE); + updatedSite.isCatalogV1Enabled = true; updatedSite.catalog.collections = [ { label: "My Datasets", @@ -518,8 +520,9 @@ describe("HubSites:", () => { }, ]); }); - it("converts catalog group changes to the old catalog format and stores the new catalog in data.catalogv2", async () => { + it("stores the new catalog in data.catalogv2 and (if isCatalogV1Enabled) converts catalog group changes to the old catalog format ", async () => { const updatedSite = cloneObject(SITE); + updatedSite.isCatalogV1Enabled = true; updatedSite.catalog.scopes.item.filters = [ { predicates: [ @@ -547,6 +550,39 @@ describe("HubSites:", () => { expect(modelToUpdate.data.catalog).toEqual({ groups: ["9001"] }); expect(modelToUpdate.data.catalogV2).toEqual(expectedCatalogV2); }); + + it("deletes old catalog and search categories if upgraded to the v2 catalog", async () => { + const updatedSite = cloneObject(SITE); + updatedSite.isCatalogV1Enabled = false; + updatedSite.catalog.scopes.item.filters = [ + { + predicates: [ + { + group: ["9001"], + }, + ], + }, + ]; + updatedSite.catalog.scopes.event.filters = [ + { + predicates: [ + { + group: ["1006"], + }, + ], + }, + ]; + const expectedCatalogV2 = cloneObject(updatedSite.catalog); + const chk = await updateSite(updatedSite, MOCK_HUB_REQOPTS); + + expect(chk.id).toBe(GUID); + expect(fetchSiteModelSpy).toHaveBeenCalledTimes(1); + const modelToUpdate = updateModelSpy.calls.argsFor(0)[0]; + expect(modelToUpdate.data.catalog).toBeUndefined(); + expect(modelToUpdate.data.values.searchCategories).toBeUndefined(); + expect(modelToUpdate.data.useCatalogV2).toBe(true); + expect(modelToUpdate.data.catalogV2).toEqual(expectedCatalogV2); + }); }); describe("createSite:", () => { let uniqueDomainSpy: jasmine.Spy; @@ -718,7 +754,10 @@ describe("HubSites:", () => { const modelToUpdate = updateModelSpy.calls.argsFor(0)[0]; expect(modelToUpdate.data.values.clientId).toBe("FAKE_CLIENT_KEY"); - expect(modelToUpdate.data.catalog.groups).toContain("9001"); + expect(modelToUpdate.data.useCatalogV2).toBe(true); + expect(modelToUpdate.data.catalogV2).toBeDefined(); + expect(modelToUpdate.data.catalogV2.collections.length).toBe(0); + expect(modelToUpdate.data.catalog).toBeUndefined(); expect(chk.name).toBe("Special Site"); expect(chk.url).toBe("https://site.myorg.com"); expect(chk.culture).toBe("fr-ca"); diff --git a/packages/common/test/sites/_internal/removeCatalogV1FromUpgradedSite.test.ts b/packages/common/test/sites/_internal/removeCatalogV1FromUpgradedSite.test.ts new file mode 100644 index 00000000000..24277adb9c9 --- /dev/null +++ b/packages/common/test/sites/_internal/removeCatalogV1FromUpgradedSite.test.ts @@ -0,0 +1,43 @@ +import { IModel } from "../../../src"; +import { removeCatalogV1FromUpgradedSite } from "../../../src/sites/_internal/removeCatalogV1FromUpgradedSite"; + +describe("removeCatalogV1FromUpgradedSite", () => { + it("removes catalog property if useCatalogV2 is true", () => { + const model = { + data: { + useCatalogV2: true, + catalog: { foo: "bar" }, + other: "value", + }, + }; + const result = removeCatalogV1FromUpgradedSite(model as any); + expect(result.data.catalog).toBeUndefined(); + expect(result.data.useCatalogV2).toBe(true); + expect(result.data.other).toBe("value"); + }); + + it("does not remove catalog property if useCatalogV2 is false", () => { + const model = { + data: { + useCatalogV2: false, + catalog: { foo: "bar" }, + }, + }; + const result = removeCatalogV1FromUpgradedSite(model as any); + expect(result.data.catalog).toEqual({ foo: "bar" }); + expect(result.data.useCatalogV2).toBe(false); + }); + + it("returns a clone, does not mutate original", () => { + const model = { + data: { + useCatalogV2: true, + catalog: { foo: "bar" }, + }, + } as unknown as IModel; + const original = JSON.parse(JSON.stringify(model)); + const result = removeCatalogV1FromUpgradedSite(model as any); + expect(model).toEqual(original); + expect(result).not.toBe(model); + }); +}); diff --git a/packages/common/test/sites/upgrade-site-schema.test.ts b/packages/common/test/sites/upgrade-site-schema.test.ts index b64f3bb5590..ca5100ebde8 100644 --- a/packages/common/test/sites/upgrade-site-schema.test.ts +++ b/packages/common/test/sites/upgrade-site-schema.test.ts @@ -12,6 +12,7 @@ import * as migrateBadBasemapModule from "../../src/sites/_internal/migrateBadBa import * as ensureBaseTelemetry from "../../src/sites/_internal/ensureBaseTelemetry"; import * as _migrateToV2CatalogModule from "../../src/sites/_internal/_migrate-to-v2-catalog"; import * as ensureLowercaseOrgUrlKeySlugAndKeywordModule from "../../src/sites/_internal/ensureLowercaseOrgUrlKeySlugAndKeyword"; +import * as removeCatalogV1FromUpgradedSite from "../../src/sites/_internal/removeCatalogV1FromUpgradedSite"; import { SITE_SCHEMA_VERSION } from "../../src/sites/site-schema-version"; import { expectAllCalled, expectAll } from "./test-helpers.test"; import { IModel } from "../../src/hub-types"; @@ -32,6 +33,7 @@ describe("upgradeSiteSchema", () => { let ensureBaseTelemetrySpy: jasmine.Spy; let migrateToV2CatalogSpy: jasmine.Spy; let slugFixSpy: jasmine.Spy; + let removeCatalogV1FromUpgradedSiteSpy: jasmine.Spy; beforeEach(() => { applySpy = spyOn(_applySiteSchemaModule, "_applySiteSchema").and.callFake( @@ -90,6 +92,10 @@ describe("upgradeSiteSchema", () => { ensureLowercaseOrgUrlKeySlugAndKeywordModule, "ensureLowercaseOrgUrlKeySlugAndKeyword" ).and.callFake((model: IModel) => model); + removeCatalogV1FromUpgradedSiteSpy = spyOn( + removeCatalogV1FromUpgradedSite, + "removeCatalogV1FromUpgradedSite" + ).and.callFake((model: IModel) => model); }); it("runs schema upgrades", async () => { @@ -119,6 +125,7 @@ describe("upgradeSiteSchema", () => { migrateLinkUnderlinesCapabilitySpy, migrateToV2CatalogSpy, slugFixSpy, + removeCatalogV1FromUpgradedSiteSpy, ], expect ); @@ -155,7 +162,11 @@ describe("upgradeSiteSchema", () => { expectAll([migrateBadBasemapSpy], "toHaveBeenCalled", true, expect); expectAll([ensureBaseTelemetrySpy], "toHaveBeenCalled", true, expect); expectAll( - [migrateLinkUnderlinesCapabilitySpy, slugFixSpy], + [ + migrateLinkUnderlinesCapabilitySpy, + slugFixSpy, + removeCatalogV1FromUpgradedSiteSpy, + ], "toHaveBeenCalled", true, expect