[WIP] PortalJS Cloud portal-scoped APIs #22
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis PR removes org-specific name translation and env-dependent logic across UI, queries, and utils. Components and pages now use dataset/group/org .name directly. Queries stop applying main_org filters and return raw API results. Several helper functions and props are removed or simplified. README updates reflect env var changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant P as Next.js Page<br/>/@:org/:dataset
participant Q as Queries Layer
participant A as CKAN API
rect rgba(230, 240, 255, 0.5)
note over P,Q: New flow (no name translation)
U->>P: Request dataset page
P->>Q: getDataset(name)
Q->>A: package_show(id = name)
A-->>Q: dataset.result
Q-->>P: Dataset
P->>Q: getDatasetActivityStream(name)
Q->>A: activity_list(id = name)
A-->>Q: activities
Q-->>P: Activity stream
P-->>U: Render page with links using dataset.name
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
README.md (1)
69-69: Consider adding a clarifying comment about portal-scoped API endpoints.The updated example URL with
@my-portal-main-org-namecorrectly reflects the new portal-scoped API architecture. However, since the PR removes client-side org-specific transformations, it might be helpful to add a brief comment explaining that this is the portal-specific API endpoint structure in PortalJS Cloud, to avoid confusion with the removed client-side logic.For example:
```bash # This is the URL of the CKAN instance. Use the example value if you are using PortalJS Cloud. +# For PortalJS Cloud, each portal has a scoped API endpoint with @ prefix (e.g., @your-portal-name) NEXT_PUBLIC_DMS=https://api.cloud.portaljs.com/@my-portal-main-org-name</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between ad3a3bc69cfc17eb13a65dc23a9d2fead13a996d and f223343cf32b38cb28141b095ac82c2b7f95ef6c. </details> <details> <summary>📒 Files selected for processing (15)</summary> * `README.md` (2 hunks) * `components/dataset/individualPage/DatasetInfo.tsx` (2 hunks) * `components/dataset/search/DatasetCard.tsx` (1 hunks) * `components/dataset/search/DatasetItem.tsx` (2 hunks) * `components/home/mainSection/PopularDatasets.tsx` (1 hunks) * `lib/queries/dataset.ts` (4 hunks) * `lib/queries/groups.ts` (2 hunks) * `lib/queries/orgs.ts` (2 hunks) * `lib/queries/utils.ts` (0 hunks) * `lib/utils.ts` (0 hunks) * `pages/[org]/[dataset]/index.tsx` (3 hunks) * `pages/[org]/[dataset]/r/[resourceId].tsx` (0 hunks) * `pages/[org]/index.tsx` (1 hunks) * `pages/groups/[groupName].tsx` (1 hunks) * `pages/index.tsx` (1 hunks) </details> <details> <summary>💤 Files with no reviewable changes (3)</summary> * lib/utils.ts * lib/queries/utils.ts * pages/[org]/[dataset]/r/[resourceId].tsx </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧬 Code graph analysis (2)</summary> <details> <summary>pages/index.tsx (2)</summary><blockquote> <details> <summary>lib/queries/groups.ts (1)</summary> * `getAllGroups` (6-15) </details> <details> <summary>lib/queries/orgs.ts (1)</summary> * `getAllOrganizations` (21-30) </details> </blockquote></details> <details> <summary>components/dataset/individualPage/DatasetInfo.tsx (1)</summary><blockquote> <details> <summary>schemas/dataset.interface.ts (1)</summary> * `Dataset` (4-39) </details> </blockquote></details> </details> </details> <details> <summary>🔇 Additional comments (14)</summary><blockquote> <details> <summary>components/dataset/search/DatasetItem.tsx (1)</summary><blockquote> `9-9`: **LGTM! Simplified to use direct dataset names.** The removal of `getDatasetName` import and the direct usage of `dataset.name` in the href construction aligns well with the PR objective of eliminating org-specific name translation logic. Also applies to: 25-25 </blockquote></details> <details> <summary>components/home/mainSection/PopularDatasets.tsx (1)</summary><blockquote> `30-30`: **LGTM! Consistent with the broader refactor.** The href construction now uses `dataset.organization.name` and `dataset.name` directly, eliminating the `getDatasetName` utility. This change is consistent with similar updates in DatasetCard and DatasetItem components. </blockquote></details> <details> <summary>components/dataset/search/DatasetCard.tsx (1)</summary><blockquote> `69-69`: **LGTM! Simplified dataset name handling.** The direct assignment of `dataset.name` to `datasetName` removes the previous org-specific prefixing logic, aligning with the PR's goal of eliminating environment-driven transformations. </blockquote></details> <details> <summary>lib/queries/dataset.ts (2)</summary><blockquote> `94-95`: **LGTM! Simplified dataset retrieval.** The direct return of the dataset without name translation is consistent with the PR's goal of removing org-specific transformations. The function now returns raw CKAN API results. --- `10-10`: **Approve hardcoded "tags" facet** SearchContext’s generic searchFacets handler automatically includes the “tags” key in all UI facets. </blockquote></details> <details> <summary>lib/queries/groups.ts (1)</summary><blockquote> `17-29`: **LGTM! Simplified to return raw API results.** The `getGroup` function now returns `group.result` directly without name transformations, which is consistent with the overall refactor. The default `include_datasets = false` parameter is appropriate. </blockquote></details> <details> <summary>lib/queries/orgs.ts (1)</summary><blockquote> `6-18`: **LGTM! Simplified organization data retrieval.** The function now returns raw `organization.result` without name transformations or per-dataset adjustments, which is consistent with the broader refactor to eliminate environment-driven logic. </blockquote></details> <details> <summary>components/dataset/individualPage/DatasetInfo.tsx (1)</summary><blockquote> `16-16`: **All `dataset._name` references removed** Search across `.ts`/`.tsx` files returned no matches; removal is complete. </blockquote></details> <details> <summary>pages/[org]/[dataset]/index.tsx (3)</summary><blockquote> `33-33`: **LGTM!** Using `datasetName` directly simplifies the activity stream fetch and aligns with the removal of name transformation logic across the codebase. --- `49-50`: **Good addition of error logging.** The error logging improves debuggability when dataset fetching fails. --- `96-100`: **LGTM!** The multi-line conditional improves readability while maintaining the same logic. </blockquote></details> <details> <summary>pages/[org]/index.tsx (1)</summary><blockquote> `51-51`: **All org.name usages are valid** No `org._name` references remain; `getOrganization` returns a `name` field compatible with `ckan.getOrgActivityStream(org.name)`. </blockquote></details> <details> <summary>pages/groups/[groupName].tsx (1)</summary><blockquote> `35-35`: **Confirm `Group.name` compatibility.** No `group._name` references remain; verify that the `Group` interface from `@portaljs/ckan` includes a `name: string` field and that `CKAN.getGroupActivityStream` accepts it. </blockquote></details> <details> <summary>pages/index.tsx (1)</summary><blockquote> `26-27`: **Home and MainSection no longer require an orgs prop** — orgCount is still computed from getAllOrganizations(). </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| } | ||
|
|
||
| let fqList: string[] = [mainOrg ? `main_org:${mainOrg}` : ""]; | ||
| let fqList: string[] = []; |
There was a problem hiding this comment.
🧩 Analysis chain
Breaking change: Default main_org filter removed.
The empty fqList initialization removes the previous default organization filter that was likely applied. This is a behavioral change that will cause search results to include datasets from all organizations instead of being scoped to a main organization.
Verify that removing the default org filter doesn't break existing search functionality or expose unintended datasets:
🏁 Script executed:
#!/bin/bash
# Search for searchDatasets calls to verify they handle the new behavior
ast-grep --pattern $'searchDatasets({
$$$
})'Length of output: 597
🏁 Script executed:
#!/bin/bash
echo "=== fqList usage ==="
rg -n "fqList" -C5 lib/queries/dataset.ts
echo
echo "=== searchDatasets calls ==="
rg -nP --type=ts '\bsearchDatasets\s*\(' -C2 -g '!**/node_modules/**'Length of output: 2827
🏁 Script executed:
#!/bin/bash
echo "=== initialRequestOption in pages/search.tsx ==="
rg -n "initialRequestOption" -C5 pages/search.tsx
echo
echo "=== packagesOptions in SearchContext.tsx ==="
rg -n "packagesOptions" -C5 components/dataset/search/SearchContext.tsx
echo
echo "=== visualizationsOptions in SearchContext.tsx ==="
rg -n "visualizationsOptions" -C5 components/dataset/search/SearchContext.tsxLength of output: 2085
Breaking change: default organization filter removed
All searchDatasets calls now receive orgs: [] (or omit orgs), so no organization:(…) fq-param is sent. Restore the default filter by adding orgs: ['<MAIN_ORG>'] to existing calls (pages/index.tsx, pages/search.tsx, SearchContext.tsx) or reintroduce it inside searchDatasets.
🤖 Prompt for AI Agents
In lib/queries/dataset.ts around line 32, the default organization filter was
removed causing calls to send no organization fq; reintroduce the default by
making the searchDatasets function accept a defaulted orgs param (e.g. orgs =
['<MAIN_ORG>']) or, if orgs is undefined/empty inside the function, set orgs =
['<MAIN_ORG>']; then ensure fqList is built to push an organization:(<MAIN_ORG>)
filter when orgs contains the main org so the organization fq is included in
requests (alternatively add orgs: ['<MAIN_ORG>'] to callers pages/index.tsx,
pages/search.tsx, SearchContext.tsx).
Comment on lines
+6
to
+14
| export const getAllGroups = async () => { | ||
| const groups = await CkanRequest.get<CkanResponse<Group[]>>( | ||
| `group_list?all_fields=True`, | ||
| { | ||
| ckanUrl: DMS, | ||
| } | ||
| ); | ||
|
|
||
| return organizations.result.map((o) => { | ||
| return { ...o, _name: o.name }; | ||
| }); | ||
| } | ||
|
|
||
| const groupsTree = await CkanRequest.get< | ||
| CkanResponse<Group & { children: Group[] }> | ||
| >(`group_tree_section?type=group&id=${mainGroup}`, { | ||
| ckanUrl: DMS, | ||
| }); | ||
|
|
||
| let children = groupsTree.result.children; | ||
|
|
||
| if (detailed) { | ||
| children = await Promise.all( | ||
| children.map(async (g) => { | ||
| const groupDetails = await CkanRequest.get<CkanResponse<Group>>( | ||
| `group_show?id=${g.id}`, | ||
| { | ||
| ckanUrl: DMS, | ||
| } | ||
| ); | ||
| return groupDetails.result; | ||
| }) | ||
| ); | ||
| } | ||
|
|
||
| children = children.map((c) => { | ||
| const publicName = privateToPublicGroupName(c.name); | ||
| return { ...c, name: publicName }; | ||
| }); | ||
|
|
||
| return children; | ||
| return groups.result |
There was a problem hiding this comment.
🧩 Analysis chain
Breaking change: getAllGroups signature simplified.
The function signature changed from accepting a detailed parameter to being parameter-less. This is a breaking change for any existing call sites.
Verify all call sites have been updated:
🏁 Script executed:
#!/bin/bash
# Search for getAllGroups function calls
rg -n "getAllGroups\(" --type ts --type tsx -C 2Length of output: 27
🏁 Script executed:
#!/bin/bash
rg -n "getAllGroups\(" -g "*.ts" -g "*.tsx" -C 2Length of output: 487
Update call sites to match simplified getAllGroups signature.
- In pages/groups/index.tsx (line 13), change
await getAllGroups({ detailed: true })
to
await getAllGroups(). - Verify no other calls pass arguments to
getAllGroups.
🤖 Prompt for AI Agents
In lib/queries/groups.ts around lines 6 to 14 the getAllGroups function
signature was simplified to take no arguments, so update all call sites
accordingly; specifically in pages/groups/index.tsx at line 13 change await
getAllGroups({ detailed: true }) to await getAllGroups(), and run a project-wide
search for getAllGroups( to remove any passed arguments and ensure callers
expect a Promise<Group[]>, adjusting imports/usage if necessary.
Comment on lines
+21
to
+29
| export const getAllOrganizations = async () => { | ||
| const organizations = await CkanRequest.get<CkanResponse<Organization[]>>( | ||
| `organization_list?all_fields=True`, | ||
| { | ||
| ckanUrl: DMS, | ||
| } | ||
| ); | ||
|
|
||
| return organizations; | ||
| return organizations.result |
There was a problem hiding this comment.
🧩 Analysis chain
Breaking change: getAllOrganizations signature simplified.
The function signature changed from accepting a detailed parameter to being parameter-less, and it now returns raw API results. This is a breaking change that affects call sites.
Verify all call sites have been updated:
🏁 Script executed:
#!/bin/bash
# Search for getAllOrganizations function calls
rg -n "getAllOrganizations\(" --type ts --type tsx -C 2Length of output: 27
🏁 Script executed:
#!/bin/bash
# Search for getAllOrganizations function calls in .ts and .tsx files
rg -n "getAllOrganizations\(" -g '*.ts' -g '*.tsx' -C2Length of output: 505
Update call site in pages/organizations.tsx
Remove the now-unsupported { detailed: true } argument from the getAllOrganizations() call at pages/organizations.tsx:12.
🤖 Prompt for AI Agents
In lib/queries/orgs.ts around lines 21 to 29: the public API for
getAllOrganizations no longer accepts a parameter, but the call site in
pages/organizations.tsx still passes { detailed: true } (at line 12). Open
pages/organizations.tsx, locate the getAllOrganizations(...) invocation on or
near line 12, and remove the now-unsupported argument so the call is simply
getAllOrganizations(). Leave the import and usage otherwise unchanged.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Documentation
Refactor
Bug Fixes