feat(application-system): Add notifications support to pruning #17029
Conversation
WalkthroughThe changes in this pull request enhance the application lifecycle management by integrating user notification capabilities. This includes modifications to the 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
|
Datadog ReportAll test runs ✅ 23 Total Test Services: 0 Failed, 21 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (2) |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17029 +/- ##
==========================================
+ Coverage 35.66% 35.81% +0.15%
==========================================
Files 6947 6935 -12
Lines 147519 147435 -84
Branches 41942 42189 +247
==========================================
+ Hits 52612 52810 +198
+ Misses 94907 94625 -282 Flags with carried forward coverage won't be shown. Click here to find out more.
... and 170 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (17)
apps/application-system/api/src/app/modules/application/lifecycle/application-lifecycle.service.ts (2)
Line range hint
132-147: Optimize notification sending by handling errors per applicationTo prevent one failed notification from stopping the entire process, handle errors within the loop when sending notifications.
Apply this diff to catch errors for each notification:
for (const { application } of success) { const notification = this.pruneNotifications.get(application.id) if (notification) { + try { await this.sendPrunedNotification(notification, application.id) + } catch (error) { + this.logger.error( + `Failed to send notification for application ${application.id}: ${error}`, + ) + } } }🧰 Tools
🪛 Biome (1.9.4)
[error] 157-157: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
157-157: Simplify the conditional using optional chainingMake the condition more concise by utilizing optional chaining for better readability.
Apply this diff:
- if (lifeCycle && lifeCycle.shouldBePruned && lifeCycle.pruneMessage) { + if (lifeCycle?.shouldBePruned && lifeCycle.pruneMessage) {🧰 Tools
🪛 Biome (1.9.4)
[error] 157-157: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
libs/clients/user-notification/src/lib/userNotificationEagerClient.module.ts (1)
6-7: Consider documenting the eager loading strategy.The eager loading pattern is used here, which can impact the application's startup time. Consider adding a comment explaining why eager loading is preferred over lazy loading in this case.
+// Eager loading is used to ensure notification APIs are immediately available +// during application startup, which is critical for the pruning notification system @Module({ providers: eagerExportedApis, exports: eagerExportedApis, })libs/application/types/src/lib/ApplicationLifecycle.ts (2)
1-6: Add JSDoc comment to document the pruneMessage property.The new property is well-typed, but adding documentation would improve maintainability.
export interface ApplicationLifecycle { isListed: boolean pruneAt: Date | null + /** Message to be displayed when the application is pruned */ pruneMessage?: string }
19-22: Enhance PruningNotification type with documentation and validation.The type structure is good, but consider these improvements:
- Add JSDoc explaining the difference between external and internal bodies
- Consider using a more specific type for notificationTemplateId
+/** + * Represents a notification to be sent during application pruning + * @property externalBody - Message shown to external users/applicants + * @property internalBody - Message shown to internal users/staff + * @property notificationTemplateId - ID of the notification template to use + */ export type PruningNotification = { externalBody?: string internalBody?: string - notificationTemplateId: string + notificationTemplateId: `pruning-${string}` // Template literal type for better type safety }libs/application/template-api-modules/src/lib/index.ts (1)
10-12: Consider using named exports for better tree-shaking.While the current exports are functional, consider using named exports directly at the source files instead of re-exporting. This can improve tree-shaking by making the module dependencies more explicit.
Example refactor:
-export { NotificationsService } from './notification/notifications.service' -export { ApplicationsNotificationsModule } from './notification/notifications.module' -export { NotificationType } from './notification/notificationsTemplates' +export * from './notification'Then in
./notification/index.ts:export const NotificationsService = /* ... */ export const ApplicationsNotificationsModule = /* ... */ export const NotificationType = /* ... */libs/clients/user-notification/src/lib/eagerApiConfiguration.ts (1)
14-22: Add type safety and null checks for configuration.The configuration usage could benefit from additional type safety and null checks.
Consider adding these improvements:
- basePath: `${config.basePath}`, + basePath: config.basePath ?? throw new Error('basePath is required in UserNotificationClientConfig'),Also, consider creating an interface for the enhanced fetch options:
interface EnhancedFetchOptions { name: string; organizationSlug: string; }libs/application/core/src/lib/constants.ts (1)
23-30: Add JSDoc documentation and input validationWhile the implementation is clean, consider these improvements:
- Add JSDoc documentation to explain:
- Purpose of the function
- Parameters and return type
- Example usage
- Add validation for the message parameter
Here's a suggested implementation:
+/** + * Creates a new lifecycle configuration that extends the default lifecycle with pruning notifications. + * + * @param message - Either a static notification or a function that generates a notification based on the application + * @returns A new StateLifeCycle with pruning notification support + * + * @example + * // Static notification + * const lifecycle = defaultLifecycleWithPruneMessage({ title: 'Pruning soon', body: 'Your application will be removed' }); + * + * // Dynamic notification + * const lifecycle = defaultLifecycleWithPruneMessage((app) => ({ + * title: `Pruning ${app.name}`, + * body: `Application ${app.id} will be removed` + * })); + */ export const defaultLifecycleWithPruneMessage = ( message: | PruningNotification | ((application: PruningApplication) => PruningNotification), ) => ({ + if (!message) { + throw new Error('Prune message is required'); + } ...DefaultStateLifeCycle, pruneMessage: message, })apps/application-system/api/src/app/modules/application/lifecycle/test/application-lifecycle-unit.spec.ts (2)
Line range hint
63-76: Add notification-related mocks and verificationsThe mock implementations don't appear to handle the new notification functionality. Consider:
- Adding notification-related methods to the mocks
- Verifying notification fields in the update mock
Let's verify if there are notification-related interfaces or types that should be mocked:
#!/bin/bash # Search for notification-related interfaces and types ast-grep --pattern 'interface $name { $$$ notify$_($$$) $$$ }'
Line range hint
94-157: Expand test coverage for notification functionalityThe test suite needs additional test cases to verify the new notification support:
- Verify notifications are sent on successful pruning
- Verify notification handling for pruning failures
- Validate notification content and format
Let's check the notification-related test patterns in other files:
#!/bin/bash # Search for notification-related test patterns rg -t ts "describe.*notification" --glob "**/*.spec.ts"libs/application/templates/README.md (3)
3-8: Documentation improvements neededConsider the following enhancements to improve clarity:
- Add TypeScript type definitions for
PruningNotificationandPruningApplication- Explain when pruning typically occurs
- Add a comma after "which" in line 7 for better readability
Here's a suggested addition:
interface PruningNotification { externalBody: string; internalBody: string; notificationTemplateId: string; } interface PruningApplication { // Add relevant application properties }🧰 Tools
🪛 LanguageTool
[uncategorized] ~7-~7: Possible missing comma found.
Context: ...ument containing aPruningApplicationobject which contains the application's data. ...(AI_HYDRA_LEO_MISSING_COMMA)
9-29: Code example needs refinementThe example is helpful but could be improved:
- Fix typo: "accimplished" → "accomplished" in line 9
- Add type annotation for the
applicationparameter- Consider explaining the format of
notificationTemplateIdHere's the suggested improvement:
stateMachineConfig: { initial: States.DRAFT, states: { [States.DRAFT]: { meta: { name: application.general.name.defaultMessage, progress: 0.4, - lifecycle: defaultLifecycleWithPruneMessage((application: any) => ({ + lifecycle: defaultLifecycleWithPruneMessage((application: PruningApplication) => ({ externalBody: `Application has been in draft for more than 30 days and has been pruned.`, internalBody: `Application for ${application.externalData.nationalRegistry.data.fullName} has been in draft for more than 30 days. Please note that if desired the application may be re-submitted.`, notificationTemplateId: 'HNIPP.AS.*', // Format: <system>.<module>.<specific-template> })),🧰 Tools
🪛 LanguageTool
[typographical] ~9-~9: After the expression ‘for example’ a comma is usually used.
Context: ... to use the body variables or not. For example an application might want to send a not...(COMMA_FOR_EXAMPLE)
9-9: Add missing commaAdd a comma after "For example" to improve readability.
-For example an application might want to send a notification +For example, an application might want to send a notification🧰 Tools
🪛 LanguageTool
[typographical] ~9-~9: After the expression ‘for example’ a comma is usually used.
Context: ... to use the body variables or not. For example an application might want to send a not...(COMMA_FOR_EXAMPLE)
libs/application/types/src/lib/StateMachine.ts (1)
77-90: Consider extracting the complex type for better maintainabilityWhile the implementation is correct, consider extracting the function parameter type into a separate named type for better readability and reuse:
type PruningNotificationContext = Pick< ApplicationWithAttachments, | 'id' | 'typeId' | 'state' | 'applicant' | 'attachments' | 'answers' | 'externalData' > type PruningMessageFunction = ( application: PruningNotificationContext ) => PruningNotificationThen simplify the original type:
pruneMessage?: PruningNotification | PruningMessageFunctionapps/application-system/api/src/app/modules/application/lifecycle/test/application-lifecycle-notification.spec.ts (3)
1-19: Consider organizing imports by source typeConsider grouping imports into these categories for better maintainability:
- External dependencies (@nestjs)
- Internal shared modules (@island.is)
- Local modules (../)
+// External dependencies +import { Test } from '@nestjs/testing' + +// Internal shared modules import { LOGGER_PROVIDER } from '@island.is/logging' import { FileService } from '@island.is/application/api/files' import { CreateNotificationResponse, NotificationsApi, } from '@island.is/clients/user-notification' -import { Test } from '@nestjs/testing' import { getApplicationTemplateByTypeId } from '@island.is/application/template-loader' import { TestApp } from '@island.is/testing/nest' import { ApplicationTypes, PruningApplication, } from '@island.is/application/types' +// Local modules import { ApplicationLifeCycleService } from '../application-lifecycle.service' import { ApplicationService } from '@island.is/application/api/core' import { ApplicationChargeService } from '../../charge/application-charge.service'
24-60: Improve type safety for mock loggerThe mock logger could benefit from proper typing to ensure type safety and better IDE support.
+import { Logger } from '@island.is/logging' + - const mockLogger = { + const mockLogger: jest.Mocked<Logger> = { info: jest.fn(), error: jest.fn(), - child: () => mockLogger, + child: jest.fn().mockReturnThis(), }
250-311: Enhance error handling test coverageWhile the basic error case is covered, consider adding tests for:
- Different types of API errors (network, timeout, validation)
- Retry mechanism verification (if implemented)
- Specific error message validation
Example test case:
it('should handle API timeout errors appropriately', async () => { const mockNotification = { recipient: 'test-user', templateId: 'test-template', args: [], } const mockApplicationId = 'test-id' const timeoutError = new Error('Request timeout') timeoutError.name = 'TimeoutError' notificationApi.notificationsControllerCreateHnippNotification.mockRejectedValue(timeoutError) await service['sendPrunedNotification'](mockNotification, mockApplicationId) expect(mockLogger.error).toHaveBeenCalledWith( expect.stringContaining('Request timeout'), ) // Add assertions for retry mechanism if implemented })
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (13)
apps/application-system/api/src/app/modules/application/lifecycle/application-lifecycle.module.ts(2 hunks)apps/application-system/api/src/app/modules/application/lifecycle/application-lifecycle.service.ts(7 hunks)apps/application-system/api/src/app/modules/application/lifecycle/test/application-lifecycle-notification.spec.ts(1 hunks)apps/application-system/api/src/app/modules/application/lifecycle/test/application-lifecycle-unit.spec.ts(1 hunks)libs/application/api/core/src/lib/application/application.service.ts(1 hunks)libs/application/core/src/lib/constants.ts(2 hunks)libs/application/template-api-modules/src/lib/index.ts(1 hunks)libs/application/templates/README.md(1 hunks)libs/application/types/src/lib/ApplicationLifecycle.ts(1 hunks)libs/application/types/src/lib/StateMachine.ts(2 hunks)libs/clients/user-notification/src/index.ts(1 hunks)libs/clients/user-notification/src/lib/eagerApiConfiguration.ts(1 hunks)libs/clients/user-notification/src/lib/userNotificationEagerClient.module.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
apps/application-system/api/src/app/modules/application/lifecycle/application-lifecycle.module.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."
apps/application-system/api/src/app/modules/application/lifecycle/application-lifecycle.service.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."
apps/application-system/api/src/app/modules/application/lifecycle/test/application-lifecycle-notification.spec.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."
apps/application-system/api/src/app/modules/application/lifecycle/test/application-lifecycle-unit.spec.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/application/api/core/src/lib/application/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/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/template-api-modules/src/lib/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/README.md (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/types/src/lib/ApplicationLifecycle.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/types/src/lib/StateMachine.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/user-notification/src/index.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/clients/user-notification/src/lib/eagerApiConfiguration.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/user-notification/src/lib/userNotificationEagerClient.module.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 (1.9.4)
apps/application-system/api/src/app/modules/application/lifecycle/application-lifecycle.service.ts
[error] 157-157: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 LanguageTool
libs/application/templates/README.md
[uncategorized] ~7-~7: Possible missing comma found.
Context: ...ument containing a PruningApplication object which contains the application's data. ...
(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~9-~9: After the expression ‘for example’ a comma is usually used.
Context: ... to use the body variables or not. For example an application might want to send a not...
(COMMA_FOR_EXAMPLE)
🔇 Additional comments (20)
apps/application-system/api/src/app/modules/application/lifecycle/application-lifecycle.service.ts (1)
37-37: Verify the provider for NotificationsApi in the module
Ensure that NotificationsApi is properly imported and provided in the module to enable correct dependency injection.
libs/clients/user-notification/src/lib/userNotificationEagerClient.module.ts (3)
5-9: LGTM! Module structure follows NestJS best practices.
The module configuration correctly provides and exports the APIs, following the standard NestJS module pattern.
3-3: Verify the exported APIs configuration.
Let's ensure the eagerExportedApis are properly configured for the notification system.
✅ Verification successful
The eager exported APIs are properly configured
The verification shows that:
- The
eagerExportedApisare correctly configured ineagerApiConfiguration.tswithUserNotificationApiandNotificationsApi - The configuration includes proper setup with
basePathand enhanced fetch - The
UserNotificationEagerClientModuleis correctly imported and used in the application lifecycle module
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the configuration of eagerExportedApis
# Check the configuration file content
echo "Checking eagerApiConfiguration.ts content:"
rg -A 10 "eagerExportedApis" ./libs/clients/user-notification/src/lib/
# Verify the APIs are properly imported in the application lifecycle module
echo "Checking ApplicationLifecycleModule integration:"
rg -A 5 "UserNotificationEagerClientModule" ./apps/application-system/api/src/
Length of output: 3773
1-9: Ensure reusability across NextJS apps.
As this is in the libs directory, we need to ensure it follows the coding guidelines for reusability.
✅ Verification successful
Let me gather more information about the module's dependencies and implementation to ensure a comprehensive verification.
Let me try one more verification to check the workspace configuration and dependencies:
The module is framework-agnostic and follows reusability guidelines
Based on the verification results:
- The module is built using NestJS, which is framework-agnostic and can be used with any JavaScript/TypeScript application
- No Next.js specific dependencies or implementations are found in the module
- The module exports clean interfaces through its index.ts
- The implementation uses standard TypeScript/JavaScript features and common libraries
- It's currently being used in the application system API, demonstrating its reusability
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the module's reusability and dependencies
# Check for any NextJS-specific dependencies that might limit reusability
echo "Checking for NextJS dependencies:"
rg -l "next" ./libs/clients/user-notification/
# Check if the module is imported by multiple apps
echo "Checking module usage across apps:"
rg -l "UserNotificationEagerClientModule" ./apps/
Length of output: 352
Script:
#!/bin/bash
# Check the actual implementation and dependencies of the module
echo "Checking module implementation:"
cat ./libs/clients/user-notification/src/lib/eagerApiConfiguration.ts
# Check package.json for dependencies
echo -e "\nChecking package dependencies:"
cat ./libs/clients/user-notification/package.json
# Check if there are any framework-specific implementations
echo -e "\nChecking for framework-specific code:"
rg -l -t typescript "import.*from.*@next" ./libs/clients/user-notification/
Length of output: 1353
Script:
#!/bin/bash
# Check workspace configuration for dependencies
echo "Checking workspace package.json:"
cat package.json
# Check for any Next.js related configurations
echo -e "\nChecking for Next.js configurations:"
fd -t f "next.config.js|next.config.ts" .
# Check the module's exports
echo -e "\nChecking module exports:"
cat ./libs/clients/user-notification/src/index.ts
Length of output: 19442
libs/clients/user-notification/src/index.ts (2)
3-3: LGTM! Verify the module file exists.
The export syntax follows the established pattern in the file.
Let's verify the existence of the eager client module:
✅ Verification successful
Module exists and follows NestJS module pattern
The exported module exists and follows the correct NestJS module pattern with proper decorators and configuration. The eager loading strategy is properly implemented through the eagerExportedApis configuration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence of the UserNotificationEagerClientModule file
fd userNotificationEagerClient.module.ts libs/clients/user-notification/src/lib
Length of output: 158
Script:
#!/bin/bash
# Let's verify the module's content to understand its implementation
cat libs/clients/user-notification/src/lib/userNotificationEagerClient.module.ts
Length of output: 309
3-3: Consider the impact of eager loading on tree-shaking.
Since this is an eager-loaded module being exported from a shared library, we should ensure it doesn't negatively impact tree-shaking and bundle sizes for applications that don't use the notification features.
Let's check how this module is being used across the codebase:
Consider these recommendations:
- If the module is only used in specific features, consider using dynamic imports
- Document the rationale for eager loading in the module file
- Ensure the module has minimal side effects to preserve tree-shaking capabilities
✅ Verification successful
Eager loading is justified in this case
The eager loading implementation is well-designed and necessary here because:
- The module provides essential API clients (UserNotificationApi, NotificationsApi) that are used across multiple application modules
- The implementation is tree-shaking friendly as it only exports factory providers without side effects
- The configuration is properly injected through NestJS's dependency injection system
- The module is used in core application features that require immediate availability of notification services
The current implementation follows best practices for shared API clients in a NestJS application, and the eager loading won't negatively impact tree-shaking.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check usage patterns of the eager client module
# This will help understand if the eager loading is necessary and its impact on tree-shaking
# Search for imports of the eager client module
rg -l "UserNotificationEagerClientModule"
# Check if there are any dynamic imports that could be used instead
ast-grep --pattern 'import($$$)' | rg "UserNotificationEagerClient"
Length of output: 328
Script:
#!/bin/bash
# Let's examine how this module is being used in the application lifecycle module
cat "apps/application-system/api/src/app/modules/application/lifecycle/application-lifecycle.module.ts"
# Let's also check the implementation of the eager client module
cat "libs/clients/user-notification/src/lib/userNotificationEagerClient.module.ts"
# Check for any providers or services that might require eager loading
rg -A 5 "providers.*UserNotification"
Length of output: 2332
Script:
#!/bin/bash
# Let's check what APIs are eagerly exported
cat "libs/clients/user-notification/src/lib/eagerApiConfiguration.ts"
# Let's also check if there are any initialization or side effects
rg -l "constructor.*UserNotification"
# Check for any direct dependencies on this module
rg -A 3 "import.*@island.is/clients/user-notification"
Length of output: 3824
libs/application/types/src/lib/ApplicationLifecycle.ts (1)
8-17: Document the PruningApplication type and verify property selection.
The type is well-structured, but please:
- Add documentation explaining its purpose and usage
- Verify that all picked properties are necessary for pruning operations
✅ Verification successful
Let's also check the actual implementation where this type is used:
And let's check if there are any related test files that might give us more context:
All picked properties in PruningApplication type are necessary and correctly used
Based on the implementation evidence:
id: Used for identifying applications and updating them during pruningattachments: Required for S3 file cleanup and pruninganswers: Cleared during pruning processexternalData: Cleared during pruning processtypeId: Used to get application template and state machine configstate: Used to determine lifecycle configurationapplicant: Used for notification purposes
The type definition follows the single responsibility principle and includes only the properties needed for the pruning operation, as verified by the implementation and tests.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check usage of picked properties in pruning-related code
# Search for usage of these properties in pruning-related code
echo "Searching for property usage in pruning context..."
for prop in id attachments answers externalData typeId state applicant; do
echo "Checking usage of $prop:"
rg -A 2 "pruning.*\.$prop|prune.*\.$prop"
done
Length of output: 3450
Script:
#!/bin/bash
# Let's search for pruning-related code to understand the context better
echo "Searching for pruning-related code..."
rg -l "prune|pruning" --type ts
echo -e "\nChecking implementation details..."
ast-grep --pattern 'function $FUNC($$$) {
$$$
}' | rg -A 10 "prune|pruning"
Length of output: 9627
Script:
rg -A 5 "PruningApplication" --type ts
Length of output: 7255
Script:
fd -e test.ts -e spec.ts | xargs rg -l "prune|pruning"
Length of output: 670
Script:
# Let's check the actual pruning implementation to verify property usage
echo "Checking pruning implementation..."
rg -A 10 "findAllDueToBePruned|pruneApplication" --type ts
# Let's also check the lifecycle tests to understand the pruning process
echo -e "\nChecking pruning tests..."
cat "apps/application-system/api/src/app/modules/application/lifecycle/test/application-lifecycle-unit.spec.ts"
Length of output: 13418
libs/application/template-api-modules/src/lib/index.ts (1)
10-12: Verify reusability of notification components.
The notification-related exports form a cohesive API. However, since this is in the libs directory, we should verify that these components are designed for reuse across different NextJS applications.
✅ Verification successful
Notification components are designed for reuse and follow modular architecture
The verification shows that the notification components are well-structured and properly isolated:
- The components follow a modular architecture with clear separation of concerns (service, module, and types)
- The implementation uses standard NestJS patterns (@Injectable, @module)
- Dependencies are from common shared libraries (@island.is/logging, @nestjs/common)
- No direct dependencies on application-specific code were found in the notification components
- The exports are used by multiple parts of the system, demonstrating their reusability
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the notification components are imported by multiple applications
# and verify their independence from application-specific logic
# Search for imports of NotificationsService
echo "Checking NotificationsService usage across apps:"
rg --type ts "import.*NotificationsService.*from.*template-api-modules" apps/
# Search for imports of ApplicationsNotificationsModule
echo "Checking ApplicationsNotificationsModule usage across apps:"
rg --type ts "import.*ApplicationsNotificationsModule.*from.*template-api-modules" apps/
# Check for application-specific dependencies in notification components
echo "Checking for application-specific dependencies:"
rg --type ts "import.*from.*@island.is/application-system" libs/application/template-api-modules/src/lib/notification/
Length of output: 632
Script:
#!/bin/bash
# Let's try a broader search to understand the notification components usage and dependencies
# Check the implementation of notification components
echo "Checking notification components implementation:"
ast-grep --pattern 'export class NotificationsService {
$$$
}'
echo "\nChecking module implementation:"
ast-grep --pattern 'export class ApplicationsNotificationsModule {
$$$
}'
# Look for any imports in notification files
echo "\nChecking all imports in notification files:"
fd notification libs/application/template-api-modules/src/lib/ --type f --exec rg --type ts "^import" {}
# Check if these components are used anywhere in the codebase
echo "\nChecking usage across the entire codebase:"
rg --type ts "NotificationsService|ApplicationsNotificationsModule|NotificationType" -g '!*.test.ts' -g '!*.spec.ts'
Length of output: 49955
libs/clients/user-notification/src/lib/eagerApiConfiguration.ts (2)
1-9: LGTM! Well-structured imports with proper typing.
The imports are well-organized, using named imports which support tree-shaking. The use of generated types from the fetch directory ensures type safety.
11-26: Verify the exported APIs are properly generated.
The code relies on generated API classes. Let's verify their existence and structure.
libs/application/core/src/lib/constants.ts (2)
1-5: LGTM! Clean type imports
The imports follow TypeScript best practices by explicitly importing types from a shared module, promoting type safety and reusability across the application.
Line range hint 1-30: Implementation follows coding guidelines
The code successfully meets all requirements for libs/** files:
- ✓ Reusable across different NextJS apps (generic constants and utilities)
- ✓ Strong TypeScript typing
- ✓ Effective tree-shaking support through named exports
apps/application-system/api/src/app/modules/application/lifecycle/application-lifecycle.module.ts (3)
19-22: LGTM! Clean import organization
The new imports for user notification functionality are well-organized using destructured imports, maintaining consistency with the existing code style.
33-33: Verify eager loading implementation
The use of UserNotificationEagerClientModule suggests eager loading of notification APIs. While this ensures immediate availability of notification services, it could impact startup time.
✅ Verification successful
Eager loading of notification module is consistent with the codebase architecture
Based on the codebase analysis, the use of UserNotificationEagerClientModule is appropriate because:
- The module is used in the application lifecycle context where immediate availability of notifications is crucial
- The codebase shows consistent usage of eager loading for notification-related functionality across different modules
- No performance issues are evident from lazy-loaded alternatives, as the notification client is a core functionality in the application lifecycle
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if there are any lazy-loaded alternatives or if eager loading is consistently used across the codebase
# Search for other notification-related modules to verify consistency in loading strategy
rg -t typescript "Notification.*Module"
# Check for any lazy loading patterns in the codebase
ast-grep --pattern 'forRootAsync|forFeatureAsync'
Length of output: 128
Script:
#!/bin/bash
# Search for notification-related modules and their loading patterns
rg "Notification.*Module"
# Look for any module imports or declarations related to notifications
rg "import.*Notification.*Module"
# Search for lazy loading patterns in the codebase
rg "loadChildren|forRoot|forFeature"
# Check the specific module implementation
fd -e ts "user-notification.*module"
Length of output: 56000
37-42: Verify config loading order
The UserNotificationClientConfig is placed first in the load array. While this works, we should verify that there are no dependencies between these configs that would require a specific order.
✅ Verification successful
Let me analyze the config dependencies by searching for any imports or usage of these configs.
Based on the code analysis, I can now provide a final response about the config loading order:
Config loading order is correct - no dependencies between configs
The current order in the load array is appropriate because:
- Each config is independently defined and used:
UserNotificationClientConfigis used for notification client setupsigningModuleConfigis used by signing serviceApplicationFilesConfigis used for application file processingFileStorageConfigis used by file storage service
The imports and usage patterns show that these configs are not dependent on each other - they are used independently in their respective services and modules. The order of loading does not impact their functionality since they don't reference each other's values.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential config dependencies and overrides
# Search for references to these configs to understand dependencies
rg -t typescript "UserNotificationClientConfig|signingModuleConfig|ApplicationFilesConfig|FileStorageConfig"
# Look for any config merge or override patterns
ast-grep --pattern 'ConfigModule.forRoot({
$$$
load: [$$$]
$$$
})'
Length of output: 82151
Script:
#!/bin/bash
# Search for imports and usage of these specific configs
rg -t ts "import.*UserNotificationClientConfig|import.*signingModuleConfig|import.*ApplicationFilesConfig|import.*FileStorageConfig" -A 5
# Search for any dependencies between these configs in their definitions
fd -e ts -e js "UserNotificationClientConfig|signingModuleConfig|ApplicationFilesConfig|FileStorageConfig" --exec cat {}
Length of output: 11501
apps/application-system/api/src/app/modules/application/lifecycle/test/application-lifecycle-unit.spec.ts (1)
18-18: Consider adding test cases for different application states
While adding the 'draft' state is good, the test suite should verify pruning behavior across different application states to ensure comprehensive coverage of the new notification functionality.
Let's check what other states are used in the codebase:
Consider creating additional test cases using different application states to ensure the pruning notifications work correctly across all possible states.
✅ Verification successful
Based on the search results, I can now provide a final response:
The 'draft' state is correctly used in the test files
The test files consistently use 'draft' state for testing pruning functionality, which aligns with the application lifecycle. This is appropriate because:
- The state is used in both notification and unit tests
- It's used consistently across different test scenarios
- The state matches the common application lifecycle pattern seen in other templates (from earlier search results showing States enums)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find all possible application states defined in the codebase
ast-grep --pattern 'type ApplicationState = $_'
Length of output: 49
Script:
#!/bin/bash
# Let's try different approaches to find application states
# 1. Search for enum or type definitions related to application state
rg -i "enum.*state|type.*state" -A 5
# 2. Search for state assignments or comparisons
rg "state: ['\"](.*?)['\"]" --only-matching
# 3. Look for constants or variables defining states
rg "APPLICATION_STATE|ApplicationState" -A 3
Length of output: 102608
Script:
#!/bin/bash
# Let's search specifically for application states in the application system module
rg -A 5 "enum.*State|type.*State" apps/application-system/
Length of output: 702
Script:
#!/bin/bash
# Let's try to find application states in the test file and surrounding context
rg -B 5 -A 5 "state: ['\"](.*?)['\"]" apps/application-system/api/src/app/modules/application/lifecycle/test/
Length of output: 6256
libs/application/types/src/lib/StateMachine.ts (2)
11-15: LGTM! Import statements are well-organized
The new imports for ApplicationWithAttachments and PruningNotification are properly structured and align with TypeScript best practices.
Also applies to: 19-19
80-89: Verify the required fields for pruning notifications
Please confirm if all the picked fields (id, typeId, state, applicant, attachments, answers, externalData) are necessary for generating pruning notifications. Including sensitive data like answers and externalData might be unnecessary.
✅ Verification successful
Based on the code analysis, I can now provide a definitive response about the fields used in pruning notifications:
The picked fields are necessary for pruning notifications
Looking at the implementation in application-lifecycle.service.ts, these fields are used as follows:
id: Required for notification tracking and mappingtypeId: Used to get application template and state configurationstate: Used to determine the pruning configurationapplicant: Required as the notification recipientattachments: Required for pruning attachmentsanswersandexternalData: Used by template-specific pruning message functions to generate customized notification content
The fields are used either directly in the notification system or indirectly through template-specific pruning message functions that can use this data to generate appropriate notification content.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usage of these fields in pruning notification generation
ast-grep --pattern 'pruneMessage?($application) {
$$$
$application.$_
$$$
}'
Length of output: 84
Script:
#!/bin/bash
# Search for pruning-related code and notifications
rg -l "prune|notification" --type ts
# After finding relevant files, let's examine their content
rg "prune.*notification|notification.*prune" -A 5 -B 5 --type ts
Length of output: 35760
Script:
#!/bin/bash
# Let's examine the implementation of pruneMessage to understand field usage
ast-grep --pattern 'pruneMessage($application) {
$$$
}'
# Also search for pruneMessage property definitions
rg "pruneMessage.*=.*{" -A 5 -B 5 --type ts
Length of output: 205
Script:
#!/bin/bash
# Search for pruneMessage implementations and definitions with fixed regex
rg "pruneMessage" -A 10 -B 5 --type ts
# Also look for the StateMachine usage context
rg "StateMachine" -A 10 -B 5 --type ts
Length of output: 48225
libs/application/api/core/src/lib/application/application.service.ts (2)
Line range hint 240-251: Consider optimizing query performance and adding documentation.
The query conditions are correct, but consider these improvements:
- Add JSDoc comments explaining the pruning criteria
- Ensure database indexes exist for optimal query performance
Add documentation:
+ /**
+ * Retrieves all applications that are due to be pruned.
+ * An application is considered due when:
+ * 1. It has a pruneAt date set in the past
+ * 2. It hasn't been pruned yet (pruned = false)
+ *
+ * @returns Promise<Application[]> Applications ready for pruning
+ */
async findAllDueToBePruned(): Promise<Application[]> {Let's verify the existing indexes:
✅ Verification successful
Indexes are already optimized for the pruning query
The codebase already has a partial index pruned_and_pruneAt_idx on the application table that covers both conditions (pruned and prune_at) with the exact filtering criteria used in the query. The only remaining suggestion is to add documentation.
Add documentation:
+ /**
+ * Retrieves all applications that are due to be pruned.
+ * An application is considered due when:
+ * 1. It has a pruneAt date set in the past
+ * 2. It hasn't been pruned yet (pruned = false)
+ *
+ * @returns Promise<Application[]> Applications ready for pruning
+ */
async findAllDueToBePruned(): Promise<Application[]> {🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for database migration files that might contain index definitions
rg -A 5 "CREATE.*INDEX.*(?:prune_at|pruned)"
Length of output: 687
229-239: Verify the necessity of all retrieved fields for notifications.
While the additional fields (applicant, answers, externalData) are likely needed for notification content, retrieving all fields might impact performance. Consider:
- Documenting which notification templates require these fields
- Creating a specific type for pruning-related fields to maintain type safety
Let's verify the usage of these fields in notification templates:
Consider creating a dedicated DTO (Data Transfer Object) for pruning notifications to:
- Explicitly document required fields
- Optimize database queries
- Maintain type safety
Example:
interface PruningNotificationDTO {
id: string;
typeId: string;
applicant: string;
// Add only the specific answer fields needed
answers: Pick<FormValue, 'relevantFields'>;
// Add only the specific external data needed
externalData: Pick<ExternalData, 'relevantFields'>;
}✅ Verification successful
The additional fields are necessary for pruning notifications.
The code analysis reveals that these fields are required for the pruning notification system:
applicant- Used to identify the recipient of the pruning notificationanswersandexternalData- Used in pruning message templates to customize notification content
This is evidenced by:
- The
PruningApplicationtype inApplicationLifecycle.tsexplicitly requires these fields - The
preparePrunedNotificationmethod inapplication-lifecycle.service.tsuses these fields to generate notifications - The pruning message can be customized based on application data through a function that receives these fields:
pruneMessage(application: ApplicationWithAttachments)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usage of applicant, answers, or externalData in notification-related files
rg -A 5 "notification.*template.*(?:applicant|answers|externalData)"
# Search for pruning-related notification templates
ast-grep --pattern 'template = {
$$$
pruning
$$$
}'
Length of output: 130
Script:
#!/bin/bash
# Let's try a broader search for notification templates and pruning-related code
rg -l "notification" | xargs rg -A 5 "template"
# Search for any usage of these fields in pruning-related code
rg -A 5 "prune|pruning" | grep -E "applicant|answers|externalData"
# Look for notification-related types or interfaces
ast-grep --pattern 'interface $_ {
$$$
applicant
$$$
}'
# Search for notification service implementations
rg -A 10 "class.*NotificationService|sendNotification"
Length of output: 146826
Script:
#!/bin/bash
# Let's search for pruning-related code and notification templates
rg -A 10 "findAllDueToBePruned|pruneMessage"
# Search for lifecycle configuration related to pruning
rg -A 10 "shouldBePruned.*pruneMessage"
# Look for notification-related code in application lifecycle
rg -A 10 "notification.*prune|prune.*notification"
# Search for the usage of applicant, answers, and externalData in pruning context
rg -l "prune|pruning" | xargs rg -A 5 "applicant|answers|externalData"
Length of output: 105690
...pplication-system/api/src/app/modules/application/lifecycle/application-lifecycle.service.ts
Show resolved
Hide resolved
...em/api/src/app/modules/application/lifecycle/test/application-lifecycle-notification.spec.ts
Show resolved
Hide resolved
...em/api/src/app/modules/application/lifecycle/test/application-lifecycle-notification.spec.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (5)
apps/application-system/api/src/app/modules/application/lifecycle/test/application-lifecycle-notification.spec.ts (3)
24-28: Enhance type safety for mock loggerConsider using TypeScript's type system to ensure the mock logger matches the expected interface:
const mockLogger: Logger = { info: jest.fn(), error: jest.fn(), child: () => mockLogger, } as jest.Mocked<Logger>
250-283: Strengthen assertions in successful notification testThe current test verifies the API call and logging, but should also validate:
- The exact structure of the notification payload
- The correct formatting of template arguments
- The logging of specific notification details
expect( notificationApi.notificationsControllerCreateHnippNotification, ).toHaveBeenCalledWith({ createHnippNotificationDto: mockNotification, }) +expect(mockNotification).toMatchObject({ + recipient: expect.any(String), + templateId: expect.any(String), + args: expect.arrayContaining([ + expect.objectContaining({ + key: expect.any(String), + value: expect.any(String), + }), + ]), +})
285-309: Enhance error handling test coverageThe error handling test should be expanded to cover:
- Different types of API errors (network, validation, etc.)
- Proper error message formatting
- Retry behavior if implemented
it('should handle specific API error types', async () => { const testCases = [ { error: new TypeError('Network error'), expectedMessage: 'Network error' }, { error: new Error('Validation failed'), expectedMessage: 'Validation failed' }, ] for (const { error, expectedMessage } of testCases) { notificationApi.notificationsControllerCreateHnippNotification .mockRejectedValueOnce(error) await service['sendPrunedNotification'](mockNotification, mockApplicationId) expect(mockLogger.error).toHaveBeenCalledWith( expect.stringContaining(expectedMessage) ) } })apps/application-system/api/src/app/modules/application/lifecycle/application-lifecycle.service.ts (2)
157-157: Simplify condition using optional chainingYou can simplify the condition by using optional chaining to improve readability.
Apply this diff:
-if (lifeCycle && lifeCycle.shouldBePruned && lifeCycle.pruneMessage) { +if (lifeCycle?.shouldBePruned && lifeCycle.pruneMessage) {🧰 Tools
🪛 Biome (1.9.4)
[error] 157-157: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
59-59: Avoid unnecessary type assertionsUsing a type assertion here may mask potential type mismatches. Consider updating the return type of
findAllDueToBePruned()to returnPruningApplication[]directly to enhance type safety.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
apps/application-system/api/src/app/modules/application/lifecycle/application-lifecycle.service.ts(7 hunks)apps/application-system/api/src/app/modules/application/lifecycle/test/application-lifecycle-notification.spec.ts(1 hunks)libs/application/types/src/lib/StateMachine.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/application/types/src/lib/StateMachine.ts
🧰 Additional context used
📓 Path-based instructions (2)
apps/application-system/api/src/app/modules/application/lifecycle/application-lifecycle.service.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."
apps/application-system/api/src/app/modules/application/lifecycle/test/application-lifecycle-notification.spec.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."
🪛 Biome (1.9.4)
apps/application-system/api/src/app/modules/application/lifecycle/application-lifecycle.service.ts
[error] 157-157: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (5)
apps/application-system/api/src/app/modules/application/lifecycle/test/application-lifecycle-notification.spec.ts (2)
203-248: Consolidate duplicate test suite setup
The second describe('ApplicationLifeCycleService') block duplicates the test setup. Merge it with the first test suite to maintain better organization and reduce code duplication.
62-200: Add test coverage for additional edge cases
Consider adding test cases for:
- Application with missing applicant field
- Template with invalid notification template ID
- Empty or malformed message content
- Edge cases around state machine configuration
Example test case:
it('should handle missing applicant field', async () => {
const mockApplication = {
id: '123',
typeId: ApplicationTypes.EXAMPLE,
state: 'draft',
// applicant field missing
answers: {},
externalData: {},
attachments: [],
}
const result = await service['preparePrunedNotification'](mockApplication)
expect(result).toBeNull()
expect(mockLogger.error).toHaveBeenCalledWith(
expect.stringContaining('missing applicant field')
)
})apps/application-system/api/src/app/modules/application/lifecycle/application-lifecycle.service.ts (3)
71-76: Handle potential exceptions when preparing notifications
Wrap the call to preparePrunedNotification in a try-catch block to handle any potential errors without interrupting the pruning process.
Apply this diff to add error handling:
for (const { application } of this.processingApplications) {
+ try {
const notification = await this.preparePrunedNotification(application)
if (notification) {
this.pruneNotifications.set(application.id, notification)
}
+ } catch (error) {
+ this.logger.error(
+ `Failed to prepare notification for application ${application.id}: ${error}`,
+ error,
+ )
+ }
}151-187: preparePrunedNotification method implementation looks good
The method correctly constructs the notification with appropriate error handling and type checks.
🧰 Tools
🪛 Biome (1.9.4)
[error] 157-157: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
189-208: sendPrunedNotification method handles notifications effectively
The implementation properly sends notifications with error handling, ensuring robustness in the notification process.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
libs/application/templates/README.md (2)
3-7: Enhance documentation readability with proper formatting and punctuation.Consider these improvements to the documentation:
- Add a comma after "If desired"
- Format type names consistently using backticks
- Structure the explanation with bullet points for better readability
Apply these changes:
## Pruning notifications -If desired it is possible to have a custom message sent to an application's applicant when it is pruned. +If desired, it is possible to have a custom message sent to an application's applicant when it is pruned. This is done by adding a `pruneMessage` object of type `PruningNotification` or a function returning a `PruningNotification` to the application's lifecycle configuration. -When executed, the function will be passed an argument that is a `PruningApplication` object which contains the application's data. The `PruningNotification` object has `externalBody`, `internalBody` and `notificationTemplateId`. The former two can be used to fill in the values for the `externalBody` and `internalBody` template variables in the notification that will be sent to the user. The `notificationTemplateId` is the id of the notification template that will be used to send the notification. It is up to the user whether to use the body variables or not. +When executed, the function will be passed an argument that is a `PruningApplication` object which contains the application's data. The `PruningNotification` object has: +- `externalBody`: Used to fill the external body template variable +- `internalBody`: Used to fill the internal body template variable +- `notificationTemplateId`: The ID of the notification template to use + +It is up to the user whether to use the body variables or not.🧰 Tools
🪛 LanguageTool
[uncategorized] ~5-~5: A comma might be missing here.
Context: ...Templates ## Pruning notifications If desired it is possible to have a custom message...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
9-28: Enhance example clarity with proper punctuation and additional documentation.
- Add a comma after "For example"
- Consider adding comments to explain the configuration structure
Apply these changes:
-For example an application might want to send a notification when an application that was in the draft stage was pruned. Here is an example of how that might be accomplished in the application template: +For example, an application might want to send a notification when an application that was in the draft stage was pruned. Here is an example of how that might be accomplished in the application template: ```typescript stateMachineConfig: { initial: States.DRAFT, states: { [States.DRAFT]: { meta: { name: application.general.name.defaultMessage, + // Configure pruning notification for draft applications + // This will send notifications when applications are pruned after 30 days lifecycle: defaultLifecycleWithPruneMessage((application: PruningApplication) => ({ externalBody: `Application has been in draft for more than 30 days and has been pruned.`, internalBody: `Application for ${application.externalData.nationalRegistry.data.fullName} has been in draft for more than 30 days. Please note that if desired the application may be re-submitted.`, notificationTemplateId: 'HNIPP.AS.*', })), status: 'draft', }, }, }, },<details> <summary>🧰 Tools</summary> <details> <summary>🪛 LanguageTool</summary> [typographical] ~9-~9: After the expression ‘for example’ a comma is usually used. Context: ... to use the body variables or not. For example an application might want to send a not... (COMMA_FOR_EXAMPLE) </details> </details> </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: .coderabbit.yaml** **Review profile: CHILL** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between af739e4c9beadd6c55120a14036655de80ee3536 and b4963bb4dba3b135256d2e502f0c323455e6801a. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `libs/application/templates/README.md` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>📓 Path-based instructions (1)</summary> <details> <summary>libs/application/templates/README.md (1)</summary> 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." </details> </details><details> <summary>🪛 LanguageTool</summary> <details> <summary>libs/application/templates/README.md</summary> [uncategorized] ~5-~5: A comma might be missing here. Context: ...Templates ## Pruning notifications If desired it is possible to have a custom message... (AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA) --- [typographical] ~9-~9: After the expression ‘for example’ a comma is usually used. Context: ... to use the body variables or not. For example an application might want to send a not... (COMMA_FOR_EXAMPLE) </details> </details> </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
...
Attach a link to issue if relevant
What
Add the ability for applications to send notifications to users when their applications are pruned
Why
For instance a user who hasn't completed an application might be confused when an application they hadn't completed disappeared.
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests