Add application assignment UI for policies#10422
Open
hwupathum wants to merge 9 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughAdds an application assignment UI to the consent policy edit page. A new API module manages policy–application assignments via REST, backed by a React hook that fetches assigned application IDs. A new ChangesApplication Assignment UI for Consent Policies
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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
requested review from
DonOmalVindula,
JayaShakthi97,
NipuniBhagya and
pavinduLakshan
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
features/admin.consents.v1/api/consent-policy-apps.ts (1)
38-40: ⚡ Quick winRename
ApplicationObjectto match interface naming rules.Line 38 defines an interface without the required
Interfacesuffix; please rename it and update usages.Suggested refactor
-export interface ApplicationObject { +export interface ApplicationObjectInterface { id: string; } @@ - (response.data as ApplicationObject[]).map((app: ApplicationObject): string => app.id) + (response.data as ApplicationObjectInterface[]) + .map((app: ApplicationObjectInterface): string => app.id) @@ - data: { id } as ApplicationObject, + data: { id } as ApplicationObjectInterface,As per coding guidelines,
Use Interface suffix for all interface names (e.g., ApplicationListInterface, FeatureConfigInterface).Also applies to: 57-57, 85-85
🤖 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 `@features/admin.consents.v1/api/consent-policy-apps.ts` around lines 38 - 40, The interface `ApplicationObject` defined at line 38 does not follow the required naming convention which mandates an `Interface` suffix for all interface names. Rename `ApplicationObject` to `ApplicationObjectInterface` and update all usages of this interface throughout the file, including the usages at lines 57 and 85 (and any other occurrences) to use the new name `ApplicationObjectInterface` instead.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 @.changeset/plain-dolls-dance.md:
- Line 9: The changeset description contains a typo where "assignemnt" is
misspelled. Correct this spelling error to "assignment" in the changeset file to
ensure the description reads "Add application assignment UI for policies"
instead of the current misspelled version.
In `@features/admin.consents.v1/api/consent-policy-apps.ts`:
- Around line 27-31: The getPurposeAppsUrl and getPurposeAppUrl functions are
composing URLs using the generic serverConfigurations endpoint instead of the
dedicated consent-policy endpoint contract. Update both functions to use the
tenant-host-safe consent-policy endpoint from
features/common.consents.v1/configs/endpoints.ts instead of constructing the URL
from store.getState().config.endpoints.serverConfigurations, ensuring the base
URL path correctly routes to the consent-policy endpoint in sub-org contexts.
- Around line 59-62: In the .catch() error handler for the AxiosError, remove
the check for HTTP 500 status code from the condition that returns an empty
array. The condition should only check for status 404 (not found), as that
legitimately represents no assignments. For 500 errors and other failures,
either rethrow the error or handle it separately to ensure server failures are
not masked as empty results and allow downstream save/delete flows to detect the
actual failure state.
In `@features/admin.consents.v1/components/policy-consent-applications.tsx`:
- Around line 69-71: The interface PolicyConsentApplicationsProps does not
follow the required naming convention that mandates all interface names use the
Interface suffix. Rename PolicyConsentApplicationsProps to
PolicyConsentApplicationsInterface at line 69, and update any references to this
interface throughout the file to use the new name. Also check lines 80-82 as
mentioned in the comment and apply the same Interface suffix naming convention
to any other interface definitions found there.
- Around line 192-216: The persistAssignments function is being called without
awaiting, which allows overlapping remove/assign actions to race and produce
inconsistent server state. Additionally, when API calls fail in the
deleteConsentPolicyApps or saveConsentPolicyApps error handlers, the local state
is left stale with only an alert shown and no state reconciliation. Make
persistAssignments async and ensure all callers await its completion to prevent
race conditions. In both the deleteConsentPolicyApps and saveConsentPolicyApps
catch blocks, add state reconciliation logic (either rollback the optimistic
update or refetch the current state from the server) in addition to showing the
alert. Apply the same async/await pattern and error reconciliation to the other
instances of persistAssignments usage mentioned in the comment.
- Around line 67-68: Replace the semantic-ui-react import statement to use
Oxygen UI equivalents exclusively. Remove the imports for Modal, Header, Icon,
and Divider from semantic-ui-react and import Dialog, DialogTitle,
DialogContent, and DialogActions from `@oxygen-ui/react` instead. Then locate all
usages of these components throughout the file: replace Header components (used
in the JSX rendering sections) with a Typography component or styled component
from MUI, replace all Icon usages with MUI icon components, replace the Divider
component import with the Oxygen UI Divider equivalent, and refactor the Modal
component (which wraps content in lines 460-538) to use the Dialog component
structure with DialogTitle, DialogContent, and DialogActions for proper
organization.
In `@features/admin.consents.v1/pages/policy-consent-edit.tsx`:
- Around line 83-87: The activeTab state can remain at a non-existent tab index
when switching from a multi-tab policy to a single-tab default policy. Add a
useEffect hook that monitors policy changes and resets activeTab to 0 whenever
the policy switches to a default policy (single-tab mode). This ensures that
when activeTab is set via setActiveTab, it always corresponds to an existing
tab. Apply similar reset logic at the locations referenced in the "Also applies
to" section around lines 205-233.
---
Nitpick comments:
In `@features/admin.consents.v1/api/consent-policy-apps.ts`:
- Around line 38-40: The interface `ApplicationObject` defined at line 38 does
not follow the required naming convention which mandates an `Interface` suffix
for all interface names. Rename `ApplicationObject` to
`ApplicationObjectInterface` and update all usages of this interface throughout
the file, including the usages at lines 57 and 85 (and any other occurrences) to
use the new name `ApplicationObjectInterface` instead.
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 36f03f6f-c440-4883-98b4-58836fb9ba74
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
.changeset/plain-dolls-dance.mdapps/console/src/public/deployment.config.jsonfeatures/admin.consents.v1/api/consent-policy-apps.tsfeatures/admin.consents.v1/components/consent-description-editor.tsxfeatures/admin.consents.v1/components/edit-policy-consent.tsxfeatures/admin.consents.v1/components/edit-preference-management.tsxfeatures/admin.consents.v1/components/policy-consent-applications.tsxfeatures/admin.consents.v1/components/policy-consent-preview.tsxfeatures/admin.consents.v1/components/policy-consents-list.tsxfeatures/admin.consents.v1/package.jsonfeatures/admin.consents.v1/pages/policy-consent-edit.tsxfeatures/admin.consents.v1/pages/policy-consents.tsxfeatures/admin.core.v1/store/reducers/config.tsfeatures/common.consents.v1/configs/endpoints.tsfeatures/common.consents.v1/models/endpoints.tsmodules/i18n/src/models/namespaces/consents-ns.tsmodules/i18n/src/translations/en-US/portals/consents.ts
Comment on lines
+67
to
+68
| import { Divider, Header, Icon, Modal, PaginationProps } from "semantic-ui-react"; | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify semantic-ui-react usage in this new component.
rg -n --type=tsx 'semantic-ui-react|<Modal|<Header|<Icon|<Divider' features/admin.consents.v1/components/policy-consent-applications.tsxRepository: wso2/identity-apps
Length of output: 90
🏁 Script executed:
#!/bin/bash
# First, verify the file exists
ls -la features/admin.consents.v1/components/policy-consent-applications.tsx
# Search for semantic-ui-react imports and usage without type flag
rg -n 'from.*semantic-ui-react|import.*semantic-ui-react' features/admin.consents.v1/components/policy-consent-applications.tsx
# Also search for the component usage
rg -n '<Modal|<Header|<Icon|<Divider' features/admin.consents.v1/components/policy-consent-applications.tsxRepository: wso2/identity-apps
Length of output: 799
🏁 Script executed:
#!/bin/bash
# Get file size to determine how to read it
wc -l features/admin.consents.v1/components/policy-consent-applications.tsxRepository: wso2/identity-apps
Length of output: 136
🏁 Script executed:
#!/bin/bash
# Read the file to understand the component structure and scope
head -100 features/admin.consents.v1/components/policy-consent-applications.tsxRepository: wso2/identity-apps
Length of output: 3544
🏁 Script executed:
#!/bin/bash
# Check git history to see if this is a new file
git log --oneline -n 5 -- features/admin.consents.v1/components/policy-consent-applications.tsx 2>/dev/null || echo "Could not retrieve git history"Repository: wso2/identity-apps
Length of output: 90
🏁 Script executed:
#!/bin/bash
# Read more of the file to see how semantic-ui-react components are used
sed -n '295-320p' features/admin.consents.v1/components/policy-consent-applications.tsx
sed -n '370,380p' features/admin.consents.v1/components/policy-consent-applications.tsx
sed -n '420,430p' features/admin.consents.v1/components/policy-consent-applications.tsx
sed -n '455,540p' features/admin.consents.v1/components/policy-consent-applications.tsxRepository: wso2/identity-apps
Length of output: 5711
🏁 Script executed:
#!/bin/bash
# Let me read the file sections with proper approach
sed -n '295,330p' features/admin.consents.v1/components/policy-consent-applications.tsxRepository: wso2/identity-apps
Length of output: 1261
🏁 Script executed:
#!/bin/bash
# Check if Oxygen UI has Dialog/Modal equivalent by looking at imports in other components
find features -type f -name "*.tsx" -exec grep -l "`@oxygen-ui/react`" {} \; | head -5 | xargs grep -h "Modal\|Dialog" | head -20Repository: wso2/identity-apps
Length of output: 17684
🌐 Web query:
Oxygen UI React Modal Dialog component documentation MUI Material-UI
💡 Result:
Oxygen UI is a design system and React component library developed by WSO2 that is built on top of Material-UI (MUI) [1][2][3]. Because Oxygen UI is designed to be compatible with MUI, its Dialog components function similarly to standard MUI Dialogs [4][2][3]. To use the Dialog component in Oxygen UI, you import it directly from the @wso2/oxygen-ui package [4][2]. Example Usage: import { Dialog, DialogTitle, DialogContent, DialogContentText, DialogActions, Button, } from '@wso2/oxygen-ui'; Dialog Title Dialog content goes here. Cancel Confirm Key Integration Notes: 1. Compatibility: Oxygen UI components, including the Dialog, are built for compatibility with Material-UI v7 [1]. You can often use MUI props and types directly, as the library includes full TypeScript support for both Oxygen UI and MUI components [2][3]. 2. Underlying Technology: Like standard MUI, the Oxygen UI Dialog is a higher-level abstraction built on top of the base Modal component [4][5][6]. 3. Documentation: For advanced customization or prop reference, you can refer to both the WSO2 Oxygen UI documentation (available via their repository or Storybook) and the official Material-UI documentation for the Dialog component, as they share the same underlying API structure [4][7][2][5].
Citations:
- 1: https://github.com/wso2/oxygen-ui
- 2: https://github.com/wso2/oxygen-ui/blob/main/packages/oxygen-ui/README.md
- 3: https://registry.npmjs.org/@wso2/oxygen-ui
- 4: https://raw.githubusercontent.com/wso2/oxygen-ui/main/packages/oxygen-ui/.claude/skills/oxygen-component/SKILL.md
- 5: https://mui.com/material-ui/react-modal/
- 6: https://v7.mui.com/material-ui/react-modal/
- 7: https://cdn.jsdelivr.net/npm/@oxygen-ui/react@2.4.6/esm/types/components/DialogContentText/DialogContentText.d.ts
Migrate Semantic UI React components to Oxygen UI equivalents.
This new component mixes semantic-ui-react imports (Modal, Header, Icon, Divider) with Oxygen UI components. Per coding guidelines, all new components must use Oxygen UI (@oxygen-ui/react) exclusively with MUI's styled API for styling.
Replace the following:
Modal→Dialogfrom@oxygen-ui/reactwithDialogTitle,DialogContent,DialogActionsHeader→ Typography or custom styled componentIcon→ MUI icon component or equivalentDivider→Dividerfrom@oxygen-ui/react
Affects lines 67 (import), 301–316 (Header usage), 375, 423 (Icon usage), 428 (Divider), 460–538 (Modal and subcomponents).
🤖 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 `@features/admin.consents.v1/components/policy-consent-applications.tsx` around
lines 67 - 68, Replace the semantic-ui-react import statement to use Oxygen UI
equivalents exclusively. Remove the imports for Modal, Header, Icon, and Divider
from semantic-ui-react and import Dialog, DialogTitle, DialogContent, and
DialogActions from `@oxygen-ui/react` instead. Then locate all usages of these
components throughout the file: replace Header components (used in the JSX
rendering sections) with a Typography component or styled component from MUI,
replace all Icon usages with MUI icon components, replace the Divider component
import with the Oxygen UI Divider equivalent, and refactor the Modal component
(which wraps content in lines 460-538) to use the Dialog component structure
with DialogTitle, DialogContent, and DialogActions for proper organization.
Source: Coding guidelines
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #10422 +/- ##
==========================================
+ Coverage 72.62% 72.71% +0.08%
==========================================
Files 469 469
Lines 70633 70848 +215
Branches 240 240
==========================================
+ Hits 51300 51515 +215
Misses 19222 19222
Partials 111 111
🚀 New features to boost your workflow:
|
Member
There was a problem hiding this comment.
Hope these scope updates have also been added to the default.json in framework as well
Contributor
Author
There was a problem hiding this comment.
Added in wso2/carbon-identity-framework#8136
| * @param purposeId - The purpose UUID. | ||
| * @returns Array of application IDs, or empty array if the purpose has no assignments. | ||
| */ | ||
| export const getConsentPolicyApps = (purposeId: string): Promise<string[]> => { |
Member
There was a problem hiding this comment.
Let's use custom hooks with SWR for get requests.
Comment on lines
+213
to
+215
| { !isDefaultPolicy && ( | ||
| <Tab label={ t("consents:tabs.applications.label") } /> | ||
| ) } |
Member
There was a problem hiding this comment.
cannot we keep this in a single line and get rid of wrapping parenthesis?
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@features/admin.consents.v1/hooks/use-get-consent-policy-apps.ts`:
- Around line 29-39: The interfaces ApplicationObject and
UseGetConsentPolicyAppsReturn do not follow the repository's TypeScript naming
convention which requires all interface names to have an Interface suffix.
Rename ApplicationObject to ApplicationInterface and
UseGetConsentPolicyAppsReturn to UseGetConsentPolicyAppsReturnInterface to
comply with the coding guidelines. Update all references to these interfaces
throughout the file and any other files that import them.
- Around line 38-39: Rename the interface at line 38-39 from its current name to
UseGetConsentPolicyAppsReturnInterface to follow the coding guidelines requiring
the Interface suffix. Additionally, update the mutate property type from
KeyedMutator<AxiosResponse<ApplicationObject[]>> to
KeyedMutator<ApplicationObject[]> to align with the actual data type returned by
the useRequest hook, which returns unwrapped ApplicationObject[] data (as seen
at line 74), not wrapped in AxiosResponse. This ensures type consistency and
allows consumers to correctly call mutate with ApplicationObject[] arguments.
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: b9d30767-460f-4362-be52-1158e0c5830c
📒 Files selected for processing (5)
apps/console/src/public/deployment.config.jsonfeatures/admin.consents.v1/api/consent-policy-apps.tsfeatures/admin.consents.v1/components/policy-consent-applications.tsxfeatures/admin.consents.v1/hooks/use-get-consent-policy-apps.tsfeatures/admin.consents.v1/pages/policy-consent-edit.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- features/admin.consents.v1/api/consent-policy-apps.ts
- features/admin.consents.v1/components/policy-consent-applications.tsx
- features/admin.consents.v1/pages/policy-consent-edit.tsx
Comment on lines
+29
to
+39
| interface ApplicationObject { | ||
| id: string; | ||
| } | ||
|
|
||
| interface UseGetConsentPolicyAppsReturn { | ||
| data: string[]; | ||
| error: AxiosError<RequestErrorInterface> | undefined; | ||
| isLoading: boolean; | ||
| isValidating: boolean; | ||
| mutate: KeyedMutator<AxiosResponse<ApplicationObject[]>>; | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Rename interfaces to use the required Interface suffix.
ApplicationObject and UseGetConsentPolicyAppsReturn should follow the repository rule for TypeScript interface naming.
As per coding guidelines, Use Interface suffix for all interface names (e.g., ApplicationListInterface, FeatureConfigInterface).
🤖 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 `@features/admin.consents.v1/hooks/use-get-consent-policy-apps.ts` around lines
29 - 39, The interfaces ApplicationObject and UseGetConsentPolicyAppsReturn do
not follow the repository's TypeScript naming convention which requires all
interface names to have an Interface suffix. Rename ApplicationObject to
ApplicationInterface and UseGetConsentPolicyAppsReturn to
UseGetConsentPolicyAppsReturnInterface to comply with the coding guidelines.
Update all references to these interfaces throughout the file and any other
files that import them.
Source: Coding guidelines
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Add application assignment UI for policies
Related issue wso2/product-is#26859