fix: fixed the hiding edit button in summery2#4059
Conversation
JamalAlabdullah
added
kind/bug
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an EditButton to attachment summary headers with new CSS classes; refactors repeating-group child discovery to recursively collect editable form children; updates unit and e2e tests for nested repeating-group child navigation and adjusted Endre button counts. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan for PR comments
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.
🧹 Nitpick comments (1)
src/layout/RepeatingGroup/utils.ts (1)
106-109: Remove the containerascast here.Line 107 introduces a new unchecked cast in the traversal path. Prefer a type guard on
childComponent/childrenso this stays type-safe and future container shape changes fail at compile time instead of silently at runtime.♻️ Suggested cleanup
- if (componentDef.category === CompCategory.Container) { - const containerChildren = (childComponent as { children?: string[] }).children ?? []; + if (componentDef.category === CompCategory.Container) { + const containerChildren = + 'children' in childComponent && Array.isArray(childComponent.children) ? childComponent.children : []; for (const grandChildId of containerChildren) { collectEditableChildren(grandChildId, layoutLookups, parentComponent, rowWithExpressions, acc); } return; }As per coding guidelines, "Avoid using
anyor type casting (as type) in TypeScript; instead, improve typing by removing such casts andanys to maintain proper type safety".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/layout/RepeatingGroup/utils.ts` around lines 106 - 109, The code uses an unchecked cast on childComponent to access children inside the CompCategory.Container branch; instead add a type guard to assert the shape (e.g., check 'children' in childComponent && Array.isArray(childComponent.children')) before iterating so the cast is removed—update the block guarded by componentDef.category === CompCategory.Container to retrieve containerChildren only when the guard passes and then call collectEditableChildren(grandChildId, layoutLookups, parentComponent, rowWithExpressions, acc) for each grandChildId; this keeps typesafe access to childComponent.children and eliminates the as-cast.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/layout/RepeatingGroup/utils.ts`:
- Around line 106-109: The code uses an unchecked cast on childComponent to
access children inside the CompCategory.Container branch; instead add a type
guard to assert the shape (e.g., check 'children' in childComponent &&
Array.isArray(childComponent.children')) before iterating so the cast is
removed—update the block guarded by componentDef.category ===
CompCategory.Container to retrieve containerChildren only when the guard passes
and then call collectEditableChildren(grandChildId, layoutLookups,
parentComponent, rowWithExpressions, acc) for each grandChildId; this keeps
typesafe access to childComponent.children and eliminates the as-cast.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c98bff95-5e69-4045-8418-3d9519a2dc51
📒 Files selected for processing (3)
src/layout/FileUpload/AttachmentSummaryComponent2.tsxsrc/layout/FileUpload/FileUploadTable/FileTableComponent.module.csssrc/layout/RepeatingGroup/utils.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/layout/RepeatingGroup/Summary2/RepeatingGroupTableSummary/RepeatingGroupTableSummary.test.tsx`:
- Around line 222-233: The test currently asserts the fallback navigation target
("repeating-group") after clicking the edit button; update it to assert the
nested child target instead: use the same setup (render with
layoutWithNestedGroupChild and clicking the editButton) but change the
expectation on the navigate mock to verify navigation to the nested child id
(the `nested-input` target referenced by `tableHeaders` / collected by
`collectEditableChildren`) — i.e., replace the
expect(navigate).toHaveBeenCalledWith('repeating-group', 'repeating-group', ...)
assertion with an assertion that the second argument is the nested child id
(e.g. 'nested-input') while keeping expect.any(Object) for the options.
- Line 12: Remove the double type cast on the `inner-group` fixture: locate the
fixture named "inner-group" (used in RepeatingGroupTableSummary tests) and
delete the trailing `as unknown as CompInternal` so the fixture is left as its
concrete Group object type; keep the import of CompInternal if still used
elsewhere but do not cast the fixture—this matches the pattern used in
SummaryGroupComponent.test.tsx and Form.test.tsx and preserves type safety.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 868701db-5fbf-4d68-a2bd-22fa4210601e
📒 Files selected for processing (1)
src/layout/RepeatingGroup/Summary2/RepeatingGroupTableSummary/RepeatingGroupTableSummary.test.tsx
...ayout/RepeatingGroup/Summary2/RepeatingGroupTableSummary/RepeatingGroupTableSummary.test.tsx
Outdated
Show resolved
Hide resolved
...ayout/RepeatingGroup/Summary2/RepeatingGroupTableSummary/RepeatingGroupTableSummary.test.tsx
Show resolved
Hide resolved
|
JamalAlabdullah
added
the
squad/utforming
There was a problem hiding this comment.
Good job! 💯 I am curious why the added styling in FileTableComponent is necessary, other than that approved and you can merge this if you have tested this well when it should be displayed and when it shouldn't be displayed. 🙂 I have tested a little, where the edit button is displayed and one case where it is hidden when readOnly is added, and it works as expected 👍
Edit: you can you deploy this to production when it is merged by using backport as labeled.
I added styling because the edit button was not in this part( see picture ), I added the button explicitly and the hover and placement was not as other buttons , thats why i added the style. |
github-project-automation
bot
moved this from 🔎 In review
to ✅ Done
in Team Altinn Studio
* fix: fixed the hiding edit button in summery2 * added test * test * test * test
|
✅ Automatic backport successful! A backport PR has been automatically created for the The release branch The cherry-pick was clean with no conflicts. Please review the backport PR when it appears. |
2 participants
Description
Before
After
Related Issue(s)
Verification/QA
kind/*andbackport*label to this PR for proper release notes groupingSummary by CodeRabbit
New Features
Style
Tests
Refactor