-
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
feat(auth-admin): Delegation ui and gql #15943
Conversation
Fetches Zendesk ticket by id
Form for creating delegation, wip form action
…to feat/delegation-ui-gql
WalkthroughThis pull request introduces a new method for creating delegation administration requests in the Changes
Possibly related PRs
Suggested labels
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
Datadog ReportAll test runs ✅ 4 Total Test Services: 0 Failed, 4 Passed Test Services
🔻 Code Coverage Decreases vs Default Branch (3) |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #15943 +/- ##
==========================================
- Coverage 36.63% 36.63% -0.01%
==========================================
Files 6748 6749 +1
Lines 138831 138852 +21
Branches 39455 39457 +2
==========================================
Hits 50863 50863
- Misses 87968 87989 +21
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
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.
Great job, minor comments .. One thing this PR is missing tho. According to the design there is also a button to create a new delegation from '/delegation-admin/:nationalId'
. That button should open this page with the toNationalId preffiled.
libs/api/domains/auth-admin/src/lib/delegationAdmin/delegation-admin.service.ts
Show resolved
Hide resolved
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: 1
Outside diff range and nitpick comments (2)
libs/portals/admin/delegation-admin/src/screens/CreateDelegation/CreateDelegation.action.ts (1)
1-93
: Partial adherence to additional instructions.The file partially adheres to the additional instructions:
- It uses TypeScript for defining types and exporting them.
- However, it does not contain any reusable components or hooks.
- The instructions related to tree-shaking and bundling are not applicable to this specific file.
libs/portals/admin/delegation-admin/src/screens/CreateDelegation/CreateDelegation.tsx (1)
36-41
: Consider using dynamic options for thetype
field.The
type
field currently uses hardcoded options defined in theaccessTypeOptions
array. If these options are meant to be dynamic and fetched from an API or configuration, consider updating the implementation to load the options dynamically.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (11)
- libs/api/domains/auth-admin/src/lib/delegationAdmin/delegation-admin.resolver.ts (2 hunks)
- libs/api/domains/auth-admin/src/lib/delegationAdmin/delegation-admin.service.ts (2 hunks)
- libs/api/domains/auth-admin/src/lib/delegationAdmin/dto/createDelegation.input.ts (1 hunks)
- libs/clients/zendesk/src/lib/zendesk.service.ts (4 hunks)
- libs/portals/admin/delegation-admin/src/lib/messages.ts (1 hunks)
- libs/portals/admin/delegation-admin/src/lib/paths.ts (1 hunks)
- libs/portals/admin/delegation-admin/src/module.tsx (2 hunks)
- libs/portals/admin/delegation-admin/src/screens/CreateDelegation/CreateDelegation.action.ts (1 hunks)
- libs/portals/admin/delegation-admin/src/screens/CreateDelegation/CreateDelegation.graphql (1 hunks)
- libs/portals/admin/delegation-admin/src/screens/CreateDelegation/CreateDelegation.tsx (1 hunks)
- libs/portals/admin/delegation-admin/src/screens/Root.tsx (3 hunks)
Additional context used
Path-based instructions (11)
libs/portals/admin/delegation-admin/src/screens/CreateDelegation/CreateDelegation.graphql (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/portals/admin/delegation-admin/src/lib/paths.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/api/domains/auth-admin/src/lib/delegationAdmin/dto/createDelegation.input.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/api/domains/auth-admin/src/lib/delegationAdmin/delegation-admin.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."
libs/portals/admin/delegation-admin/src/module.tsx (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/portals/admin/delegation-admin/src/screens/CreateDelegation/CreateDelegation.action.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/portals/admin/delegation-admin/src/screens/Root.tsx (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/api/domains/auth-admin/src/lib/delegationAdmin/delegation-admin.resolver.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/clients/zendesk/src/lib/zendesk.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."
libs/portals/admin/delegation-admin/src/lib/messages.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/portals/admin/delegation-admin/src/screens/CreateDelegation/CreateDelegation.tsx (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."
Additional comments not posted (28)
libs/portals/admin/delegation-admin/src/screens/CreateDelegation/CreateDelegation.graphql (2)
1-5
: LGTM!The GraphQL mutation definition for creating a delegation looks good and follows the correct syntax.
1-5
: Verify the input type and mutation implementation.Please ensure the following:
- The
CreateDelegationInput
input type is properly defined with all the necessary fields.- The
authCreateDelegation
mutation is implemented correctly in the backend to handle the delegation creation.- Consider returning additional fields besides the
id
if needed by the client.libs/portals/admin/delegation-admin/src/lib/paths.ts (1)
4-4
: LGTM!The code changes are approved.
The new path
CreateDelegation
is correctly defined in theDelegationAdminPaths
enum and follows the existing naming convention.Ensure that the corresponding route handler and UI components are implemented for this path, and the path is correctly used for navigation within the application.
Also, 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/api/domains/auth-admin/src/lib/delegationAdmin/dto/createDelegation.input.ts (1)
1-25
: LGTM!The
CreateDelegationInput
class is well-defined and follows best practices:
- It uses TypeScript for defining the class and its fields, ensuring type safety.
- It leverages NestJS's
@InputType
decorator to mark the class as a GraphQL input type, making it reusable in GraphQL mutations.- It utilizes
class-validator
decorators for input validation, ensuring data integrity.- The fields are appropriately typed and marked as required or optional, providing clear expectations for the input data.
The code is clean, readable, and adheres to the project's coding standards.
libs/api/domains/auth-admin/src/lib/delegationAdmin/delegation-admin.service.ts (2)
44-44
: LGTM: The method signature and return type are appropriate.The
createDelegationAdmin
method has the correct signature and return type for handling the creation of delegation administration requests.
45-45
: Return only national IDs forto
andfrom
fields.As mentioned in the previous comment, the
createDelegationAdmin
method should only return the national IDs for theto
andfrom
fields, and let the field resolvers resolve them to the actual identities.Modify the mock response to return only the national IDs:
- from: { - name: 'foo', - nationalId: '221290-22169', - type: 'company', - }, + from: '221290-22169', - to: { - name: 'bar', - nationalId: '221290-22169', - type: 'company', - }, + to: '221290-22169',Apply the same change when implementing the actual logic for creating a delegation.
libs/portals/admin/delegation-admin/src/module.tsx (2)
13-15
: LGTM!The code segment adheres to the lazy loading pattern for optimized performance and the component is properly typed as a React component.
44-49
: LGTM, but verify the reusability of thecreateDelegationAction
andCreateDelegationScreen
components.The route is properly defined with a name, path, element, and action. It enhances the module's capabilities by allowing delegation creation directly from the admin interface.
However, please ensure that the
createDelegationAction
andCreateDelegationScreen
components are reusable across different NextJS apps and adhere to effective tree-shaking and bundling practices.libs/portals/admin/delegation-admin/src/screens/CreateDelegation/CreateDelegation.action.ts (3)
17-47
: LGTM!The schema definition looks good and follows best practices:
- Using the
zod
library for validation is a good choice.- The fields are properly typed and validated.
- The
kennitala
library is used for validating Icelandic national IDs.- The
validTo
field is correctly validated to be a future date.
49-54
: LGTM!The
CreateDelegationResult
type definition is correct and follows the expected structure.
56-93
: LGTM!The
createDelegationAction
function implementation looks good and follows best practices:
- It properly validates the form data using the defined schema.
- It handles both success and failure cases.
- It uses a GraphQL mutation to create the delegation.
- It redirects on success and returns an error object on failure.
libs/portals/admin/delegation-admin/src/screens/Root.tsx (5)
4-10
: LGTM!The new imports are necessary for the added functionality and layout changes.
14-14
: LGTM!The new import is necessary for the added navigation functionality.
23-23
: LGTM!The
navigate
constant is necessary for the added navigation functionality.
43-85
: LGTM, but verify the admin access.The layout and functionality changes are approved:
- The new grid layout improves the responsiveness and organization of the UI elements.
- The new
Button
component enhances the user interaction by providing a clear action for creating new delegations.- The form changes ensure it fits within the new grid structure.
As mentioned in the previous comment, ensure that the delegation creation functionality is only available to users with the admin scope for delegations (
@admin.island.is/delegation-system:admin
). You can use theuseAuth
hook to check the user's scopes:import { AdminPortalScope } from '@island.is/auth/scopes' const { userInfo } = useAuth() const hasAdminAccess = userInfo?.scopes.includes( AdminPortalScope.delegationSystemAdmin, )Then, conditionally render the
Button
component based on thehasAdminAccess
variable.
Line range hint
1-90
: LGTM!The component adheres to the following guidelines:
- Reusability: The component imports and uses reusable components and hooks from other libraries.
- TypeScript usage: The component uses TypeScript for defining props and types.
- Bundling practices: The component follows effective tree-shaking and bundling practices by importing only the necessary components and hooks.
libs/api/domains/auth-admin/src/lib/delegationAdmin/delegation-admin.resolver.ts (1)
70-76
: LGTM!The
createDelegationSystem
mutation method in theDelegationAdminResolver
class is implemented correctly:
- It is annotated with the
@Mutation
decorator, indicating it's a GraphQL mutation.- It accepts the
input
parameter of typeCreateDelegationInput
for type safety and validation.- It retrieves the authenticated
user
using the@CurrentUser()
decorator, ensuring only authorized users can create delegations.- It delegates the creation logic to the
createDelegationAdmin
service method, following the single responsibility principle.The code changes are approved.
libs/clients/zendesk/src/lib/zendesk.service.ts (4)
Line range hint
8-13
: LGTM!The
SubmitTicketInput
type is well-defined and follows TypeScript best practices by clearly specifying optional and required properties.
21-24
: LGTM!The
Ticket
type is well-defined and follows TypeScript best practices by clearly specifying the properties.
129-129
: LGTM!The
submitTicket
method is correctly using the updatedSubmitTicketInput
type for its parameter.
155-170
: LGTM!The
getTicket
method is well-implemented and follows best practices:
- It uses the
Ticket
type for the return value, ensuring type safety.- It handles errors appropriately by logging them and throwing a new error with a descriptive message.
- It uses async/await syntax for better readability and error handling.
libs/portals/admin/delegation-admin/src/lib/messages.ts (4)
44-47
: The comment from the previous review is still valid:This already exists here above, but feel free to remove that since it is not being used
48-51
: LGTM!The message definition for
delegationAdminCreateNewDelegationDescription
looks good.
52-55
: The comment from the previous review is still valid:re-use the previous variable for "Stofn nytt umbod" since these values will always be the same
56-119
: LGTM!The added message definitions from line 56 to 119 look good. They cover various aspects of the delegation management functionality and provide clear and concise descriptions. The messages follow a consistent naming convention and enhance the user experience by providing localized labels, placeholders, and error messages.
libs/portals/admin/delegation-admin/src/screens/CreateDelegation/CreateDelegation.tsx (3)
1-25
: Imports look good!The imports are well-organized, follow a consistent pattern, and only include what's necessary for the component.
26-183
: Component structure and implementation look solid!The
CreateDelegationScreen
component is well-structured and follows good practices:
- It uses a clear layout with Grid components.
- It handles form state using React hooks.
- It delegates form submission to an action file.
- It uses localized messages for labels and error messages.
- It conditionally renders fields based on state.
- It uses
InputMask
for formatting national IDs.- It displays a global error message on submission failure.
Overall, the component is implemented in a clean and maintainable way.
51-179
: Verify error handling and field-specific error messages.The component displays a global error message if the form submission fails, which is good. However, it's important to ensure that field-specific error messages are properly displayed for each form field.
Please verify that the
errorMessage
prop is correctly set for each form field and that the corresponding error messages are displayed when theactionData
contains field-specific errors.
libs/api/domains/auth-admin/src/lib/delegationAdmin/delegation-admin.service.ts
Outdated
Show resolved
Hide resolved
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.
Great job, minor comments, mostly nit
libs/portals/admin/delegation-admin/src/screens/CreateDelegation/CreateDelegation.tsx
Show resolved
Hide resolved
libs/portals/admin/delegation-admin/src/screens/CreateDelegation/CreateDelegation.tsx
Show resolved
Hide resolved
libs/portals/admin/delegation-admin/src/screens/DelegationAdminDetails/DelegationAdmin.tsx
Show resolved
Hide resolved
libs/portals/admin/delegation-admin/src/screens/DelegationAdminDetails/DelegationAdmin.tsx
Show resolved
Hide resolved
libs/portals/admin/delegation-admin/src/screens/CreateDelegation/CreateDelegation.tsx
Outdated
Show resolved
Hide resolved
libs/portals/admin/delegation-admin/src/screens/CreateDelegation/CreateDelegation.tsx
Outdated
Show resolved
Hide resolved
libs/portals/admin/delegation-admin/src/screens/CreateDelegation/CreateDelegation.tsx
Outdated
Show resolved
Hide resolved
…is into feat/delegation-ui-gql
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
Ui for creating a custom delegation
Function for getting a Zendesk ticket by id
Graphql mution for creating a delegation
Why
For admin to handle creating a custom delegation when received on paper
Screenshots / Gifs
Checklist:
Summary by CodeRabbit
New Features
User Experience Improvements