feat(dev-portal-web): INFRA-1456 service user section - #7886
feat(dev-portal-web): INFRA-1456 service user section#7886SavelevMatthew wants to merge 4 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a B2B service-user section with environment-aware registration, grouped access-right editing, GraphQL persistence, pending-change diffs, localization, and navigation wiring. Registration now refetches either B2C or B2B rights based on app type. ChangesB2B service-user access rights
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant ServiceUserSection
participant AccessRightSetForm
participant Apollo
participant B2BAccessRightAPI
Operator->>ServiceUserSection: select environment
ServiceUserSection->>Apollo: query service-user rights
Apollo->>B2BAccessRightAPI: appId and environment
B2BAccessRightAPI-->>Apollo: rights and access-right sets
Apollo-->>AccessRightSetForm: current permissions
Operator->>AccessRightSetForm: toggle permissions and save
AccessRightSetForm->>Apollo: create access-right set
Apollo->>B2BAccessRightAPI: permission payload
B2BAccessRightAPI-->>Apollo: created status and permissions
Apollo-->>AccessRightSetForm: completion status
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d75125bc5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const currentRightSet = useMemo(() => pendingRightSet ?? approvedRightSet, [approvedRightSet, pendingRightSet]) | ||
|
|
||
| useEffect(() => { | ||
| if (currentRightSet) { |
There was a problem hiding this comment.
When switching from an environment that has a pending/approved right set to one that has a service user but no right set yet, this guard skips the effect and leaves both groupedPermissions and initialGroupedPermissions populated from the previous environment. The user then sees stale permissions for the new environment and the Save button can stay disabled until they make a change, so the first right set may be created from the wrong baseline. Reset to the all-false default whenever currentRightSet is absent for the selected environment.
Useful? React with 👍 / 👎.
| canReadBillingReceipts | ||
| canReadBillingReceiptFiles |
There was a problem hiding this comment.
The fragment that drives ShowedPermissions and GROUPED_PERMISSIONS stops at list permissions, so none of the existing B2B service permissions such as canExecuteRegisterExternalPayments, canExecuteRegisterMetersReadings, or canExecuteSetPaymentPosReceiptUrl can be viewed or requested from this new form. When an app needs those mutation rights, the owner has no way to enable them through the service-user section, and saving a replacement set only submits the displayed booleans, leaving these mutation permissions at their defaults.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
that's ok, we've decided to limit abilities for now
![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 (3)
apps/dev-portal-web/domains/miniapp/components/B2BApp/edit/service-user/AccessRightSetForm.tsx (3)
196-211: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAvoid nested
setStateinside another state's updater.
setInitialGroupedPermissionsis called as a side effect inside the updater function passed tosetGroupedPermissions. Updater functions should be pure (React may invoke them more than once, e.g. under Strict Mode double-invocation), so triggering another state update from within one is an anti-pattern even though it's currently harmless since the computed value is deterministic.Since the new value only depends on
currentRightSetand the staticGROUPED_PERMISSIONSshape (not on the previousgroupedPermissionsstate), you can compute it once and call both setters directly.♻️ Proposed refactor
useEffect(() => { - if (currentRightSet) { - setGroupedPermissions(prev => { - const newValue = prev.map(row => ({ - group: row.group, - permissions: row.permissions.map(permission => ({ - key: permission.key, - value: currentRightSet[permission.key] ?? permission.value, - })), - })) - setInitialGroupedPermissions(newValue) - - return newValue - }) - } + if (!currentRightSet) return + const newValue: Array<RowType> = Object.entries(GROUPED_PERMISSIONS).map(([group, permissions]) => ({ + group: group as keyof typeof GROUPED_PERMISSIONS, + permissions: permissions.map(permission => ({ + key: permission, + value: currentRightSet[permission] ?? false, + })), + })) + setInitialGroupedPermissions(newValue) + setGroupedPermissions(newValue) }, [currentRightSet])🤖 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 `@apps/dev-portal-web/domains/miniapp/components/B2BApp/edit/service-user/AccessRightSetForm.tsx` around lines 196 - 211, Refactor the useEffect handling currentRightSet so it computes the updated permission groups from GROUPED_PERMISSIONS and currentRightSet once, then calls setGroupedPermissions and setInitialGroupedPermissions separately. Remove the nested setInitialGroupedPermissions call from the setGroupedPermissions updater and avoid relying on previous state.
126-131: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winControlled
Checkboxhas noonChangehandler.The
checkedprop is set withoutonChange. This triggers a React "controlled input without onChange" warning; toggling only works because the parent row'sonClickhappens to update state on click. Add an explicit (no-op or delegating)onChangeso the component is self-consistent and warning-free.♻️ Proposed fix
- render: (value: boolean) => <Checkbox checked={value}/>, + render: (value: boolean, record) => <Checkbox checked={value} onChange={() => onChange(record)}/>,🤖 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 `@apps/dev-portal-web/domains/miniapp/components/B2BApp/edit/service-user/AccessRightSetForm.tsx` around lines 126 - 131, Update the Checkbox rendered by the table column’s render function to include an explicit onChange handler alongside checked={value}; use the appropriate no-op or existing state-delegating behavior while preserving the parent row click flow.
231-243: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInconsistent AntD imperative API: static
notificationvs.Modal.useModal().
Modal.useModal()(line 262) is used correctly to get a context-awarecontextHolder, butnotification.success(line 235) uses the static call. AntD 5 documentation explicitly recommends against the static form because it "cannot consume context like dynamic theme, and ConfigProvider data will not work." Consider switching tonotification.useNotification()(or theAppwrapper) for consistency and correct theme propagation, mirroring the Modal pattern already used here.🤖 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 `@apps/dev-portal-web/domains/miniapp/components/B2BApp/edit/service-user/AccessRightSetForm.tsx` around lines 231 - 243, Update the onCompleted callback in AccessRightSetForm to use the context-aware notification.useNotification() API instead of the static notification.success call. Mirror the existing Modal.useModal() pattern by obtaining the notification API and contextHolder, render that contextHolder with the form, and invoke success through the returned notification instance while preserving the existing message, description, and duration.
🤖 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 `@apps/dev-portal-web/lang/ru.json`:
- Line 320: Correct the Russian grammar in the labels for canReadTicketComments
and canReadTicketCommentFiles by changing “к заявками” to “к заявкам”, matching
the casing used by the corresponding canManage labels.
- Line 329: Correct the typo in the Russian translation value for
pages.apps.b2b.id.sections.serviceUser.accessRightSetForm.permissions.canManageMeterReportingPeriods.label,
changing “счачи” to “сдачи” while leaving the key and surrounding translations
unchanged.
---
Nitpick comments:
In
`@apps/dev-portal-web/domains/miniapp/components/B2BApp/edit/service-user/AccessRightSetForm.tsx`:
- Around line 196-211: Refactor the useEffect handling currentRightSet so it
computes the updated permission groups from GROUPED_PERMISSIONS and
currentRightSet once, then calls setGroupedPermissions and
setInitialGroupedPermissions separately. Remove the nested
setInitialGroupedPermissions call from the setGroupedPermissions updater and
avoid relying on previous state.
- Around line 126-131: Update the Checkbox rendered by the table column’s render
function to include an explicit onChange handler alongside checked={value}; use
the appropriate no-op or existing state-delegating behavior while preserving the
parent row click flow.
- Around line 231-243: Update the onCompleted callback in AccessRightSetForm to
use the context-aware notification.useNotification() API instead of the static
notification.success call. Mirror the existing Modal.useModal() pattern by
obtaining the notification API and contextHolder, render that contextHolder with
the form, and invoke success through the returned notification instance while
preserving the existing message, description, and duration.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f4de232f-0d47-472d-8c02-1f86518b98b9
📒 Files selected for processing (15)
apps/dev-portal-web/domains/miniapp/components/B2BApp/edit/service-user/AccessRightSetForm.module.cssapps/dev-portal-web/domains/miniapp/components/B2BApp/edit/service-user/AccessRightSetForm.tsxapps/dev-portal-web/domains/miniapp/components/B2BApp/edit/service-user/DiffContainer.tsxapps/dev-portal-web/domains/miniapp/components/B2BApp/edit/service-user/ServiceUserSection.tsxapps/dev-portal-web/domains/miniapp/components/B2CApp/edit/service-user/ServiceUserSection.tsxapps/dev-portal-web/domains/miniapp/components/User/edit/RegisterUserForm.tsxapps/dev-portal-web/domains/miniapp/components/User/edit/RegisterUserModal/index.tsxapps/dev-portal-web/domains/miniapp/constants/b2bAppAccessRightSet.tsapps/dev-portal-web/domains/miniapp/hooks/useB2BMenuItems.tsxapps/dev-portal-web/domains/miniapp/queries/B2BAppAccessRight.graphqlapps/dev-portal-web/domains/miniapp/queries/B2BAppAccessRightSet.graphqlapps/dev-portal-web/gql/index.tsapps/dev-portal-web/lang/en.jsonapps/dev-portal-web/lang/ru.jsonapps/dev-portal-web/pages/apps/b2b/[id].tsx
| "pages.apps.b2b.id.sections.serviceUser.accessRightSetForm.permissions.canManageTickets.label": "Управление заявками", | ||
| "pages.apps.b2b.id.sections.serviceUser.accessRightSetForm.permissions.canReadTicketFiles.label": "Просмотр вложений к заявкам", | ||
| "pages.apps.b2b.id.sections.serviceUser.accessRightSetForm.permissions.canManageTicketFiles.label": "Управление вложениями к заявкам", | ||
| "pages.apps.b2b.id.sections.serviceUser.accessRightSetForm.permissions.canReadTicketComments.label": "Просмотр комментариев к заявками", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Grammar: "к заявками" should be "к заявкам".
Both canReadTicketComments and canReadTicketCommentFiles labels use the wrong case ("к заявками" instead of dative "к заявкам"), inconsistent with the correctly-cased canManageTicketComments/canManageTicketCommentFiles labels right below them.
✏️ Proposed fix
- "pages.apps.b2b.id.sections.serviceUser.accessRightSetForm.permissions.canReadTicketComments.label": "Просмотр комментариев к заявками",
+ "pages.apps.b2b.id.sections.serviceUser.accessRightSetForm.permissions.canReadTicketComments.label": "Просмотр комментариев к заявкам",- "pages.apps.b2b.id.sections.serviceUser.accessRightSetForm.permissions.canReadTicketCommentFiles.label": "Просмотр вложений в комментариях к заявками",
+ "pages.apps.b2b.id.sections.serviceUser.accessRightSetForm.permissions.canReadTicketCommentFiles.label": "Просмотр вложений в комментариях к заявкам",Also applies to: 322-322
🤖 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 `@apps/dev-portal-web/lang/ru.json` at line 320, Correct the Russian grammar in
the labels for canReadTicketComments and canReadTicketCommentFiles by changing
“к заявками” to “к заявкам”, matching the casing used by the corresponding
canManage labels.
| "pages.apps.b2b.id.sections.serviceUser.accessRightSetForm.permissions.canReadMeterReadings.label": "Просмотр показаний приборов учёта", | ||
| "pages.apps.b2b.id.sections.serviceUser.accessRightSetForm.permissions.canManageMeterReadings.label": "Управление показаниями приборов учёта", | ||
| "pages.apps.b2b.id.sections.serviceUser.accessRightSetForm.permissions.canReadMeterReportingPeriods.label": "Просмотр периодов сдачи показаний", | ||
| "pages.apps.b2b.id.sections.serviceUser.accessRightSetForm.permissions.canManageMeterReportingPeriods.label": "Управление периодами счачи показаний", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Typo: "счачи" → "сдачи".
✏️ Proposed fix
- "pages.apps.b2b.id.sections.serviceUser.accessRightSetForm.permissions.canManageMeterReportingPeriods.label": "Управление периодами счачи показаний",
+ "pages.apps.b2b.id.sections.serviceUser.accessRightSetForm.permissions.canManageMeterReportingPeriods.label": "Управление периодами сдачи показаний",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "pages.apps.b2b.id.sections.serviceUser.accessRightSetForm.permissions.canManageMeterReportingPeriods.label": "Управление периодами счачи показаний", | |
| "pages.apps.b2b.id.sections.serviceUser.accessRightSetForm.permissions.canManageMeterReportingPeriods.label": "Управление периодами сдачи показаний", |
🤖 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 `@apps/dev-portal-web/lang/ru.json` at line 329, Correct the typo in the
Russian translation value for
pages.apps.b2b.id.sections.serviceUser.accessRightSetForm.permissions.canManageMeterReportingPeriods.label,
changing “счачи” to “сдачи” while leaving the key and surrounding translations
unchanged.
|
1 participant
Summary by CodeRabbit