feat(service-portal): default defender and has chosen fields for subpoena #16306
Conversation
…s/island.is into service-portal/subpoena-fixes-2
WalkthroughThe pull request introduces modifications to the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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: 1
🧹 Outside diff range and nitpick comments (6)
libs/service-portal/law-and-order/src/screens/Subpoena/Subpoena.graphql (1)
10-11: LGTM! Consider adding comments for clarity.The addition of
defaultChoiceandhasChosenfields aligns well with the PR objectives. These new fields will enhance the subpoena defender selection process as intended.Consider adding inline comments to briefly explain the purpose of these new fields. This can improve code readability and maintainability. For example:
defaultChoice # The default defender choice for the subpoena hasChosen # Indicates if a defender has been chosenlibs/api/domains/law-and-order/src/models/subpoena.model.ts (3)
38-43: LGTM! Consider adding JSDoc comments for the new fields.The addition of
defaultChoiceandhasChosenfields aligns well with the PR objectives. The types are appropriate, and the changes adhere to TypeScript best practices.To further improve code clarity and maintainability, consider adding JSDoc comments to describe the purpose and usage of these new fields.
Example improvement:
/** * The default choice for the subpoena defender. */ @Field(() => DefenseChoiceEnum) defaultChoice!: DefenseChoiceEnum /** * Indicates whether a choice has been made for the subpoena defender. */ @Field(() => Boolean, { nullable: true }) hasChosen?: boolean
Line range hint
7-22: Consider reviewing the nullability of Text class fields.While not directly related to the current PR changes, I noticed that all fields in the Text class are nullable. This could potentially lead to runtime errors if not handled properly in the consuming code.
Consider reviewing whether some of these fields should be required (non-nullable) based on your business logic. If they are indeed always optional, it might be beneficial to add comments explaining why, to prevent future confusion.
Example improvement:
@ObjectType('LawAndOrderSubpoenaTexts') export class Text { @Field({ nullable: true }) intro?: string @Field({ nullable: false }) // If this should always be present confirmation!: string @Field({ nullable: true }) description?: string @Field({ nullable: true }) claim?: string }
Line range hint
45-55: Consider adding JSDoc comments to the Subpoena class.The Subpoena class structure looks good and adheres to the coding guidelines. To further improve code clarity and support effective tree-shaking, consider adding JSDoc comments to describe the purpose of the class and its fields.
This will enhance code documentation and make it easier for other developers to understand and use this class across different NextJS apps.
Example improvement:
/** * Represents a subpoena in the law and order domain. */ @ObjectType('LawAndOrderSubpoena') export class Subpoena { /** Textual content related to the subpoena. */ @Field({ nullable: true }) texts?: Text /** Actions that can be performed on the subpoena. */ @Field(() => [Action], { nullable: true }) actions?: Array<Action> /** Data associated with the subpoena. */ @Field(() => Data, { nullable: true }) data?: Data }libs/api/domains/law-and-order/src/lib/helpers/mappers.ts (1)
50-68: LGTM: New mapping function is well-implemented.The
mapDefenseChoiceForSubpoenaDefaultChoicefunction is correctly implemented and follows the existing patterns in the file. It adheres to the coding guidelines for thelibsdirectory:
- It's exported, making it reusable across different NextJS apps.
- It uses TypeScript for defining parameter and return types.
- The pure function implementation supports effective tree-shaking.
Consider adding a brief JSDoc comment to describe the function's purpose and parameters, enhancing code documentation. For example:
/** * Maps the SubpoenaDataDefaultDefenderChoiceEnum to DefenseChoiceEnum. * @param choice - The default defender choice for a subpoena. * @returns The corresponding DefenseChoiceEnum value. */ export const mapDefenseChoiceForSubpoenaDefaultChoice = ( choice?: SubpoenaDataDefaultDefenderChoiceEnum, ): DefenseChoiceEnum => { // ... (existing implementation) }libs/api/domains/law-and-order/src/lib/law-and-order.service.ts (1)
144-148: Approved: Enhanced data structure with new propertiesThe changes to the
getSubpoenamethod improve the structure of the returned data by addingdefaultChoiceandhasChosenproperties, aligning with the PR objectives. The use of the localdefenderChoicevariable is a good practice for preventing potential null reference errors.Consider adding null checks or using optional chaining for
subpoena?.data.defaultDefenderChoiceandsubpoena?.data.hasChosenDefenderto enhance null safety:defaultChoice: mapDefenseChoiceForSubpoenaDefaultChoice( subpoena?.data?.defaultDefenderChoice ), hasChosen: subpoena?.data?.hasChosenDefender ?? false,This change would make the code more robust against potential undefined values.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (8)
- libs/api/domains/law-and-order/src/lib/helpers/mappers.ts (2 hunks)
- libs/api/domains/law-and-order/src/lib/law-and-order.service.ts (2 hunks)
- libs/api/domains/law-and-order/src/models/subpoena.model.ts (1 hunks)
- libs/clients/judicial-system-sp/src/clientConfig.json (1 hunks)
- libs/service-portal/core/src/components/IntroHeader/IntroHeader.tsx (2 hunks)
- libs/service-portal/law-and-order/src/components/DefenderChoices/DefenderChoices.tsx (2 hunks)
- libs/service-portal/law-and-order/src/screens/Subpoena/Subpoena.graphql (1 hunks)
- libs/service-portal/law-and-order/src/screens/Subpoena/Subpoena.tsx (5 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
libs/api/domains/law-and-order/src/lib/helpers/mappers.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/law-and-order/src/lib/law-and-order.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/law-and-order/src/models/subpoena.model.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/judicial-system-sp/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/service-portal/core/src/components/IntroHeader/IntroHeader.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/service-portal/law-and-order/src/components/DefenderChoices/DefenderChoices.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/service-portal/law-and-order/src/screens/Subpoena/Subpoena.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/service-portal/law-and-order/src/screens/Subpoena/Subpoena.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 (18)
libs/service-portal/law-and-order/src/screens/Subpoena/Subpoena.graphql (1)
Line range hint
1-24: Overall, the changes look good and align with the PR objectives.The addition of
defaultChoiceandhasChosenfields enhances the subpoena data structure as intended. The removal of thealertsfield (as mentioned in the AI summary) also seems appropriate for this update.These changes should improve the handling of subpoena defender choices in the Service Portal, which is in line with the PR's goals.
To ensure these changes are consistently applied across the codebase, please run the following script:
This script will help identify any places where the new fields should be used or where the old
alertsfield might still be referenced.✅ Verification successful
Verified Changes:
- Verified that
defaultChoiceandhasChosenfields are properly used inSubpoena.tsx.- No remaining references to the
alertsfield found.These changes are correctly applied and align with the PR objectives.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of new fields and removal of alerts field # Check for usage of new fields echo "Checking usage of new fields:" rg --type typescript "defaultChoice|hasChosen" libs/service-portal/law-and-order/src # Check for any remaining usage of the alerts field echo "Checking for remaining usage of alerts field:" rg --type typescript "alerts.*type|alerts.*message" libs/service-portal/law-and-order/srcLength of output: 418
Script:
#!/bin/bash # Description: Verify the usage of new fields and removal of alerts field # Check for usage of new fields echo "Checking usage of new fields:" rg "defaultChoice|hasChosen" --glob "*.ts" --glob "*.tsx" libs/service-portal/law-and-order/src # Check for any remaining usage of alerts field echo "Checking for remaining usage of alerts field:" rg "alerts.*type|alerts.*message" --glob "*.ts" --glob "*.tsx" libs/service-portal/law-and-order/srcLength of output: 771
libs/service-portal/core/src/components/IntroHeader/IntroHeader.tsx (3)
8-9: LGTM: Import changes are consistent and follow best practices.The addition of
SkeletonLoaderandStackimports from '@island.is/island-ui/core' is consistent with the existing import style and supports effective tree-shaking and bundling practices.
46-65: LGTM: Improved loading state and enhanced component flexibility.The changes to the rendering logic are well-structured and improve both the user experience and component flexibility:
- The new loading state using
SkeletonLoaderprovides a more intuitive visual feedback.- The restructured conditional rendering enhances code organization and readability.
- The addition of conditional rendering for
props.introComponentandprops.childrenincreases the component's reusability across different NextJS apps.These improvements align well with our coding guidelines for reusable and flexible components.
Line range hint
1-92: LGTM: Adherence to coding guidelines forlibs/**/*pattern confirmed.The changes in this file maintain adherence to the coding guidelines for the
libs/**/*pattern:
- The component remains reusable across different NextJS apps.
- TypeScript is consistently used for defining props and the component is properly exported.
- The new imports support effective tree-shaking and bundling practices.
The modifications enhance the component while preserving its compliance with our coding standards.
libs/api/domains/law-and-order/src/lib/helpers/mappers.ts (1)
4-4: LGTM: Import addition is appropriate.The new import
SubpoenaDataDefaultDefenderChoiceEnumis correctly added and aligns with the new function implementation. This adheres to the TypeScript usage guidelines for defining types.libs/service-portal/law-and-order/src/screens/Subpoena/Subpoena.tsx (8)
6-6: LGTM: SkeletonLoader import addedThe addition of
SkeletonLoaderfrom the shared UI library aligns with the PR objective of improving the loading experience. This change supports reusability across different NextJS apps and maintains effective tree-shaking practices.
47-49: LGTM: New state variable for defender popupThe addition of the
defenderPopUpstate variable using theuseStatehook is appropriate for controlling the visibility of the defender choices modal. The use of TypeScript for type definition (boolean) adheres to the coding guidelines, and the naming is clear and descriptive.
66-75: LGTM: IntroHeader component structure improvedThe changes to the
IntroHeadercomponent improve readability by making the structure more explicit. The conditional rendering of theAlertMessageenhances the user experience by providing confirmation text when available. The use of shared UI components (Box,AlertMessage) supports reusability across different NextJS apps.
77-77: LGTM: SkeletonLoader added for improved loading stateThe addition of the
SkeletonLoadercomponent enhances the loading experience, which aligns with the PR objectives. The conditional rendering based on theloadingstate is a good practice, and the use of the shared UI component supports reusability across different NextJS apps.
Line range hint
108-139: LGTM: Defender choice rendering logic improvedThe updated rendering logic for the defender choice section now accurately reflects whether a defender has been chosen by using
subpoena.data.hasChosen. This change aligns with the PR objective of adding new fields for subpoena defenders. The conditional rendering of theInfoLinecomponent based on bothhasChosenanddefenderChoiceimproves the accuracy of when to display the defender choice information.
142-147: LGTM: DefenderChoices component rendering updatedThe conditional rendering of the
DefenderChoicescomponent based onhasChosenensures it's only shown when appropriate. The addition of thechoiceprop set tosubpoena.data.defaultChoicealigns with the PR objective of adding new fields for subpoena defenders. These changes likely improve the functionality and user experience of the defender selection process.
Line range hint
149-161: LGTM: DefenderChoices component in modal updatedThe update to the
choiceprop in the modal'sDefenderChoicescomponent, now usingsubpoena.data.defenderChoice, ensures consistency between the main view and the modal view. This change aligns with the PR objective of improving the handling of defender choices and enhances the overall user experience.
Line range hint
1-185: Overall assessment: Excellent implementationThe changes in this file align well with the PR objectives, improving the handling of subpoenas and defense choices. The implementation adheres to the coding guidelines for files in the
libsdirectory, demonstrating:
- Reusability of components and hooks across different NextJS apps.
- Proper usage of TypeScript for defining props and types.
- Effective tree-shaking and bundling practices through appropriate imports.
The changes enhance the user experience, particularly in terms of loading states and defender choice management. The code structure and component logic have been improved, making the file more maintainable and readable.
libs/api/domains/law-and-order/src/lib/law-and-order.service.ts (2)
24-24: LGTM: New import enhances mapping functionalityThe addition of
mapDefenseChoiceForSubpoenaDefaultChoiceto the imports is a good practice. It adheres to TypeScript usage guidelines and supports effective tree-shaking by importing a specific function. This change potentially enhances the reusability of the mapping logic across different parts of the application.
Line range hint
1-214: Overall: Changes align with coding guidelines and enhance functionalityThe modifications to
LawAndOrderServiceadhere to the coding guidelines for files in thelibsdirectory:
- Reusability: The changes maintain and potentially improve the service's reusability across different NextJS apps by enhancing the
getSubpoenamethod's functionality.- TypeScript usage: The file consistently uses TypeScript for defining types and structures, including the new properties added to the returned data object.
- Tree-shaking: The specific import of
mapDefenseChoiceForSubpoenaDefaultChoiceand focused changes support effective tree-shaking and bundling practices.The changes successfully implement the PR objectives of adding
hasChosenanddefaultChoicefields while improving the overall structure of the subpoena data.libs/service-portal/law-and-order/src/components/DefenderChoices/DefenderChoices.tsx (3)
10-10: LGTM: SkeletonLoader import addedThe addition of
SkeletonLoaderfrom '@island.is/island-ui/core' is a good improvement for enhancing the loading experience. This change maintains consistency with other UI component imports and aligns with the coding guideline of reusability across different NextJS apps.
110-111: Improved loading state implementationThe update to use
SkeletonLoaderinstead ofLoadingDotsenhances the visual representation of the loading state. The simplified conditional rendering improves code readability. This change aligns well with the PR objective of improving the loading experience for the subpoena component.
Line range hint
1-238: Adherence to coding guidelines confirmedThe
DefenderChoicescomponent adheres well to the specified coding guidelines:
- It uses TypeScript for defining props and types, enhancing type safety.
- It utilizes reusable components from '@island.is/island-ui/core', promoting consistency across the application.
- The component structure and import statements support effective tree-shaking and bundling practices.
These practices contribute to the overall maintainability and performance of the application.
Datadog ReportAll test runs ✅ 3 Total Test Services: 0 Failed, 2 Passed Test Services
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16306 +/- ##
=======================================
Coverage 36.85% 36.85%
=======================================
Files 6798 6798
Lines 140535 140531 -4
Branches 39961 39957 -4
=======================================
Hits 51799 51799
+ Misses 88736 88732 -4
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
…oena (#16306) * fix: def info and alert * feat: add feature flag to resolver * fix: move ff call to seperate function * feat: add default choices ans has chosen + loading states * fix: use type * fix: undefined type issue * fix: simplify check
…-pages (#16234) * Service portal removal. Add portals my pages * minor fixes * Fix * path fix * fix(portals-admin): locklist (#16279) * fix(portals-admin): locklist * tweak * msg id fix * tweak --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(service-portal): feature flag resolver for documents (#16285) * fix: def info and alert * feat: add feature flag to resolver * fix: move ff call to seperate function --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(vehicles-bulk-mileage): Fixes after testing review (#16295) * fix: testing fixes v1 * fix: testing comments v2 * fix: better message * fix: function name * fix: duplicate loading --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(tests): New @island/testing/e2e library (#16287) * Add @swc-node/register and @swc/core * Add testing/e2e library * update project.json for testing/e2e * fix import for libTestingE2e --------- Co-authored-by: Kristofer <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(parental-leave): ApplicationRights (#15901) * feat(parental-leave): ApplicationRights Added applicationRights to parental-leave when sending application. Since we are using a new way of calculating periods * Fix days used by period calculation * Tests for new periods * rename function with proper camelCase * Refactor: Made duplicate code into a function * Make ApplicationRights nullable * refactor: function instead of duplicate code * remove console.log * error handling for period data * clientConfig nullable fix * Fixes for calculation of months. And using clamp to get correct value of daysLeft * Multiply amount of months by 30 for period calculation with month durations * Fix old calculation of endDate with months --------- Co-authored-by: hfhelgason <[email protected]> Co-authored-by: veronikasif <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(passport-application): Updated readme (#16296) * updated readme * updated readme * chore: nx format:write update dirty files --------- Co-authored-by: andes-it <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(regulations-admin): date format signature, remove self affect, disclaimer text (#16288) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(regulations-admin): No diff no addition in appendix (#16293) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(web): Global alert banner - Handle null case (#16298) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(web): Change custom syslumenn pages config for header (#16299) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(j-s): Digital mailbox API (#16301) * feat(j-s): Block create subpoena on staging and dev * Update subpoena.service.ts * fix(j-s): Fix mailbox API * remove changes not meant for this branch * Update subpoena.service.ts * fix(j-s): reverting changes from other branch * Update subpoena.response.ts * Update subpoena.response.ts * Update subpoena.response.ts * Update subpoena.response.ts --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(signature-collection): Fix list reviewed toggle (#16300) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * chore(scripts): Stricter shell script checking (#16242) * Set style level for shellcheck * Linting & formatting scripts * Remove _podman.sh script * Format all scripts * Add reviewdog/action-shfmt step * Configure shfmt * Merge from main * Linting * Move shfmt to before lint * Remove reviewdog * Allow external sources in shellcheck * Use Reviewdog for shellcheck * Set version for Reviewdog --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * chore(new-primary-school): Update messages namespace (#16302) Co-authored-by: veronikasif <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(driving-license): check if 65+ renewal is possible (#16292) * check if 65 renewal is possible * remove console log * cleanup * coderabbit tweaks * coderabbit changes * quick fix * add type? --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(service-portal): default defender and has chosen fields for subpoena (#16306) * fix: def info and alert * feat: add feature flag to resolver * fix: move ff call to seperate function * feat: add default choices ans has chosen + loading states * fix: use type * fix: undefined type issue * fix: simplify check * Update service setup for my pages infra * chore: charts update dirty files * Remove from infra * undo rename --------- Co-authored-by: albinagu <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> Co-authored-by: Ásdís Erna Guðmundsdóttir <[email protected]> Co-authored-by: Þorkell Máni Þorkelsson <[email protected]> Co-authored-by: Svanhildur Einarsdóttir <[email protected]> Co-authored-by: Kristofer <[email protected]> Co-authored-by: helgifr <[email protected]> Co-authored-by: hfhelgason <[email protected]> Co-authored-by: veronikasif <[email protected]> Co-authored-by: Rafn Árnason <[email protected]> Co-authored-by: andes-it <[email protected]> Co-authored-by: Rúnar Vestmann <[email protected]> Co-authored-by: mannipje <[email protected]> Co-authored-by: unakb <[email protected]> Co-authored-by: juni-haukur <[email protected]> Co-authored-by: birkirkristmunds <[email protected]> Co-authored-by: Kristján Albert <[email protected]>
Service Portal - Law and order 🚓
What
Checklist:
Summary by CodeRabbit
Release Notes
New Features
defaultChoiceandhasChosen.SkeletonLoaderin various components.defenderPopUpstate.Bug Fixes
These updates aim to improve user experience and provide more detailed information regarding defender choices in the subpoena process.