-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(auth-admin-web): remove delegation controls from old ids admin for client and api-scope #14999
Conversation
WalkthroughThe updates primarily focus on simplifying the user interface and refactoring DTOs. In the UI, multiple checkbox fields have been replaced with links to manage settings in a new IDS Admin. DTOs have been restructured for better code maintainability, with specific properties being excluded from certain classes. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range and nitpick comments (2)
apps/auth-admin-web/components/Resource/forms/ApiScopeCreateForm.tsx (1)
Line range hint
61-61
: Consider simplifying the computed expressions as suggested by the static analysis tool. This can enhance code readability and performance.- // Example for one of the lines: - const isValid = name.length > 0 ? ValidationUtils.validateApiScope(name) : false + const isValid = !!name.length && ValidationUtils.validateApiScope(name)Also applies to: 87-87, 88-88, 132-132, 148-148, 150-150, 155-155, 156-156, 167-167, 168-168, 170-170, 175-175, 183-183, 194-194, 195-195, 198-198, 204-204, 218-218, 229-229, 230-230
apps/auth-admin-web/components/Client/form/ClientCreateForm.tsx (1)
Line range hint
45-45
: Use self-closing tags for JSX elements without children.- const [clientTypeInfo, setClientTypeInfo] = useState<JSX.Element>(<div></div>) + const [clientTypeInfo, setClientTypeInfo] = useState<JSX.Element>(<div />)This change makes the JSX cleaner and is a best practice in React development.
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (9)
- apps/auth-admin-web/components/Client/form/ClientCreateForm.tsx (1 hunks)
- apps/auth-admin-web/components/Resource/forms/ApiScopeCreateForm.tsx (1 hunks)
- apps/services/auth/admin-api/src/app/modules/resources/resources.controller.ts (4 hunks)
- libs/auth-api-lib/src/index.ts (1 hunks)
- libs/auth-api-lib/src/lib/clients/dto/base/client-base.dto.ts (1 hunks)
- libs/auth-api-lib/src/lib/clients/dto/client-update.dto.ts (1 hunks)
- libs/auth-api-lib/src/lib/clients/dto/client.dto.ts (1 hunks)
- libs/auth-api-lib/src/lib/resources/dto/api-scope-create.dto.ts (1 hunks)
- libs/auth-api-lib/src/lib/resources/resources.service.ts (4 hunks)
Files skipped from review due to trivial changes (1)
- libs/auth-api-lib/src/lib/clients/dto/base/client-base.dto.ts
Additional Context Used
Biome (80)
apps/auth-admin-web/components/Client/form/ClientCreateForm.tsx (20)
45-45: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
70-70: The computed expression can be simplified without the use of a string literal.
71-71: The computed expression can be simplified without the use of a string literal.
108-108: Change to an optional chain.
199-199: The computed expression can be simplified without the use of a string literal.
205-205: The computed expression can be simplified without the use of a string literal.
210-210: The computed expression can be simplified without the use of a string literal.
213-213: The computed expression can be simplified without the use of a string literal.
266-266: The computed expression can be simplified without the use of a string literal.
275-275: The computed expression can be simplified without the use of a string literal.
280-280: The computed expression can be simplified without the use of a string literal.
289-289: The computed expression can be simplified without the use of a string literal.
289-289: The computed expression can be simplified without the use of a string literal.
298-298: The computed expression can be simplified without the use of a string literal.
299-299: The computed expression can be simplified without the use of a string literal.
308-308: The computed expression can be simplified without the use of a string literal.
308-308: The computed expression can be simplified without the use of a string literal.
317-317: The computed expression can be simplified without the use of a string literal.
318-318: The computed expression can be simplified without the use of a string literal.
328-328: The computed expression can be simplified without the use of a string literal.
apps/auth-admin-web/components/Resource/forms/ApiScopeCreateForm.tsx (20)
61-61: Change to an optional chain.
87-87: The computed expression can be simplified without the use of a string literal.
88-88: The computed expression can be simplified without the use of a string literal.
132-132: The computed expression can be simplified without the use of a string literal.
148-148: The computed expression can be simplified without the use of a string literal.
150-150: The computed expression can be simplified without the use of a string literal.
155-155: The computed expression can be simplified without the use of a string literal.
156-156: The computed expression can be simplified without the use of a string literal.
167-167: The computed expression can be simplified without the use of a string literal.
168-168: The computed expression can be simplified without the use of a string literal.
170-170: The computed expression can be simplified without the use of a string literal.
175-175: The computed expression can be simplified without the use of a string literal.
183-183: The computed expression can be simplified without the use of a string literal.
194-194: The computed expression can be simplified without the use of a string literal.
195-195: The computed expression can be simplified without the use of a string literal.
198-198: The computed expression can be simplified without the use of a string literal.
204-204: The computed expression can be simplified without the use of a string literal.
218-218: The computed expression can be simplified without the use of a string literal.
229-229: The computed expression can be simplified without the use of a string literal.
230-230: The computed expression can be simplified without the use of a string literal.
apps/services/auth/admin-api/src/app/modules/resources/resources.controller.ts (20)
95-95: Decorators are not valid here.
96-96: Decorators are not valid here.
97-97: Decorators are not valid here.
134-134: Decorators are not valid here.
135-135: Decorators are not valid here.
136-136: Decorators are not valid here.
185-185: Decorators are not valid here.
186-186: Decorators are not valid here.
187-187: Decorators are not valid here.
188-188: Decorators are not valid here.
226-226: Decorators are not valid here.
242-242: Decorators are not valid here.
259-259: Decorators are not valid here.
260-260: Decorators are not valid here.
285-285: Decorators are not valid here.
303-303: Decorators are not valid here.
313-313: Decorators are not valid here.
314-314: Decorators are not valid here.
315-315: Decorators are not valid here.
337-337: Decorators are not valid here.
libs/auth-api-lib/src/lib/resources/resources.service.ts (20)
43-43: Decorators are not valid here.
45-45: Decorators are not valid here.
47-47: Decorators are not valid here.
49-49: Decorators are not valid here.
51-51: Decorators are not valid here.
53-53: Decorators are not valid here.
55-55: Decorators are not valid here.
57-57: Decorators are not valid here.
59-59: Decorators are not valid here.
61-61: Decorators are not valid here.
63-63: Decorators are not valid here.
65-65: Decorators are not valid here.
67-67: Decorators are not valid here.
113-120: This else clause can be omitted because previous branches break early.
302-302: Unexpected any. Specify a different type.
302-302: Unexpected any. Specify a different type.
315-315: Unexpected any. Specify a different type.
336-336: Do not use template literals if interpolation and special-character handling are not needed.
359-359: Do not use template literals if interpolation and special-character handling are not needed.
409-409: Unexpected any. Specify a different type.
Path-based Instructions (8)
libs/auth-api-lib/src/lib/clients/dto/client-update.dto.ts (1)
Pattern
libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/auth-api-lib/src/lib/clients/dto/client.dto.ts (1)
Pattern
libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/auth-api-lib/src/lib/resources/dto/api-scope-create.dto.ts (1)
Pattern
libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/auth-api-lib/src/index.ts (1)
Pattern
libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
apps/auth-admin-web/components/Resource/forms/ApiScopeCreateForm.tsx (1)
Pattern
apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
libs/auth-api-lib/src/lib/resources/resources.service.ts (1)
Pattern
libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
apps/services/auth/admin-api/src/app/modules/resources/resources.controller.ts (2)
Pattern
apps/services/**/*
: "Confirm that the code adheres to the following:
- NestJS architecture, including modules, services, and controllers.
- Dependency injection patterns and service encapsulation.
- Integration and unit testing coverage and practices."
Pattern
apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/auth-admin-web/components/Client/form/ClientCreateForm.tsx (1)
Pattern
apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
Additional comments not posted (15)
libs/auth-api-lib/src/lib/clients/dto/client-update.dto.ts (1)
4-11
: The refactoring ofClientUpdateDTO
to exclude certain properties usingOmitType
aligns with the PR's goal to simplify the client DTO structure. This change ensures that the DTO only includes relevant fields, enhancing clarity and maintainability.libs/auth-api-lib/src/lib/clients/dto/client.dto.ts (1)
5-12
: The update inClientDTO
to exclude specific delegation-related properties usingOmitType
is a significant improvement. It simplifies the DTO structure and focuses on the essential attributes, which is crucial for the new administrative requirements.libs/auth-api-lib/src/lib/resources/dto/api-scope-create.dto.ts (1)
5-27
: The creation ofCreateApiScopeDTO
with specific properties and validation rules is a positive change. It ensures that the necessary data for API scopes is validated at the DTO level, which enhances data integrity and aligns with best practices in API design.libs/auth-api-lib/src/index.ts (1)
71-71
: The addition ofapi-scope-create.dto
to the exports inindex.ts
is crucial for making it accessible across the application. This change supports better modularity and reusability of the DTO.apps/auth-admin-web/components/Resource/forms/ApiScopeCreateForm.tsx (1)
452-461
: The addition of a paragraph with a link to the new IDS Admin is a user-friendly way to direct users to the updated management interface. This change is in line with the PR's objectives to phase out old delegation controls and should improve the user experience by guiding them to the new system.libs/auth-api-lib/src/lib/resources/resources.service.ts (3)
39-39
: Ensure that theCreateApiScopeDTO
is properly utilized in the methods whereApiScopeDTO
was previously used.
520-520
: The methodcreateApiScope
correctly uses the newCreateApiScopeDTO
. Ensure that theassertSameAsGroup
method is appropriately validating the new DTO fields.
530-530
: TheupdateApiScope
method has been updated to useCreateApiScopeDTO
, which is consistent with the changes in thecreateApiScope
method. This should ensure that the API scope updates are handled with the new DTO structure.
[APROVED]apps/services/auth/admin-api/src/app/modules/resources/resources.controller.ts (4)
21-21
: The import ofCreateApiScopeDTO
is correctly added to support the new DTO structure for API scope creation.
437-437
: The methodcreateApiScope
now correctly acceptsCreateApiScopeDTO
instead of the previousApiScopeDTO
. This change aligns with the updated DTO structure and should help ensure that the API scope creation process adheres to the new data requirements.
459-459
: The methodupdateApiScope
has been updated to acceptCreateApiScopeDTO
instead ofApiScopeDTO
. This change is consistent with the new DTO structure and ensures that the API scope update process is aligned with the latest data requirements.
Line range hint
95-95
: The static analysis tool has flagged several lines with the error "Decorators are not valid here." Please verify these lines to ensure that decorators are used correctly in the context of NestJS, which may involve checking for misplaced decorators or syntax errors.Also applies to: 96-96, 97-97, 134-134, 135-135, 136-136, 185-185, 186-186, 187-187, 188-188, 226-226, 242-242, 259-259, 260-260, 285-285, 303-303, 313-313, 314-314, 315-315, 337-337
Verification successful
The decorators in the file
apps/services/auth/admin-api/src/app/modules/resources/resources.controller.ts
appear to be used correctly within the context of NestJS. The decorators such as@UseGuards
,@ApiExcludeController
,@Controller
,@Audit
,@Scopes
,@Get
,@ApiQuery
,@ApiOkResponse
,@Post
,@ApiCreatedResponse
,@Put
, and@Delete
are all valid NestJS decorators and are placed appropriately above class methods and parameters.
- The
@Query
decorator is used correctly to extract query parameters.- The
@Param
decorator is used correctly to extract route parameters.- The
@Body
decorator is used correctly to extract the body of the request.- The
@CurrentUser
decorator is used correctly to extract the current user from the request.No syntax errors or misplaced decorators were found in the specified lines.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct usage of decorators in the specified lines. # Test: Search for decorator usage errors. Expect: No syntax errors or misplaced decorators. rg --type python -C 3 '^\s*@' apps/services/auth/admin-api/src/app/modules/resources/resources.controller.tsLength of output: 141
Script:
#!/bin/bash # Description: Verify the correct usage of decorators in the specified lines. # Test: Search for decorator usage errors. Expect: No syntax errors or misplaced decorators. rg --type ts -C 3 '^\s*@' apps/services/auth/admin-api/src/app/modules/resources/resources.controller.tsLength of output: 21476
apps/auth-admin-web/components/Client/form/ClientCreateForm.tsx (3)
715-724
: Update the link to the new IDS Admin.The change aligns with the PR's objective to direct users to the new admin portal for delegation management. Ensure the URL is correct and accessible.
Line range hint
70-70
: Simplify computed expressions in JSX.
[REFACTOR_Suggestion]- className={`detail-flow${localization.fields['clientType'].selectItems[clientType].flow ? ' show' : ' hidden'}`} + className={`detail-flow ${localization.fields['clientType'].selectItems[clientType].flow ? 'show' : 'hidden'}`}This change removes unnecessary string literals and simplifies the expression, making the code cleaner and easier to read.
Also applies to: 71-71, 199-199, 205-205, 210-210, 213-213, 266-266, 275-275, 280-280, 289-289, 298-298, 299-299, 308-308, 317-317, 318-318, 328-328
Line range hint
108-108
: Use optional chaining for safer property access.
[REFACTOR_Suggestion]- if (response) { + if (response?.length > 0) {This change ensures that the code does not throw an error if
response
is undefined, improving the robustness of the application.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14999 +/- ##
==========================================
- Coverage 37.13% 37.12% -0.01%
==========================================
Files 6391 6391
Lines 130067 130059 -8
Branches 37108 37106 -2
==========================================
- Hits 48294 48288 -6
+ Misses 81773 81771 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 14 Total Test Services: 0 Failed, 14 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
☝️
What
Deprecate delegation management for Clients and api scopes in the old ids-admin
Why
We are refactoring DelegationProviders and DelegationTypes and the changes are not compatible with the old ids-admin ui. Instead of changing it there we will point users to the new admin portal instead.
Screenshots / Gifs
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
Style
Tests
Chores
Revert