feat(frontend): resource-selection UI in MyOpenCRE (Part of #586) - #1005
feat(frontend): resource-selection UI in MyOpenCRE (Part of #586)#1005skypank-coder wants to merge 3 commits into
Conversation
|
Warning Review limit reached
Next review available in: 13 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Summary by CodeRabbit
WalkthroughAdds per-user resource selection loading and saving through ChangesResource Selector Feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 2
🧹 Nitpick comments (1)
application/frontend/src/hooks/useResourceSelection.ts (1)
22-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse async/await in the new frontend effects.
application/frontend/src/hooks/useResourceSelection.ts#L22-L49: replace the load Promise chain with an inner async function.application/frontend/src/components/ResourceSelector/ResourceSelector.tsx#L27-L55: replace the standards Promise chain with an inner async function.Preserve each effect's cleanup guard.
As per coding guidelines, “Prefer async/await over raw Promise chains or callbacks in new TypeScript frontend code.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@application/frontend/src/hooks/useResourceSelection.ts` around lines 22 - 49, Replace the Promise chain in the useResourceSelection effect at application/frontend/src/hooks/useResourceSelection.ts:22-49 with an inner async function using async/await, while preserving the active cleanup guard, status handling, error reporting, and loading updates. Apply the same async/await conversion to the standards-loading effect in application/frontend/src/components/ResourceSelector/ResourceSelector.tsx:27-55, preserving its cleanup guard and existing behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@application/frontend/src/components/ResourceSelector/ResourceSelector.tsx`:
- Around line 80-93: Update the anonymous-user handling in ResourceSelector so
every !isLoggedIn state avoids rendering the selector or allowing saves; show
the existing login prompt and button only when capabilities.login is enabled,
and show an unavailable message or otherwise hide the selector when login is
disabled. Add coverage for capabilities.login false with an anonymous user.
In `@application/frontend/src/hooks/useResourceSelection.ts`:
- Around line 39-49: Expose an explicit initial-load success/failure state from
useResourceSelection, setting it only after the selection request succeeds or
fails. In application/frontend/src/hooks/useResourceSelection.ts lines 39-49,
preserve the failure state instead of treating the empty selection as editable
data; in
application/frontend/src/components/ResourceSelector/ResourceSelector.tsx lines
91-115, use that state to disable selection editing and Save until the initial
request succeeds.
---
Nitpick comments:
In `@application/frontend/src/hooks/useResourceSelection.ts`:
- Around line 22-49: Replace the Promise chain in the useResourceSelection
effect at application/frontend/src/hooks/useResourceSelection.ts:22-49 with an
inner async function using async/await, while preserving the active cleanup
guard, status handling, error reporting, and loading updates. Apply the same
async/await conversion to the standards-loading effect in
application/frontend/src/components/ResourceSelector/ResourceSelector.tsx:27-55,
preserving its cleanup guard and existing behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3e6ef221-d1bb-48ec-9a20-2f38a9d1a20f
📒 Files selected for processing (9)
application/frontend/src/components/ResourceSelector/ResourceSelector.test.tsxapplication/frontend/src/components/ResourceSelector/ResourceSelector.tsxapplication/frontend/src/hooks/index.tsapplication/frontend/src/hooks/useResourceSelection.test.tsapplication/frontend/src/hooks/useResourceSelection.tsapplication/frontend/src/pages/MyOpenCRE/MyOpenCRE.tsxapplication/frontend/src/setupTests.tsjest.component.config.jspackage.json
…re in ResourceSelector (OWASP#586)
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
application/frontend/src/hooks/useResourceSelection.ts (1)
39-43: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject invalid successful load payloads.
At Line 40, a
200response without aselectedarray leavesselectedempty andloadErrorfalse.ResourceSelectorthen enables Save, so a malformed API response can replace persisted preferences with an empty selection. Treat this payload as a load failure. Add a regression test for a200response with an invalid body.Proposed fix
.then((data) => { - if (active && data && Array.isArray(data.selected)) { + if (!data || !Array.isArray(data.selected)) { + throw new Error('Invalid /user/resources response'); + } + if (active) { setSelected(data.selected); } })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@application/frontend/src/hooks/useResourceSelection.ts` around lines 39 - 43, Update the successful-load handling in useResourceSelection so a response is accepted only when data.selected is an array; otherwise reject it through the existing load-error path and keep persisted selection unchanged. Ensure ResourceSelector does not enable Save for this malformed 200 response, and add a regression test covering a successful response with an invalid body.
🧹 Nitpick comments (1)
application/frontend/src/hooks/useResourceSelection.ts (1)
27-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
async/awaitfor the initial load flow.Define a local async function inside
useEffectinstead of using thethen/catch/finallychain. This follows the frontend TypeScript convention and makes the status handling easier to maintain.As per coding guidelines, “Prefer async/await over raw Promise chains or callbacks in new TypeScript frontend code.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@application/frontend/src/hooks/useResourceSelection.ts` around lines 27 - 55, The initial resource-loading flow in useEffect should use a locally defined async function with try/catch/finally instead of the current then/catch/finally chain. Preserve the existing status handling, active guard, state updates, loading reset, and error logging while invoking the async loader from the effect.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@application/frontend/src/hooks/useResourceSelection.ts`:
- Around line 39-43: Update the successful-load handling in useResourceSelection
so a response is accepted only when data.selected is an array; otherwise reject
it through the existing load-error path and keep persisted selection unchanged.
Ensure ResourceSelector does not enable Save for this malformed 200 response,
and add a regression test covering a successful response with an invalid body.
---
Nitpick comments:
In `@application/frontend/src/hooks/useResourceSelection.ts`:
- Around line 27-55: The initial resource-loading flow in useEffect should use a
locally defined async function with try/catch/finally instead of the current
then/catch/finally chain. Preserve the existing status handling, active guard,
state updates, loading reset, and error logging while invoking the async loader
from the effect.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5859db14-83c1-4625-b53e-3ac8102c4c58
📒 Files selected for processing (4)
application/frontend/src/components/ResourceSelector/ResourceSelector.test.tsxapplication/frontend/src/components/ResourceSelector/ResourceSelector.tsxapplication/frontend/src/hooks/useResourceSelection.test.tsapplication/frontend/src/hooks/useResourceSelection.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- application/frontend/src/components/ResourceSelector/ResourceSelector.tsx
|
Fixed — a 200 with no selected[] is now treated as a load failure (loadError), so Save stays disabled and persisted preferences aren't overwritten; added a regression test. Also refactored the initial load to async/await. |
1 participant
What & why
Final core piece of #586, building on the merged backend chain: user + selection
persistence (#980), the GET/PUT /rest/v1/user/resources API (#981), and server-side
filtering of /rest/v1/standards (#1001). This wires the frontend on top.
Frontend-only — no backend changes.
Changes
useResourceSelectionhook — GET/PUT/rest/v1/user/resources, sameraw-fetch/status handling as
useUser(200 ok, 401 → anonymous, other → error);returns
{ selected, loading, save, saving, error }.ResourceSelectorcomponent — fetches the full universe via/rest/v1/standards?all=true(must bypass the per-user filter, or the pickercould never show unselected standards), renders a semantic-ui-react checkbox
list pre-checked from the saved selection, with an explicit Save button and
Loader / saved / error
<Message>states. Prompts to log in when logged out;renders nothing when the MyOpenCRE capability is off.
existing
capabilities.myopencregating).Reuses
useUser/useCapabilities/useEnvironmentunchanged. react-router v5Tests / first jsdom component-test lane
This adds the repo's first jsdom component-test lane:
jest.component.config.js(+
src/setupTests.ts) and a newyarn testscript, reusing@testing-library/reactts-jestalready inpackage.json. The existingtest:e2e/ puppeteer config isuntouched (no
jest.config.jsis introduced, so barejestkeeps its defaults),and this lane is not wired into
make frontendor CI yet.10 tests: hook load/401/save-PUT-body; component
?all=truefetch, pre-checkedselection, toggle+Save, success + error messages, logged-out prompt, myopencre-off gate.
Verification (local)
yarn test→ 10 passedyarn test:e2e --listTestsstill listsbasic-e2e.test.ts(e2e config unchanged)make frontend(webpack prod) builds cleanyarn lint(prettier) applied