From 3a13434ee28e1c524ea0213ac30ff83cf6b5f079 Mon Sep 17 00:00:00 2001 From: Aaron Putterman <60486980+abp6318@users.noreply.github.com> Date: Mon, 14 Aug 2023 11:42:50 -0400 Subject: [PATCH] fix: user selects none; location disappears correctly (#1155) * feat: dynamic location picker for entities * feat: added translation, corrected some params * feat: moving changes to a separate PR * feat: updated tests * fix: update package lock * fix: get location items more flexible * fix: suggested change from tom * fix: updated tests, added to get type from entity * fix: made edit test more meaningful * fix: improving if statement * fix: simplified logic w tom * fix: updated tests --- package-lock.json | 48 ++++++---- packages/common/src/content/edit.ts | 9 +- packages/common/src/core/getTypeFromEntity.ts | 3 + .../schemas/internal/getLocationOptions.ts | 69 +++++++++++++- packages/common/test/content/edit.test.ts | 48 ++++++++++ .../internal/getLocationOptions.test.ts | 95 ++++++++++++++++++- 6 files changed, 248 insertions(+), 24 deletions(-) diff --git a/package-lock.json b/package-lock.json index e9bc77a73a2..749280e0ec3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -22644,9 +22644,10 @@ }, "node_modules/cz-lerna-changelog/node_modules/npm/node_modules/lodash._baseindexof": { "version": "3.1.0", - "extraneous": true, + "dev": true, "inBundle": true, - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/cz-lerna-changelog/node_modules/npm/node_modules/lodash._baseuniq": { "version": "4.6.0", @@ -22661,21 +22662,24 @@ }, "node_modules/cz-lerna-changelog/node_modules/npm/node_modules/lodash._bindcallback": { "version": "3.0.1", - "extraneous": true, + "dev": true, "inBundle": true, - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/cz-lerna-changelog/node_modules/npm/node_modules/lodash._cacheindexof": { "version": "3.0.2", - "extraneous": true, + "dev": true, "inBundle": true, - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/cz-lerna-changelog/node_modules/npm/node_modules/lodash._createcache": { "version": "3.1.2", - "extraneous": true, + "dev": true, "inBundle": true, "license": "MIT", + "peer": true, "dependencies": { "lodash._getnative": "^3.0.0" } @@ -22689,9 +22693,10 @@ }, "node_modules/cz-lerna-changelog/node_modules/npm/node_modules/lodash._getnative": { "version": "3.9.1", - "extraneous": true, + "dev": true, "inBundle": true, - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/cz-lerna-changelog/node_modules/npm/node_modules/lodash._root": { "version": "3.0.1", @@ -22709,9 +22714,10 @@ }, "node_modules/cz-lerna-changelog/node_modules/npm/node_modules/lodash.restparam": { "version": "3.6.1", - "extraneous": true, + "dev": true, "inBundle": true, - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/cz-lerna-changelog/node_modules/npm/node_modules/lodash.union": { "version": "4.6.0", @@ -64966,7 +64972,7 @@ }, "packages/common": { "name": "@esri/hub-common", - "version": "13.33.1", + "version": "13.36.1", "license": "Apache-2.0", "dependencies": { "abab": "^2.0.5", @@ -83376,7 +83382,8 @@ "lodash._baseindexof": { "version": "3.1.0", "bundled": true, - "extraneous": true + "dev": true, + "peer": true }, "lodash._baseuniq": { "version": "4.6.0", @@ -83391,17 +83398,20 @@ "lodash._bindcallback": { "version": "3.0.1", "bundled": true, - "extraneous": true + "dev": true, + "peer": true }, "lodash._cacheindexof": { "version": "3.0.2", "bundled": true, - "extraneous": true + "dev": true, + "peer": true }, "lodash._createcache": { "version": "3.1.2", "bundled": true, - "extraneous": true, + "dev": true, + "peer": true, "requires": { "lodash._getnative": "^3.0.0" } @@ -83415,7 +83425,8 @@ "lodash._getnative": { "version": "3.9.1", "bundled": true, - "extraneous": true + "dev": true, + "peer": true }, "lodash._root": { "version": "3.0.1", @@ -83432,7 +83443,8 @@ "lodash.restparam": { "version": "3.6.1", "bundled": true, - "extraneous": true + "dev": true, + "peer": true }, "lodash.union": { "version": "4.6.0", diff --git a/packages/common/src/content/edit.ts b/packages/common/src/content/edit.ts index 55d580e7aa5..3b566ddabe5 100644 --- a/packages/common/src/content/edit.ts +++ b/packages/common/src/content/edit.ts @@ -5,7 +5,7 @@ import { getItem, removeItem, } from "@esri/arcgis-rest-portal"; -import { IHubContent, IHubContentEditor, IHubEditableContent } from "../core"; +import { IHubContentEditor, IHubEditableContent } from "../core"; // Note - we separate these imports so we can cleanly spy on things in tests import { @@ -20,6 +20,7 @@ import { getPropertyMap } from "./_internal/getPropertyMap"; import { cloneObject } from "../util"; import { IModel } from "../types"; import { computeProps } from "./_internal/computeProps"; +import { getProp } from "../objects/get-prop"; // TODO: move this to defaults? const DEFAULT_CONTENT_MODEL: IModel = { @@ -104,6 +105,12 @@ export async function updateContent( // we are not attempting to handle "concurrent edit" conflict resolution // but this is where we would apply that sort of logic const modelToUpdate = mapper.entityToStore(content, model); + + // prevent map from displaying when boundary is 'none' + const locationType = getProp(modelToUpdate, "item.properties.location.type"); + modelToUpdate.item.properties.boundary = + locationType === "none" ? "none" : "item"; + // TODO: if we have resources disconnect them from the model for now. // if (modelToUpdate.resources) { // resources = configureBaseResources( diff --git a/packages/common/src/core/getTypeFromEntity.ts b/packages/common/src/core/getTypeFromEntity.ts index 61a979f2f4f..04afe801f5b 100644 --- a/packages/common/src/core/getTypeFromEntity.ts +++ b/packages/common/src/core/getTypeFromEntity.ts @@ -35,6 +35,9 @@ export function getTypeFromEntity( case "Group": type = "group"; break; + case "Hub Content": + type = "content"; + break; default: // TODO: other families go here? feedback? solution? template? const contentFamilies = ["app", "content", "dataset", "document", "map"]; diff --git a/packages/common/src/core/schemas/internal/getLocationOptions.ts b/packages/common/src/core/schemas/internal/getLocationOptions.ts index 2d397876a26..b31c6322ffa 100644 --- a/packages/common/src/core/schemas/internal/getLocationOptions.ts +++ b/packages/common/src/core/schemas/internal/getLocationOptions.ts @@ -1,9 +1,21 @@ -import { extentToBBox, getGeographicOrgExtent } from "../../../extent"; +import { + bBoxToExtent, + extentToBBox, + getGeographicOrgExtent, + isBBox, +} from "../../../extent"; import { IHubRequestOptions } from "../../../types"; import { getTypeFromEntity } from "../../getTypeFromEntity"; import { ConfigurableEntity } from "./ConfigurableEntity"; import { IHubLocation, IHubLocationOption } from "../../types/IHubLocation"; +import { IExtent } from "@esri/arcgis-rest-types"; + +const getItemExtent = (itemExtent: number[][]): IExtent => { + return isBBox(itemExtent) + ? ({ ...bBoxToExtent(itemExtent), type: "extent" } as unknown as IExtent) + : undefined; +}; /** * Construct the dynamic location picker options with the entity's @@ -22,9 +34,58 @@ export async function getLocationOptions( portalName: string, hubRequestOptions: IHubRequestOptions ): Promise { - const defaultExtent = await getGeographicOrgExtent(hubRequestOptions); - const location: IHubLocation = entity.location; + const typeFromEntity = getTypeFromEntity(entity); + + switch (typeFromEntity) { + case "content": + return getContentLocationOptions(entity); + default: + return getDefaultLocationOptions(entity, portalName, hubRequestOptions); + } +} +// TODO: Refactor parameters, they are icky gross +// TODO: Maybe move these to outside of core and into respective entities + +export async function getContentLocationOptions( + entity: ConfigurableEntity +): Promise { + const defaultExtent: IExtent = getItemExtent(entity.extent); + const defaultExtentIsBBox = isBBox(entity.extent); + const boundary = entity.boundary; + const isNone = boundary === "none"; + return [ + { + label: "{{shared.fields.location.none:translate}}", + location: { type: "none" }, + selected: isNone, + }, + { + label: "{{shared.fields.location.itemExtent:translate}}", + entityType: "content", + selected: !isNone, + location: { + type: "custom", + // TODO: Add custom bbox option here + // TODO: Remove "Add another location?" notification that appears when selecting a location + extent: defaultExtentIsBBox ? extentToBBox(defaultExtent) : undefined, + spatialReference: defaultExtentIsBBox + ? defaultExtent.spatialReference + : undefined, + }, + }, + ] as IHubLocationOption[]; +} + +export async function getDefaultLocationOptions( + entity: ConfigurableEntity, + portalName: string, + hubRequestOptions: IHubRequestOptions +): Promise { + const defaultExtent: IExtent = await getGeographicOrgExtent( + hubRequestOptions + ); + const location: IHubLocation = entity.location; return ( [ { @@ -64,3 +125,5 @@ export async function getLocationOptions( return option; }); } + +// TODO: Add other location options here diff --git a/packages/common/test/content/edit.test.ts b/packages/common/test/content/edit.test.ts index 08458739afb..3fd7877df74 100644 --- a/packages/common/test/content/edit.test.ts +++ b/packages/common/test/content/edit.test.ts @@ -70,6 +70,7 @@ describe("content editing:", () => { schemaVersion: 1, canEdit: false, canDelete: false, + location: { type: "none" }, }; const chk = await updateContent(content, { authentication: MOCK_AUTH }); expect(chk.id).toBe(GUID); @@ -79,6 +80,53 @@ describe("content editing:", () => { expect(updateModelSpy.calls.count()).toBe(1); const modelToUpdate = updateModelSpy.calls.argsFor(0)[0]; expect(modelToUpdate.item.description).toBe(content.description); + expect(modelToUpdate.item.properties.boundary).toBe("none"); + }); + }); + describe("update content with location:", () => { + it("converts to a model and updates the item", async () => { + const getItemSpy = spyOn(portalModule, "getItem").and.returnValue( + Promise.resolve({ + item: { + typeKeywords: [], + }, + }) + ); + const updateModelSpy = spyOn(modelUtils, "updateModel").and.callFake( + (m: IModel) => { + return Promise.resolve(m); + } + ); + const content: IHubEditableContent = { + itemControl: "edit", + id: GUID, + name: "Hello World", + tags: ["Transportation"], + description: "Some longer description", + slug: "dcdev-wat-blarg", + orgUrlKey: "dcdev", + owner: "dcdev_dude", + type: "Hub Initiative", + createdDate: new Date(1595878748000), + createdDateSource: "item.created", + updatedDate: new Date(1595878750000), + updatedDateSource: "item.modified", + thumbnailUrl: "", + permissions: [], + schemaVersion: 1, + canEdit: false, + canDelete: false, + location: { type: "item" }, + }; + const chk = await updateContent(content, { authentication: MOCK_AUTH }); + expect(chk.id).toBe(GUID); + expect(chk.name).toBe("Hello World"); + expect(chk.description).toBe("Some longer description"); + expect(getItemSpy.calls.count()).toBe(1); + expect(updateModelSpy.calls.count()).toBe(1); + const modelToUpdate = updateModelSpy.calls.argsFor(0)[0]; + expect(modelToUpdate.item.description).toBe(content.description); + expect(modelToUpdate.item.properties.boundary).toBe("item"); }); }); describe("delete content", () => { diff --git a/packages/common/test/core/schemas/internal/getLocationOptions.test.ts b/packages/common/test/core/schemas/internal/getLocationOptions.test.ts index 4a3466008e2..22081f395c3 100644 --- a/packages/common/test/core/schemas/internal/getLocationOptions.test.ts +++ b/packages/common/test/core/schemas/internal/getLocationOptions.test.ts @@ -1,8 +1,9 @@ -import { IHubProject, IHubRequestOptions } from "../../../../src"; +import { IHubContent, IHubProject, IHubRequestOptions } from "../../../../src"; +import { ConfigurableEntity } from "../../../../src/core/schemas/internal/ConfigurableEntity"; import { getLocationOptions } from "../../../../src/core/schemas/internal/getLocationOptions"; import * as ExtentModule from "../../../../src/extent"; -describe("getLocationOptions:", () => { +describe("getLocationOptions - default:", () => { let orgExtentSpy: jasmine.Spy; beforeEach(() => { orgExtentSpy = spyOn( @@ -89,3 +90,93 @@ describe("getLocationOptions:", () => { expect(chk[2].selected).toBe(true); }); }); + +describe("getLocationOptions - content:", () => { + it("custom is selected", async () => { + const entity: ConfigurableEntity = { + id: "00c", + type: "Hub Content", + location: { + type: "custom", + }, + boundary: "item", + extent: [ + [100, 100], + [120, 120], + ], + } as ConfigurableEntity; + + const chk = await getLocationOptions( + entity, + "portalName", + {} as IHubRequestOptions + ); + + expect(chk.length).toBe(2); + expect(chk[1].selected).toBe(true); + }); + it("none is selected", async () => { + const entity: ConfigurableEntity = { + id: "00c", + type: "Hub Content", + location: { + type: "none", + }, + boundary: "none", + } as ConfigurableEntity; + + const chk = await getLocationOptions( + entity, + "portalName", + {} as IHubRequestOptions + ); + + expect(chk.length).toBe(2); + expect(chk[0].selected).toBe(true); + }); + it("custom is selected if entity does not have an id", async () => { + const entity: ConfigurableEntity = { + type: "Hub Content", + location: { + type: "custom", + }, + boundary: "item", + extent: [ + [100, 100], + [120, 120], + ], + } as ConfigurableEntity; + + const chk = await getLocationOptions( + entity, + "portalName", + {} as IHubRequestOptions + ); + + expect(chk.length).toBe(2); + expect(chk[1].selected).toBe(true); + }); + it("custom is selected & boundary is set", async () => { + const entity: ConfigurableEntity = { + id: "00c", + type: "Hub Content", + location: { + type: "custom", + }, + boundary: "item", + extent: [ + [100, 100], + [120, 120], + ], + } as ConfigurableEntity; + + const chk = await getLocationOptions( + entity, + "portalName", + {} as IHubRequestOptions + ); + + expect(chk.length).toBe(2); + expect(chk[1].selected).toBe(true); + }); +});