feat(ojoi): Add involved party to #16200
Conversation
WalkthroughThe changes introduce new TypeScript models and a React component to manage user-involved parties in the Official Journal of Iceland application. A GraphQL response type for involved parties is defined, alongside a service method to fetch these parties. The new React component handles the display and selection of involved parties, incorporating state management and error handling. These updates facilitate user interactions and improve the application's ability to present user-specific data. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Outside diff range and nitpick comments (11)
libs/api/domains/official-journal-of-iceland-application/src/models/getUserInvolvedParties.input.ts (2)
5-6: LGTM: Field declaration is correct. Consider adding a comment for the definite assignment assertion.The
@Fielddecorator is correctly used withIDtype, which is appropriate for an identifier. The field name and type are well-chosen. The use of the definite assignment assertion (!) ensures the field is always initialized, which is good for type safety.Consider adding a brief comment explaining why the definite assignment assertion is used, to improve code readability:
@Field(() => ID) applicationId!: string; // Non-nullable: must be provided when creating an instance
1-7: LGTM: Code adheres to coding guidelines. Consider adding a brief class description.The code follows the guidelines for
libs/**/*files:
- It defines a reusable input type that can be used across different NextJS apps.
- TypeScript is effectively used for defining and exporting the type.
- The single-responsibility nature of this class supports effective tree-shaking and bundling.
To further improve documentation, consider adding a brief class description using JSDoc:
/** * Input type for retrieving user involved parties for a specific application. */ @InputType('GetUserInvolvedPartiesInput') export class GetUserInvolvedPartiesInput { // ... existing code }libs/application/templates/official-journal-of-iceland/src/hooks/useInvolvedParties.ts (2)
1-13: LGTM! Consider exporting types for reusability.The imports and type definitions are well-structured and use TypeScript effectively. This adheres to the coding guidelines for TypeScript usage in defining props and types.
Consider exporting the
PropsandInvolvedPartiesResponsetypes to enhance reusability across different components or hooks that might need these types:-type Props = { +export type Props = { applicationId?: string onComplete?: (data: InvolvedPartiesResponse) => void onError?: (error: Error) => void } -type InvolvedPartiesResponse = { +export type InvolvedPartiesResponse = { officialJournalOfIcelandApplicationGetUserInvolvedParties: OfficialJournalOfIcelandApplicationGetUserInvolvedPartiesResponse }
14-36: LGTM! Consider optimizing callback invocations.The hook is well-structured and follows React best practices. The query setup with Apollo Client is correct and includes proper error handling.
Consider optimizing the callback invocations using optional chaining as suggested by the static analysis tool:
- onCompleted: (data) => { - onComplete && onComplete(data) - }, + onCompleted: (data) => { + onComplete?.(data) + }, - onError: (error) => { - onError && onError(error) - }, + onError: (error) => { + onError?.(error) + },This change improves code readability and follows modern JavaScript best practices.
🧰 Tools
🪛 Biome
[error] 30-30: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 33-33: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
libs/application/templates/official-journal-of-iceland/src/components/form/FormScreen.tsx (1)
68-77: LGTM: Render logic updated with loading state handling.The conditional rendering for the loading state is well-implemented, providing clear visual feedback using the SkeletonLoader when loading is true. This change aligns with the PR objective and maintains component reusability.
Consider handling the case when the
loadingprop is undefined to ensure consistent behavior:- {loading ? ( + {loading === true ? ( <SkeletonLoader height={OJOI_INPUT_HEIGHT} repeat={3} borderRadius="standard" space={2} /> ) : ( <Box className={styles.childrenWrapper}>{children}</Box> )}This change ensures that the children are rendered when
loadingis undefined, maintaining backwards compatibility if the prop is not provided.libs/application/templates/official-journal-of-iceland/src/lib/messages/involved-parties.ts (3)
4-21: LGTM: Well-structured general messages with a minor suggestion.The
generalsection is well-organized with clear identifiers, default messages, and helpful descriptions. This structure promotes maintainability and ease of translation.Consider adding a period at the end of the
titledefault message for consistency with other messages:- defaultMessage: 'Stofnun ', + defaultMessage: 'Stofnun.',
22-30: LGTM: Input messages are correctly defined.The
inputssection is properly structured and follows the same pattern as thegeneralsection, maintaining consistency throughout the file.As the application grows, consider:
- Adding more input-related messages (e.g., labels, help texts) to this section.
- Creating a separate file for input-related messages if they become numerous, to improve maintainability.
32-56: LGTM: Comprehensive error messages with a minor suggestion for consistency.The
errorssection is well-structured and covers various error scenarios, providing clear information to users. The messages follow the same pattern as other sections, maintaining consistency throughout the file.For consistency with other message objects, consider wrapping the
errorsobject withdefineMessages:- errors: defineMessages({ + errors: { + ...defineMessages({ // ... existing error messages ... + }), }),This change would make the
errorsobject structure consistent withgeneralandinputs.libs/application/templates/official-journal-of-iceland/src/lib/types.ts (1)
22-22: LGTM! Consider adding a comment for clarity.The addition of the
involvedPartyIdfield is consistent with the existing code structure and aligns with the PR objective. It maintains type safety and follows the established naming conventions.Consider adding a brief comment explaining the purpose of this new field for better documentation. For example:
// Identifier for the involved party associated with the advertisement involvedPartyId: 'advert.involvedPartyId',libs/application/templates/official-journal-of-iceland/src/forms/Requirements.ts (1)
Line range hint
43-67: LGTM: New section for Involved Party is well-structured.The new section for Involved Party is correctly implemented and follows the existing pattern in the form. It uses appropriate building blocks (
buildSection,buildMultiField,buildCustomField) and includes the customInvolvedPartyScreencomponent, which aligns with the PR objective.Consider moving the submit field (
buildSubmitField) outside of thebuildMultiFieldto maintain consistency with other sections and potentially improve the form flow. This would involve adjusting the structure slightly:buildSection({ id: Routes.INVOLVED_PARTY, title: involvedParty.general.section, children: [ buildMultiField({ id: Routes.INVOLVED_PARTY, title: '', children: [ buildCustomField({ id: 'involvedParty', title: '', component: 'InvolvedPartyScreen', }), ], }), buildSubmitField({ id: 'toComments', title: '', refetchApplicationAfterSubmit: true, actions: [ { event: DefaultEvents.SUBMIT, name: general.continue, type: 'primary', }, ], }), ], }),This structure would be more consistent with other sections in the form.
libs/api/domains/official-journal-of-iceland-application/src/lib/ojoiApplication.resolver.ts (1)
116-125: LGTM: New query method is well-implemented.The
getUserInvolvedPartiesmethod is correctly implemented as a GraphQL query. It follows the structure of other methods in the file, uses appropriate decorators, and maintains separation of concerns by delegating to the service layer.For consistency with other methods in this file, consider using an object literal for the
@Querydecorator options:@Query(() => GetUserInvolvedPartiesResponse, { name: 'officialJournalOfIcelandApplicationGetUserInvolvedParties', })
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (16)
- libs/api/domains/official-journal-of-iceland-application/src/lib/ojoiApplication.resolver.ts (2 hunks)
- libs/api/domains/official-journal-of-iceland-application/src/lib/ojoiApplication.service.ts (2 hunks)
- libs/api/domains/official-journal-of-iceland-application/src/models/getUserInvolvedParties.input.ts (1 hunks)
- libs/api/domains/official-journal-of-iceland-application/src/models/getUserInvolvedParties.response.ts (1 hunks)
- libs/application/templates/official-journal-of-iceland/src/components/form/FormScreen.tsx (4 hunks)
- libs/application/templates/official-journal-of-iceland/src/forms/Requirements.ts (2 hunks)
- libs/application/templates/official-journal-of-iceland/src/graphql/queries.ts (1 hunks)
- libs/application/templates/official-journal-of-iceland/src/hooks/useInvolvedParties.ts (1 hunks)
- libs/application/templates/official-journal-of-iceland/src/lib/constants.ts (1 hunks)
- libs/application/templates/official-journal-of-iceland/src/lib/messages/index.ts (1 hunks)
- libs/application/templates/official-journal-of-iceland/src/lib/messages/involved-parties.ts (1 hunks)
- libs/application/templates/official-journal-of-iceland/src/lib/types.ts (1 hunks)
- libs/application/templates/official-journal-of-iceland/src/screens/InvolvedPartyScreen.tsx (1 hunks)
- libs/application/templates/official-journal-of-iceland/src/screens/index.ts (1 hunks)
- libs/clients/official-journal-of-iceland/application/src/clientConfig.json (4 hunks)
- libs/clients/official-journal-of-iceland/application/src/lib/ojoiApplicationClient.service.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (16)
libs/api/domains/official-journal-of-iceland-application/src/lib/ojoiApplication.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/api/domains/official-journal-of-iceland-application/src/lib/ojoiApplication.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/api/domains/official-journal-of-iceland-application/src/models/getUserInvolvedParties.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/official-journal-of-iceland-application/src/models/getUserInvolvedParties.response.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/application/templates/official-journal-of-iceland/src/components/form/FormScreen.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/application/templates/official-journal-of-iceland/src/forms/Requirements.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/application/templates/official-journal-of-iceland/src/graphql/queries.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/application/templates/official-journal-of-iceland/src/hooks/useInvolvedParties.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/application/templates/official-journal-of-iceland/src/lib/constants.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/application/templates/official-journal-of-iceland/src/lib/messages/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."
libs/application/templates/official-journal-of-iceland/src/lib/messages/involved-parties.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/application/templates/official-journal-of-iceland/src/lib/types.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/application/templates/official-journal-of-iceland/src/screens/InvolvedPartyScreen.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/application/templates/official-journal-of-iceland/src/screens/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."
libs/clients/official-journal-of-iceland/application/src/clientConfig.json (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/official-journal-of-iceland/application/src/lib/ojoiApplicationClient.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."
🪛 Biome
libs/application/templates/official-journal-of-iceland/src/hooks/useInvolvedParties.ts
[error] 30-30: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 33-33: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
libs/application/templates/official-journal-of-iceland/src/screens/InvolvedPartyScreen.tsx
[error] 25-25: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 33-33: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 101-101: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (30)
libs/api/domains/official-journal-of-iceland-application/src/models/getUserInvolvedParties.input.ts (2)
1-1: LGTM: Import statement is correct and follows best practices.The import statement correctly imports the necessary decorators from '@nestjs/graphql' and follows best practices by using destructuring.
3-4: LGTM: Class declaration and decorator are well-implemented.The
@InputTypedecorator is correctly used with an explicit name, which is a good practice. The class name is descriptive, follows PascalCase convention, and is properly exported for use in other parts of the application.libs/application/templates/official-journal-of-iceland/src/screens/index.ts (1)
10-10: LGTM! Verify the existence of InvolvedPartyScreen.The addition of the
InvolvedPartyScreenexport aligns with the PR objectives and follows the established pattern in the file. This change enhances reusability across NextJS apps and supports effective tree-shaking.Let's verify the existence of the
InvolvedPartyScreenfile:✅ Verification successful
Verified: 'InvolvedPartyScreen.tsx' exists in the specified path.
The new export aligns with the PR objectives and adheres to the established file patterns. No issues found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the existence of InvolvedPartyScreen.ts or InvolvedPartyScreen.tsx fd -e ts -e tsx InvolvedPartyScreen libs/application/templates/official-journal-of-iceland/src/screensLength of output: 195
libs/application/templates/official-journal-of-iceland/src/lib/messages/index.ts (1)
14-14: LGTM! Verify the new module.The addition of the 'involved-parties' export aligns with the PR objectives and follows the existing pattern in the file. This change promotes reusability and allows for effective tree-shaking, which is in line with our coding guidelines.
Let's verify the existence of the new module:
✅ Verification successful
Verified the 'involved-parties' module exists and exports correctly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the 'involved-parties' module # Test: Check if the 'involved-parties.ts' file exists fd -p "libs/application/templates/official-journal-of-iceland/src/lib/messages" -t f "involved-parties.ts" # Test: Ensure the new module exports something rg -t typescript "export" "libs/application/templates/official-journal-of-iceland/src/lib/messages/involved-parties.ts"Length of output: 364
Script:
#!/bin/bash # Description: Corrected script to verify the existence and exports of the 'involved-parties' module # Test: Check if the 'involved-parties.ts' file exists fd -t f "involved-parties.ts" "libs/application/templates/official-journal-of-iceland/src/lib/messages" # Test: Ensure the new module exports something rg "export" "libs/application/templates/official-journal-of-iceland/src/lib/messages/involved-parties.ts"Length of output: 328
libs/api/domains/official-journal-of-iceland-application/src/models/getUserInvolvedParties.response.ts (2)
1-1: Imports and decorators are correctly used.The necessary decorators from
@nestjs/graphqlare imported, and@ObjectTypedecorators are used with descriptive names, which enhances schema clarity.Also applies to: 3-3, 15-15
1-19: Code adheres to coding guidelines forlibs/**/*files.The implementation meets the following criteria:
- Defines reusable types that can be used across different NextJS apps.
- Effectively uses TypeScript for defining and exporting types.
- Supports tree-shaking and efficient bundling by only containing type definitions.
libs/application/templates/official-journal-of-iceland/src/hooks/useInvolvedParties.ts (2)
38-45: LGTM! Well-structured return value.The hook's return value is well-structured, providing all necessary information (involvedParties, loading state, and error) in a clear and accessible manner. The use of optional chaining when accessing the nested
involvedPartiesproperty is a good practice that prevents potential runtime errors.
1-45: Great job! The hook is well-implemented and follows best practices.This custom hook,
useInvolvedParties, is well-structured and adheres to the coding guidelines:
- It's located in the
libsdirectory, promoting reusability across different NextJS apps.- It effectively uses TypeScript for type definitions and prop typing.
- The code structure supports effective tree-shaking and bundling.
The hook provides a clean and reusable way to fetch involved parties data, with proper error handling and loading state management. It's a valuable addition to the shared library.
🧰 Tools
🪛 Biome
[error] 30-30: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 33-33: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
libs/application/templates/official-journal-of-iceland/src/components/form/FormScreen.tsx (3)
5-5: LGTM: New imports for loading state functionality.The addition of
SkeletonLoaderandOJOI_INPUT_HEIGHTimports aligns with the new loading state feature and follows the project's structure. This change supports the reusability guideline for components across different NextJS apps.Also applies to: 11-11
25-25: LGTM: Props type updated with loading state.The addition of the
loading?: booleanprop to thePropstype is well-defined and follows TypeScript best practices. This change supports the TypeScript usage guideline for defining props and exporting types.
34-34: LGTM: Component parameter updated.The
loadingprop is correctly added to the destructured parameters of theFormScreencomponent, maintaining consistency with other props and reflecting the Props type update.libs/application/templates/official-journal-of-iceland/src/lib/constants.ts (1)
27-27: LGTM! Verify usage in related components.The addition of
INVOLVED_PARTYto theRoutesenum is consistent with the existing pattern and aligns with the PR objective. The naming follows TypeScript conventions, and being part of a constants file promotes reusability across components.To ensure proper integration, please run the following script to check for usage of the new route:
This script will help identify where the new route is being used and where it might need to be added to existing logic.
✅ Verification successful
Further Verification Required:
The previous script encountered an issue recognizing the
typescriptfile type. Please run the following script to search forRoutes.INVOLVED_PARTYwithin.tsand.tsxfiles directly:
Verification Successful!
The
INVOLVED_PARTYroute is correctly used inRequirements.ts, and there are no switch statements or if-else blocks that require updates. The addition adheres to the coding standards and integrates seamlessly with the existing routing logic.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for usage of the new INVOLVED_PARTY route and potential places where it should be added. # Search for usage of the new INVOLVED_PARTY route echo "Searching for usage of INVOLVED_PARTY route:" rg --type typescript "Routes\.INVOLVED_PARTY" libs/application/templates/official-journal-of-iceland/ # Search for switch statements or if-else blocks that might need updating echo "\nSearching for switch statements or if-else blocks using Routes:" rg --type typescript -e "switch.*Routes" -e "if.*Routes" libs/application/templates/official-journal-of-iceland/Length of output: 528
Script:
#!/bin/bash # Description: Check for usage of the new INVOLVED_PARTY route by targeting .ts and .tsx files directly. # Search for usage of the new INVOLVED_PARTY route in .ts and .tsx files echo "Searching for usage of INVOLVED_PARTY route in .ts and .tsx files:" rg "Routes\.INVOLVED_PARTY" --glob "*.ts" --glob "*.tsx" libs/application/templates/official-journal-of-iceland/ # Search for switch statements or if-else blocks using Routes in .ts and .tsx files echo "\nSearching for switch statements or if-else blocks using Routes in .ts and .tsx files:" rg "switch\s*\(.*Routes" --glob "*.ts" --glob "*.tsx" libs/application/templates/official-journal-of-iceland/ rg "if\s*\(.*Routes" --glob "*.ts" --glob "*.tsx" libs/application/templates/official-journal-of-iceland/Length of output: 894
libs/application/templates/official-journal-of-iceland/src/lib/messages/involved-parties.ts (2)
1-3: LGTM: Import and export declarations are correct.The import of
defineMessagesand the export ofinvolvedPartyare properly structured, allowing for easy usage of these internationalized messages across different components.
1-56: Great job: File adheres to coding guidelines and best practices.This file demonstrates excellent adherence to the coding guidelines for
libs/**/*files:
- Reusability: The structured message definitions can be easily imported and used across different NextJS apps.
- TypeScript usage: The file correctly uses TypeScript for defining and exporting types.
- Tree-shaking and bundling: The modular structure of message definitions allows for effective tree-shaking and bundling.
The use of
react-intland the clear organization of messages promote good internationalization practices. The file is well-structured, maintainable, and follows consistent patterns throughout.libs/application/templates/official-journal-of-iceland/src/forms/Requirements.ts (2)
19-19: LGTM: Import addition is correct and follows guidelines.The addition of
involvedPartyto the imports is consistent with the existing pattern and aligns with the PR objective of integrating involved party functionality. This change adheres to the coding guidelines for TypeScript usage and effective tree-shaking.
57-57: Verify the impact of submit field ID change on form flow.The change of the submit field's ID from 'toDraft' to 'toComments' is noted. This modification likely affects the form's workflow and aligns with the PR objective of managing user access in the application process.
Please confirm that this ID change:
- Correctly redirects users to the comments section instead of saving as a draft.
- Doesn't break any existing functionality or data flow in the application.
- Is consistently applied across all relevant components and handlers.
You can use the following script to verify the usage of 'toComments' ID:
libs/api/domains/official-journal-of-iceland-application/src/lib/ojoiApplication.resolver.ts (2)
26-27: LGTM: Import statements are correctly added.The new import statements for
GetUserInvolvedPartiesResponseandGetUserInvolvedPartiesInputare properly placed and follow TypeScript naming conventions. This approach maintains good modularity by keeping type definitions in separate files.
Line range hint
1-125: Coding guidelines compliance: Good adherence tolibs/**/*pattern.The implementation follows the coding guidelines for
libs/**/*:
- The resolver is structured for potential reuse across different NextJS apps.
- TypeScript is effectively used for defining props and exporting types.
- The modular structure supports effective tree-shaking and bundling practices.
libs/api/domains/official-journal-of-iceland-application/src/lib/ojoiApplication.service.ts (3)
20-20: LGTM: New import statement is correct and necessary.The import statement for
GetUserInvolvedPartiesInputis correctly implemented and follows TypeScript naming conventions. It's appropriately used in the new method.
171-178: LGTM: New methodgetUserInvolvedPartiesis implemented correctly.The new method is consistent with the class structure and correctly uses async/await syntax. It properly delegates the call to the underlying service.
171-178: Verify implementation against PR objectives.The PR objectives mention that this feature is related to users associated with multiple institutions and preventing users without access from progressing. However, the current implementation doesn't seem to include any specific logic for these requirements.
Please confirm if additional logic is needed in this method or if the requirements are handled elsewhere in the system. You may want to run the following script to check for related implementations:
libs/clients/official-journal-of-iceland/application/src/lib/ojoiApplicationClient.service.ts (2)
18-18: LGTM: Import added correctlyThe new import
GetInvolvedPartiesRequestis correctly added to the existing import statement. It follows the TypeScript usage guideline and is necessary for the new method, adhering to the reusability guideline.
Line range hint
1-189: Overall assessment: Good addition with minor improvements neededThe new
getUserInvolvedPartiesmethod aligns well with the PR objective of integrating involved party functionality. It follows TypeScript usage and reusability guidelines. However, to make it production-ready, please address the suggested improvements in the previous comment regarding error handling, console.log removal, and documentation.The changes adhere to the coding guidelines for files in the
libs/**/*pattern, ensuring reusability across different NextJS apps and proper TypeScript usage. Once the minor issues are addressed, this addition will be a valuable enhancement to the service.libs/application/templates/official-journal-of-iceland/src/graphql/queries.ts (1)
193-203: LGTM! Consider adding TypeScript interface for response.The new
INVOLVED_PARTIES_QUERYis well-structured and consistent with the existing patterns in the file. It's focused and specific, which is good for tree-shaking and bundling practices.To improve type safety and adhere to TypeScript best practices, consider adding a TypeScript interface for the query response. This will enhance the reusability of the query across different NextJS apps.
Here's an example of how you could add a TypeScript interface:
interface InvolvedPartiesResponse { officialJournalOfIcelandApplicationGetUserInvolvedParties: { involvedParties: Array<{ id: string; title: string; slug: string; }>; }; } export const INVOLVED_PARTIES_QUERY = gql< InvolvedPartiesResponse, { input: GetUserInvolvedPartiesInput } >` query InvolvedParties($input: GetUserInvolvedPartiesInput!) { officialJournalOfIcelandApplicationGetUserInvolvedParties(input: $input) { involvedParties { id title slug } } } `;To ensure the query name matches the backend resolver, run the following command:
This will help confirm that the query name is correctly aligned with the backend implementation.
✅ Verification successful
The previous command failed due to an unrecognized file type. Please run the following script without specifying the file type to search across all files:
Verified Query Name Alignment
The
INVOLVED_PARTIES_QUERYcorrectly matches the backend resolverofficialJournalOfIcelandApplicationGetUserInvolvedParties. All references are consistent across the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if the query name matches the backend resolver # Test: Search for the backend resolver definition rg --type typescript "officialJournalOfIcelandApplicationGetUserInvolvedParties"Length of output: 115
Script:
#!/bin/bash # Description: Verify if the query name matches the backend resolver # Test: Search for the backend resolver definition without specifying file type rg "officialJournalOfIcelandApplicationGetUserInvolvedParties"Length of output: 1243
libs/clients/official-journal-of-iceland/application/src/clientConfig.json (5)
275-299: New endpoint for retrieving involved parties looks good.The new GET endpoint
/api/v1/applications/{id}/involved-partiesis well-structured and consistent with other endpoints in the file. It follows RESTful conventions and uses appropriate parameter and response schema.
651-657: Application state enum update looks good.The
stateenum in theApplicationschema has been expanded to include more comprehensive states: "requirements", "draft_retry", "submitted", and "complete". These additions provide a more detailed representation of an application's lifecycle, which aligns well with the PR objectives.
987-987: ApplicationAttachmentType title enum update is appropriate.The
titleenum in theApplicationAttachmentTypeschema has been updated to include "Frumrit" and "Fylgiskjöl". This change appears to be a localization update, which is consistent with the context of the Official Journal of Iceland.
1049-1080: New Institution and ApplicationUserInvolvedPartiesResponse schemas look good.The newly added
InstitutionandApplicationUserInvolvedPartiesResponseschemas are well-structured and consistent with the PR objectives. TheInstitutionschema includes all necessary fields (id, title, and slug) with appropriate descriptions and examples. TheApplicationUserInvolvedPartiesResponseschema correctly references theInstitutionschema for theinvolvedPartiesarray.
Line range hint
1-1080: Overall, the changes to clientConfig.json are well-implemented and align with the PR objectives.The additions and modifications in this file successfully implement the involved party functionality. The new endpoint, schema updates, and additions are well-structured, consistent with the existing codebase, and adhere to OpenAPI specifications. Good job on maintaining consistency and following best practices throughout the changes.
libs/application/templates/official-journal-of-iceland/src/screens/InvolvedPartyScreen.tsx (1)
46-46: Verify the availability ofstructuredCloneThe
structuredClonefunction used in line 46 may not be supported in all JavaScript environments, particularly in older Node.js versions or certain browsers. Ensure that your runtime environment supportsstructuredClone. If it doesn't, consider using an alternative likelodash.cloneDeep.
...omains/official-journal-of-iceland-application/src/models/getUserInvolvedParties.response.ts
Outdated
Show resolved
Hide resolved
...omains/official-journal-of-iceland-application/src/models/getUserInvolvedParties.response.ts
Outdated
Show resolved
Hide resolved
libs/api/domains/official-journal-of-iceland-application/src/lib/ojoiApplication.service.ts
Show resolved
Hide resolved
libs/clients/official-journal-of-iceland/application/src/lib/ojoiApplicationClient.service.ts
Outdated
Show resolved
Hide resolved
libs/application/templates/official-journal-of-iceland/src/screens/InvolvedPartyScreen.tsx
Outdated
Show resolved
Hide resolved
libs/application/templates/official-journal-of-iceland/src/screens/InvolvedPartyScreen.tsx
Outdated
Show resolved
Hide resolved
libs/application/templates/official-journal-of-iceland/src/screens/InvolvedPartyScreen.tsx
Show resolved
Hide resolved
libs/application/templates/official-journal-of-iceland/src/screens/InvolvedPartyScreen.tsx
Show resolved
Hide resolved
libs/application/templates/official-journal-of-iceland/src/screens/InvolvedPartyScreen.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
libs/application/templates/official-journal-of-iceland/src/screens/InvolvedPartyScreen.tsx (1)
57-103: Well-structured UI with room for minor improvement.The component effectively prepares options for the select input and handles different states (loading, error, no data) with appropriate UI components. The use of
AlertMessagefor warnings and errors is a good practice.However, there's a minor improvement we can make in the
onChangehandler of theOJOISelectController.Apply this diff to improve the code:
- onChange={() => - setSubmitButtonDisabled && setSubmitButtonDisabled(false) - } + onChange={() => setSubmitButtonDisabled?.(false)}This change simplifies the code and follows modern JavaScript practices by using optional chaining.
🧰 Tools
🪛 Biome
[error] 99-99: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
libs/clients/official-journal-of-iceland/application/src/lib/ojoiApplicationClient.service.ts (1)
183-198: Excellent improvements, consider adding JSDoc and minor refactoringGreat job implementing the
getUserInvolvedPartiesmethod! The error handling and removal of the console.log statement align well with the class's overall structure. To further enhance the code:
- Add a JSDoc comment to document the method's purpose and parameters.
- Consider returning the data directly without assigning it to a variable for consistency with other methods in the class.
Here's a suggested improvement:
/** * Retrieves involved parties for a user * @param params - The request parameters for getting involved parties * @param auth - The authentication object * @returns A promise that resolves to the involved parties data */ async getUserInvolvedParties(params: GetInvolvedPartiesRequest, auth: Auth) { try { return await this.ojoiApplicationApiWithAuth(auth).getInvolvedParties(params); } catch (error) { this.logger.warn('Failed to get involved parties', { error, applicationId: params.id, category: LOG_CATEGORY, }); throw error; } }This change improves documentation and maintains consistency with other methods in the class.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
- libs/api/domains/official-journal-of-iceland-application/src/models/getUserInvolvedParties.response.ts (1 hunks)
- libs/application/templates/official-journal-of-iceland/src/screens/InvolvedPartyScreen.tsx (1 hunks)
- libs/clients/official-journal-of-iceland/application/src/lib/ojoiApplicationClient.service.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/api/domains/official-journal-of-iceland-application/src/models/getUserInvolvedParties.response.ts
🧰 Additional context used
📓 Path-based instructions (2)
libs/application/templates/official-journal-of-iceland/src/screens/InvolvedPartyScreen.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/clients/official-journal-of-iceland/application/src/lib/ojoiApplicationClient.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."
🪛 Biome
libs/application/templates/official-journal-of-iceland/src/screens/InvolvedPartyScreen.tsx
[error] 25-25: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 33-33: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 99-99: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (2)
libs/application/templates/official-journal-of-iceland/src/screens/InvolvedPartyScreen.tsx (2)
1-15: Imports and component declaration look good.The necessary dependencies are imported, and the component is properly declared with TypeScript types. This follows good practices for reusability and type safety.
1-105: Overall, excellent implementation of the InvolvedPartyScreen component.The component is well-structured, follows React and TypeScript best practices, and effectively manages the display and selection of involved parties for an application. It handles various scenarios and states, including loading, errors, and different numbers of involved parties.
Key strengths:
- Proper use of hooks for state management and side effects.
- Effective error handling and user feedback through AlertMessage components.
- TypeScript usage for prop types and improved type safety.
- Reusable design that can be integrated into different NextJS apps.
The component adheres to the coding guidelines and demonstrates good practices for reusability and maintainability.
🧰 Tools
🪛 Biome
[error] 25-25: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 33-33: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 99-99: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
libs/application/templates/official-journal-of-iceland/src/screens/InvolvedPartyScreen.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- libs/application/templates/official-journal-of-iceland/src/screens/InvolvedPartyScreen.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
libs/application/templates/official-journal-of-iceland/src/screens/InvolvedPartyScreen.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."
🪛 Biome
libs/application/templates/official-journal-of-iceland/src/screens/InvolvedPartyScreen.tsx
[error] 25-25: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 33-33: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 96-96: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (3)
libs/application/templates/official-journal-of-iceland/src/screens/InvolvedPartyScreen.tsx (3)
1-15: Imports and component declaration look good.The imports are appropriate for the component's functionality, and the component declaration follows TypeScript practices. Good job on using typed props with
OJOIFieldBaseProps.
54-62: Options and select input setup look good.The creation of options for the select input, setting up the default value, and determining when to disable the select input are all implemented correctly. Good job on handling these aspects of the component.
1-102: Overall, theInvolvedPartyScreencomponent is well-implemented.The component effectively manages the display and selection of involved parties for the Official Journal of Iceland application. It adheres to React best practices, uses TypeScript for type safety, and follows the coding guidelines for reusability and modularity.
Key strengths:
- Proper use of custom hooks for state management and data fetching.
- Effective error handling and loading state management.
- Conditional rendering based on different scenarios.
- Use of localization for internationalization support.
The suggested improvements are minor and focus on code modernization:
- Using optional chaining for
setSubmitButtonDisabledcalls.- Simplifying condition checks and variable usage.
These changes will enhance code readability and maintainability without affecting the overall functionality. Great job on implementing this feature!
🧰 Tools
🪛 Biome
[error] 25-25: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 33-33: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 96-96: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
libs/application/templates/official-journal-of-iceland/src/screens/InvolvedPartyScreen.tsx
Show resolved
Hide resolved
libs/application/templates/official-journal-of-iceland/src/screens/InvolvedPartyScreen.tsx
Show resolved
Hide resolved
libs/application/templates/official-journal-of-iceland/src/screens/InvolvedPartyScreen.tsx
Show resolved
Hide resolved
✅ Actions performedComments resolved and changes approved. |
Datadog ReportAll test runs ✅ 68 Total Test Services: 0 Failed, 66 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (3) |
* Involved party client / graphql * WIP: setting up involved party screen * Involved parties now ready * Fixed PR comments. * Fixed more PR comments
What
Adding involved party integration for user tied to many institutions. Stopping users who have no access to application to go further into the application process.
Why
To allow users to go on behalf of their institutions
Screenshots / Gifs
Checklist:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
These updates enhance user experience by providing more dynamic and responsive interactions within the application.