HubSiteManager & HubSites module #741

dbouwman · 2022-03-09T14:44:24Z

Description:

Adds the HubSiteManager class, and it's backing HubSites module
Adds .sites to the Hub class.
DRY-ed up how .search(..) works for the Manager classes. They now delegate to searchContentEntities(...) which takes a conversion function that handles converting from an item to the entity type. These search functions only search against the Portal API - they never search using the Hub API.
additionally, Managers have .fromItem(IItem, requestOptions)

Note: The e2e's are intentionally brittle. They are not intended to be run in CI, but rather, are used during development / debugging to ensure the platform responds as expected. If we want to have ci-runnable e2e's we can add tasks to do that

Note Part Deux
FireFox tests failed in CI on this. Took a few hours of digging but the issue was due to rebuilding package-lock.json, which picked up jasmine-core 3.99.0from3.4.0. Forcing things back to 3.4.0` resolved the issue, and I created an issue to resolve

Instructions for testing:

run tests
run e2e tests to see it work with real items

Closes Issues: #3346
ran commit script (npm run c)

For more information see the README

codecov · 2022-03-09T19:22:49Z

Codecov Report

Merging #741 (6054f48) into master (62791d8) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 6054f48 differs from pull request most recent head 253a7dc. Consider uploading reports for the commit 253a7dc to get more accurate results

@@             Coverage Diff             @@
##            master      #741     +/-   ##
===========================================
  Coverage   100.00%   100.00%             
===========================================
  Files          345       484    +139     
  Lines         6528      8374   +1846     
  Branches      1193      1410    +217     
===========================================
+ Hits          6528      8374   +1846

Impacted Files	Coverage Δ
packages/common/src/search/_searchContent.ts	`100.00% <ø> (ø)`
packages/common/src/search/group-utils.ts	`100.00% <ø> (ø)`
packages/common/src/search/utils.ts	`100.00% <ø> (ø)`
packages/sites/src/index.ts	`100.00% <ø> (ø)`
packages/sites/src/register-site-as-application.ts	`100.00% <ø> (ø)`
packages/common/src/ArcGISContext.ts	`100.00% <100.00%> (ø)`
packages/common/src/Hub.ts	`100.00% <100.00%> (ø)`
packages/common/src/core/helpers/index.ts	`100.00% <100.00%> (ø)`
packages/common/src/core/index.ts	`100.00% <100.00%> (ø)`
packages/common/src/core/types/index.ts	`100.00% <100.00%> (ø)`
... and 155 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca8d754...253a7dc. Read the comment docs.

dbouwman · 2022-03-10T16:15:36Z

package.json

@@ -53,7 +53,7 @@
 		"husky": "^4.3.0",

 		"jasmine": "^3.6.4",
-		"jasmine-core": "^3.4.0",
+		"jasmine-core": "3.4.0",


This was needed to resolve some FireFox test failures. I've opened #742 to look into this

dbouwman · 2022-03-10T16:17:49Z

packages/common/src/ArcGISContext.ts

-        hubApiUrl: this.hubUrl,
-      };
-    }
+


We need to be able to create IHubRequestOptions when not auth'd

dbouwman · 2022-03-10T16:21:00Z

packages/common/src/search/searchContentEntities.ts

+  serializeContentFilterForPortal,
+} from ".";
+import { ISearchResponse } from "..";
+


Would appreciate input from @tomwayson on this - it all works - even after this is merged

I'm not sure what you are expecting not to work, but we definitely should not be importing from "." and ".." b/c of circular deps.

was more the use of createContentEntitySearchFn which holds a closure over a ItemToEntityFunction<T>. It's really useful but a little complex. That said, the IHubItemEntity interface now requires an fromItem(IItem):Promise<T> which is the function needed in the closure... so... complex, but not random?

dbouwman · 2022-03-10T16:22:10Z

packages/common/src/sites/HubSiteManager.ts

+  async create(
+    site: Partial<IHubSite>,
+    requestOptions?: IHubRequestOptions
+  ): Promise<IHubSite> {


Since this is a multi-step operation, I'm considering returning the IHubSite & OperationStack but mulling how best to encapsulate that w/o it getting gross

tomwayson

there's a lot of code here, so I only gave a cursory glance, esp w/ the tests. Main thing from my POV is to not export anything we don't need at the time of release in opendata-ui. We can always add exports to index.ts files later.

tomwayson · 2022-03-11T19:08:02Z

packages/common/src/items/index.ts

@@ -16,3 +16,4 @@ export * from "./get-structured-license";
 export * from "./slugs";
 export * from "./normalize-solution-template-item";
 export * from "./setItemThumbnail";
+export * from "./registerBrowserApp";


does this need to be part of the external API? Is anyone ever going to use this fn on it's own, or just registerSiteAsApplication()?

needed b/c it's re-exported in sites package

tomwayson · 2022-03-11T19:11:47Z

packages/common/src/sites/index.ts

+export * from "./fetchSiteModel";
 export * from "./get-site-by-id";
+export * from "./HubSiteManager";
+export * from "./HubSites";
+export * from "./registerSiteAsApplication";
 export * from "./site-schema-version";
+export * from "./themes";


ditto w/ all these - if we don't have a known place where we are going to call any of them from the Hub app, or hub-components, let's not export them. We can always export them later if needed. If we needlessly export them now it becomes very difficult to modify or remove them and adds to our maintenance costs.

either needed directly or re-exported in hub-sites

packages/common/src/sites/themes.ts

packages/common/src/core/helpers/index.ts

packages/common/src/core/index.ts

packages/common/src/search/index.ts

tomwayson · 2022-03-11T19:25:58Z

packages/common/src/search/searchContentEntities.ts

+  serializeContentFilterForPortal,
+} from ".";
+import { ISearchResponse } from "..";
+


I'm not sure what you are expecting not to work, but we definitely should not be importing from "." and ".." b/c of circular deps.

github-actions · 2022-03-15T13:30:36Z

There hasn't been any activity on this pull request in the past 3 days, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days. If you want this PR to never become stale, please apply the "Draft" label.

tomwayson

thanks for making these fns internal-only.

I'm still wary of the from "..". I understand that it can cause mocking issues. The way I got around those w/ content was to move the shared fn out into it's own module, or a "lower level" module that could be safely imported by all consuming locations.

dbouwman · 2022-03-22T18:50:18Z

@tomwayson I'll see what I can do about those... fwiw - I believe that's what vscode does when you move files around / automatically add the imports vs manually typing the import

tomwayson · 2022-03-22T19:07:37Z

yep, that's exactly what it does, which is unfortunate. I have already pulled this, and started fixing them locally, you want me to push what I have?

…rch fns

dbouwman commented Mar 10, 2022

View reviewed changes

tomwayson requested changes Mar 11, 2022

View reviewed changes

github-actions bot added the Stale Stale issue - update or it will be closed label Mar 15, 2022

tomwayson reviewed Mar 22, 2022

View reviewed changes

github-actions bot removed the Stale Stale issue - update or it will be closed label Mar 23, 2022

drspacemanphd mentioned this pull request Mar 23, 2022

fix(hub-common): fixed searchProjects to pass provided api #754

Closed

1 task

dbouwman and others added 9 commits March 23, 2022 14:44

feat: add hubsitemanager (wip)

1008101

refactor: tests passing

ba0ce79

feat: hub site manager class

8545fe8

docs: update entities guide

f1d3c7c

chore: resolve failing test in firefox

744047d

refactor: keep more internal

d36f967

refactor: move more internal fns into _internal folders

ce09dc2

chore(hub-common): remove circular imports (WIP)

785732e

refactor(hub-common): allow api to be passed into entity specific sea…

253a7dc

…rch fns

dbouwman force-pushed the f/hub-sites branch from b72291b to 253a7dc Compare March 23, 2022 21:03

dbouwman merged commit e0f9d7d into master Mar 23, 2022

tomwayson deleted the f/hub-sites branch March 23, 2022 21:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HubSiteManager & HubSites module #741

HubSiteManager & HubSites module #741

dbouwman commented Mar 9, 2022 •

edited

Loading

codecov bot commented Mar 9, 2022 •

edited

Loading

dbouwman Mar 10, 2022

dbouwman Mar 10, 2022

dbouwman Mar 10, 2022

tomwayson Mar 11, 2022

dbouwman Mar 21, 2022

dbouwman Mar 10, 2022

tomwayson left a comment

tomwayson Mar 11, 2022

dbouwman Mar 21, 2022

tomwayson Mar 11, 2022

dbouwman Mar 21, 2022

tomwayson Mar 11, 2022

github-actions bot commented Mar 15, 2022

tomwayson left a comment

dbouwman commented Mar 22, 2022

tomwayson commented Mar 22, 2022

HubSiteManager & HubSites module #741

HubSiteManager & HubSites module #741

Conversation

dbouwman commented Mar 9, 2022 • edited Loading

codecov bot commented Mar 9, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomwayson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 15, 2022

tomwayson left a comment

Choose a reason for hiding this comment

dbouwman commented Mar 22, 2022

tomwayson commented Mar 22, 2022

dbouwman commented Mar 9, 2022 •

edited

Loading

codecov bot commented Mar 9, 2022 •

edited

Loading