[AAASM-5211] 🐛 (iam): Key RoleCapabilityCards ROLE_BADGE_TONE by Map - #1771
Conversation
RoleCapabilityCards still indexed ROLE_BADGE_TONE as a plain object literal with `?? 'iam-role-badge--viewer'` fallback, so a role id matching an inherited Object.prototype member (constructor, toString, __proto__, …) would resolve to that inherited value instead of the fallback tone. This is the same class of gap AAASM-5229/#1744 fixed in the neighboring but distinct MemberList.tsx ROLE_BADGE_TONE — this one was left untouched by that ticket's scope and remained live.
Regression test asserting that inherited Object.prototype member names (constructor, toString, __proto__, hasOwnProperty, valueOf) and unknown role ids fall back to the default badge tone rather than resolving to a stray prototype value. Fails against the pre-fix object-literal lookup.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Chisanan232
left a comment
Chisanan232
left a comment
There was a problem hiding this comment.
✅ LGTM — reviewed and approved by Claude Code
(Recorded as a review comment because GitHub blocks self-approval on an own-authored PR; this is the sign-off of record.)
Requirement correctness
This closes a genuine gap left by the AAASM-5208 sweep, found by independently re-verifying every Story in the Epic against live origin/main rather than trusting Jira's "Done" status: RoleCapabilityCards.tsx's own ROLE_BADGE_TONE was never converted, even though the byte-identical table in the sibling MemberList.tsx was fixed under AAASM-5229 (PR #1744) — confirmed via git log and get_pull_request_files on #1744 that it only ever touched MemberList.tsx. ROLE_BADGE_TONE → Map, read site → .get() ?? 'iam-role-badge--viewer'. Regression test covers constructor/toString/__proto__/hasOwnProperty/valueOf plus an ordinary unknown-role case, all asserting the default tone renders rather than an inherited-prototype value leaking into the badge class.
CI — fully green
17/17 checks pass, including Dashboard (type-check + lint), Dashboard tests + coverage, Dashboard e2e (Playwright), aa-cli build compat, SonarCloud ×2, CodeQL, and CI Success.
Security
Closes the last remaining Epic-5208 site of this shape — no new risk introduced.
Note for the record: a second, independently-dispatched PR (#1775, AAASM-5245) found and fixed this exact same site while adding a regression lint rule, before this PR merged. Will rebase/adjust #1775 to drop its now-duplicate RoleCapabilityCards.tsx hunk once this merges, to avoid a conflict.
Merging with a merge commit as org admin.
🤖 Reviewed with Claude Code
… rule) Closes AAASM-5245 (and its parent AAASM-5216). Adds the no-restricted-syntax rule banning Record<> object-literal lookup tables, with its three documented gaps, closing the prevention half of Epic AAASM-5208. Found and fixed one genuine missed site (RoleCapabilityCards.tsx's ROLE_BADGE_TONE) independently of PR #1771, which landed first — rebased to drop the now-duplicate hunk. Reviewed and approved by Claude Code (CI green 17/17, requirement-correct, all 45 suppressions individually justified, no security concerns). Merged as org admin.
1 participant
Description
While verifying AAASM-5211's acceptance criteria (subtask AAASM-5231), found that one of the Story's four target sites was never actually fixed despite the Story/Epic being tracked as complete:
dashboard/src/features/iam/RoleCapabilityCards.tsx'sROLE_BADGE_TONEwas still a plainRecord<string, string>object literal, indexed with bracket notation (ROLE_BADGE_TONE[role] ?? 'iam-role-badge--viewer').This is the same wire-data-keyed object-literal lookup pattern the rest of Epic AAASM-5208 eradicated — a role id equal to an inherited
Object.prototypemember name (constructor,toString,__proto__,hasOwnProperty,valueOf) would resolve to that inherited value instead of falling through to the default tone class, defeating the?? 'iam-role-badge--viewer'fallback.Note: this file/variable is distinct from the byte-identical-looking
ROLE_BADGE_TONEinfeatures/iam/MemberList.tsx, which AAASM-5229's PR #1744 already converted to aMap— that PR's own description explicitly notedRoleCapabilityCards.tsxwas left untouched, out of scope for that ticket. This PR closes that gap forRoleCapabilityCards.tsx's ownROLE_BADGE_TONE, following the identicalMappattern already established inMemberList.tsx.Type of Change
Breaking Changes
Related Issues
Testing
RoleCapabilityCard — badge tonedescribe block inRoleCapabilityCards.test.tsxasserting inherited-prototype-key role ids (constructor,toString,__proto__,hasOwnProperty,valueOf) and unknown roles fall back to the default tone class. These assertions fail against the pre-fix object-literal lookup.Locally verified in
dashboard/:pnpm exec tsc --noEmit— cleanpnpm exec eslint .— cleanpnpm exec vitest run— 272 files / 3022 tests passed (full suite)Checklist
git commit -s) — advisory only per CONTRIBUTING.md