feat(application-system): handle pruned application links #15233
Conversation
WalkthroughThe recent update enhances the application's ability to mark certain data as "pruned." Several components now handle this new attribute, including service methods, data transfer objects, and error handling logic. This change aims to clearly delineate and manage pruned data across various application layers, ensuring the system can handle such instances gracefully and consistently. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as User Interface
participant SVC as ApplicationService
participant API as Application API
UI->>SVC: fetchApplication(id, auth, locale)
SVC->>API: applicationControllerFindOne(id, locale)
API-->>SVC: applicationData
alt applicationData.pruned == true
SVC-->>UI: { data: {...}, answers: {}, attachments: {}, externalData: {} }
UI->>UI: Display pruned error message
else
SVC-->>UI: applicationData
UI->>UI: Display application data
end
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 (
|
jonnigs
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #15233 +/- ##
==========================================
- Coverage 37.10% 37.10% -0.01%
==========================================
Files 6448 6448
Lines 131456 131460 +4
Branches 37575 37575
==========================================
+ Hits 48781 48782 +1
- Misses 82675 82678 +3
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 ReportBranch report: ✅ 0 Failed, 4 Passed, 0 Skipped, 2.96s Total Time |
jonnigs
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (1)
libs/application/ui-shell/src/lib/ApplicationForm.tsx (1)
Line range hint
158-158: Utilize optional chaining for safer property access.In the
populateFormfunction, when accessing properties of potentially undefined objects, use optional chaining to prevent runtime errors:- const role = template.mapUserToRole(nationalRegistryId, application) + const role = template?.mapUserToRole(nationalRegistryId, application)This change ensures that the code is more robust and can handle cases where
templatemight benull.Tools
Biome
[error] 49-49: Unnecessary use of boolean literals in conditional expression. (lint/complexity/noUselessTernary)
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (7)
- apps/application-system/api/src/app/modules/application/dto/application.response.dto.ts (1 hunks)
- libs/api/domains/application/src/lib/application.model.ts (1 hunks)
- libs/api/domains/application/src/lib/application.service.ts (1 hunks)
- libs/application/core/src/lib/messages.ts (1 hunks)
- libs/application/graphql/src/lib/fragments/application.ts (1 hunks)
- libs/application/ui-shell/src/components/ErrorShell.tsx (2 hunks)
- libs/application/ui-shell/src/lib/ApplicationForm.tsx (3 hunks)
Additional context used
Path-based instructions (7)
libs/application/graphql/src/lib/fragments/application.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/application-system/api/src/app/modules/application/dto/application.response.dto.ts (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/api/domains/application/src/lib/application.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/application/ui-shell/src/components/ErrorShell.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/ui-shell/src/lib/ApplicationForm.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/application/src/lib/application.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/application/core/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."
Biome
libs/application/ui-shell/src/lib/ApplicationForm.tsx
[error] 49-49: Unnecessary use of boolean literals in conditional expression. (lint/complexity/noUselessTernary)
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
[error] 158-158: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
Additional comments not posted (7)
libs/application/graphql/src/lib/fragments/application.ts (1)
39-39: The addition of theprunedfield to the GraphQL fragment is appropriate and aligns with the PR objectives.apps/application-system/api/src/app/modules/application/dto/application.response.dto.ts (1)
182-186: The addition of theprunedfield to theBaseApplicationResponseDtoclass is correctly implemented with appropriate decorators.libs/api/domains/application/src/lib/application.model.ts (1)
133-134: The addition of theprunedfield to theApplicationclass is correctly implemented with appropriate GraphQL decorators, ensuring it's optional.libs/application/ui-shell/src/components/ErrorShell.tsx (1)
56-60: The addition of theprunederror type in theErrorShellcomponent is correctly implemented, utilizing existing message constants effectively.Also applies to: 74-74
libs/application/ui-shell/src/lib/ApplicationForm.tsx (1)
79-81: Handling theprunedstate by redirecting to an error page.This change is consistent with the PR's objective to handle pruned applications by preventing access. The implementation is straightforward and uses existing components effectively.
libs/api/domains/application/src/lib/application.service.ts (1)
51-53: Handle pruned applications by clearing sensitive data.The implementation effectively clears out the
answers,attachments, andexternalDatafields for pruned applications, ensuring that sensitive data is not exposed. This is a secure and thoughtful way to handle data for pruned applications.libs/application/core/src/lib/messages.ts (1)
845-862: Add new error messages for pruned applications.The added messages (
prunedTitle,prunedSubTitle, andprunedDescription) are clear and consistent with the rest of the application's error handling. They provide necessary user feedback for pruned applications, aligning with the PR's objectives to handle inaccessible pruned applications appropriately.
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- libs/application/ui-shell/src/lib/ApplicationForm.tsx (3 hunks)
Additional context used
Path-based instructions (1)
libs/application/ui-shell/src/lib/ApplicationForm.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/ui-shell/src/lib/ApplicationForm.tsx
[error] 48-49: Expected an expression but instead found 'const'. (parse)
Expected an expression here.
[error] 50-50: Expected a statement but instead found ')'. (parse)
Expected a statement here.
[error] 158-158: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
[error] 49-49: Shouldn't redeclare 'delegationsChecked'. Consider to delete it or rename it. (lint/suspicious/noRedeclare)
'delegationsChecked' is defined here:
[error] 49-49: Shouldn't redeclare 'setDelegationsChecked'. Consider to delete it or rename it. (lint/suspicious/noRedeclare)
'setDelegationsChecked' is defined here:
Additional comments not posted (2)
libs/application/ui-shell/src/lib/ApplicationForm.tsx (2)
79-81: Handle pruned applications correctly to enhance user experience.The implementation correctly handles the scenario where an application is pruned by displaying an appropriate error message. This aligns with the PR's objective to prevent access to pruned applications.
195-199: Ensure thatApplicationFormcomponent is properly structured for future scalability.The restructuring of the
ApplicationFormcomponent using destructured props improves readability and maintainability. This is a positive change towards making the codebase more manageable.
jonnigs
force-pushed
the
feat/handle-pruned-application-links
branch
from
07e6ecc to
4f1bd07
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (7)
- apps/application-system/api/src/app/modules/application/dto/application.response.dto.ts (1 hunks)
- libs/api/domains/application/src/lib/application.model.ts (1 hunks)
- libs/api/domains/application/src/lib/application.service.ts (1 hunks)
- libs/application/core/src/lib/messages.ts (1 hunks)
- libs/application/graphql/src/lib/fragments/application.ts (1 hunks)
- libs/application/ui-shell/src/components/ErrorShell.tsx (2 hunks)
- libs/application/ui-shell/src/lib/ApplicationForm.tsx (3 hunks)
Files skipped from review due to trivial changes (2)
- libs/application/graphql/src/lib/fragments/application.ts
- libs/application/ui-shell/src/components/ErrorShell.tsx
Additional context used
Path-based instructions (5)
apps/application-system/api/src/app/modules/application/dto/application.response.dto.ts (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/api/domains/application/src/lib/application.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/application/ui-shell/src/lib/ApplicationForm.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/application/src/lib/application.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/application/core/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."
Biome
libs/application/ui-shell/src/lib/ApplicationForm.tsx
[error] 48-49: Expected an expression but instead found 'const'. (parse)
Expected an expression here.
[error] 50-50: Expected a statement but instead found ')'. (parse)
Expected a statement here.
[error] 158-158: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
[error] 49-49: Shouldn't redeclare 'delegationsChecked'. Consider to delete it or rename it. (lint/suspicious/noRedeclare)
'delegationsChecked' is defined here:
[error] 49-49: Shouldn't redeclare 'setDelegationsChecked'. Consider to delete it or rename it. (lint/suspicious/noRedeclare)
'setDelegationsChecked' is defined here:
Additional comments not posted (4)
apps/application-system/api/src/app/modules/application/dto/application.response.dto.ts (1)
182-186: LGTM! Theprunedproperty is correctly added with appropriate decorators for API exposure and validation.libs/api/domains/application/src/lib/application.model.ts (1)
133-134: LGTM! Theprunedfield is correctly added as an optional, nullable GraphQL field, aligning with the model's requirements.libs/application/ui-shell/src/lib/ApplicationForm.tsx (1)
79-81: The handling of pruned applications using an early return pattern is appropriate and cleanly implemented.libs/api/domains/application/src/lib/application.service.ts (1)
51-53: The logic to handle pruned applications by clearing sensitive data is secure and well-implemented.
jonnigs
force-pushed
the
feat/handle-pruned-application-links
branch
from
4f1bd07 to
637d715
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (7)
- apps/application-system/api/src/app/modules/application/dto/application.response.dto.ts (1 hunks)
- libs/api/domains/application/src/lib/application.model.ts (1 hunks)
- libs/api/domains/application/src/lib/application.service.ts (1 hunks)
- libs/application/core/src/lib/messages.ts (1 hunks)
- libs/application/graphql/src/lib/fragments/application.ts (1 hunks)
- libs/application/ui-shell/src/components/ErrorShell.tsx (2 hunks)
- libs/application/ui-shell/src/lib/ApplicationForm.tsx (3 hunks)
Files skipped from review as they are similar to previous changes (6)
- apps/application-system/api/src/app/modules/application/dto/application.response.dto.ts
- libs/api/domains/application/src/lib/application.model.ts
- libs/api/domains/application/src/lib/application.service.ts
- libs/application/core/src/lib/messages.ts
- libs/application/graphql/src/lib/fragments/application.ts
- libs/application/ui-shell/src/components/ErrorShell.tsx
Additional context used
Path-based instructions (1)
libs/application/ui-shell/src/lib/ApplicationForm.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/ui-shell/src/lib/ApplicationForm.tsx
[error] 48-49: Expected an expression but instead found 'const'. (parse)
Expected an expression here.
[error] 50-50: Expected a statement but instead found ')'. (parse)
Expected a statement here.
[error] 158-158: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
[error] 49-49: Shouldn't redeclare 'delegationsChecked'. Consider to delete it or rename it. (lint/suspicious/noRedeclare)
'delegationsChecked' is defined here:
[error] 49-49: Shouldn't redeclare 'setDelegationsChecked'. Consider to delete it or rename it. (lint/suspicious/noRedeclare)
'setDelegationsChecked' is defined here:
Additional comments not posted (3)
libs/application/ui-shell/src/lib/ApplicationForm.tsx (3)
36-40: Refine TypeScriptPropsdefinition for enhanced type safety.
79-81: Proper implementation of the pruned application handling logic.
195-199: The structure ofApplicationFormexport is correctly maintained.
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
libs/application/ui-shell/src/lib/ApplicationForm.tsx (1)
Line range hint
156-156: Consider using optional chaining to ensure safety when accessing nested properties.- const currentRole = stateInformation.roles.find((r) => r.id === role) + const currentRole = stateInformation?.roles?.find((r) => r.id === role)
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- libs/application/ui-shell/src/lib/ApplicationForm.tsx (3 hunks)
Additional context used
Path-based instructions (1)
libs/application/ui-shell/src/lib/ApplicationForm.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/ui-shell/src/lib/ApplicationForm.tsx
[error] 156-156: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
Additional comments not posted (4)
libs/application/ui-shell/src/lib/ApplicationForm.tsx (4)
36-40: Refine TypeScriptPropsdefinition for enhanced type safety.
48-48: Ensure the negation logic fordelegationsCheckedis clear and intentional.
77-79: Correct implementation to handle pruned applications by showing an error shell.
193-197: Ensure theApplicationFormcomponent correctly passes all required props toApplicationLoader.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
jonnigs
force-pushed
the
feat/handle-pruned-application-links
branch
from
e5fcb1b to
966c6e1
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
libs/application/ui-shell/src/lib/ApplicationForm.tsx (1)
Line range hint
156-156: Consider using optional chaining for better safety and readability.- const currentRole = stateInformation.roles.find((r) => r.id === role) + const currentRole = stateInformation?.roles?.find((r) => r.id === role)Using optional chaining here prevents potential runtime errors if
stateInformationorrolesis undefined.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (7)
- apps/application-system/api/src/app/modules/application/dto/application.response.dto.ts (1 hunks)
- libs/api/domains/application/src/lib/application.model.ts (1 hunks)
- libs/api/domains/application/src/lib/application.service.ts (1 hunks)
- libs/application/core/src/lib/messages.ts (1 hunks)
- libs/application/graphql/src/lib/fragments/application.ts (1 hunks)
- libs/application/ui-shell/src/components/ErrorShell.tsx (2 hunks)
- libs/application/ui-shell/src/lib/ApplicationForm.tsx (3 hunks)
Files skipped from review as they are similar to previous changes (6)
- apps/application-system/api/src/app/modules/application/dto/application.response.dto.ts
- libs/api/domains/application/src/lib/application.model.ts
- libs/api/domains/application/src/lib/application.service.ts
- libs/application/core/src/lib/messages.ts
- libs/application/graphql/src/lib/fragments/application.ts
- libs/application/ui-shell/src/components/ErrorShell.tsx
Additional context used
Path-based instructions (1)
libs/application/ui-shell/src/lib/ApplicationForm.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/ui-shell/src/lib/ApplicationForm.tsx
[error] 156-156: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
Additional comments not posted (3)
libs/application/ui-shell/src/lib/ApplicationForm.tsx (3)
36-40: Refine TypeScriptPropsdefinition for enhanced type safety.
48-48: Simplify the conditional expression for better readability.
77-79: Handle pruned applications by displaying an appropriate error shell.This change correctly implements the handling for pruned applications, making them inaccessible as intended by the PR objectives.
What
If a user has a link to his application, he can enter it and edit even if it's been pruned
Why
An application that has been pruned should not be accessible
Screenshots / Gifs
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
ApplicationFormto use a definedPropstype for better code clarity.Internationalization