BAL-1964 - Transaction monitoring - add dormant rule

[liorzam]

PR Type

enhancement, bug_fix

---

Description

• Added a new 'DORMANT' alert definition in the alert generation script with high severity.
• Integrated comprehensive tests for the 'DORMANT' rule to ensure it triggers correctly under various scenarios.
• Enhanced type safety in HTTP exception filters and updated type definitions in data analytics services.
• Refactored and added new functionalities in DataAnalyticsService to support the 'evaluateDormantAccount' function.

---

Changes walkthrough 📝

	Relevant files
Enhancement	generate-alerts.ts Add new 'DORMANT' alert definition                                              services/workflows-service/scripts/alerts/generate-alerts.ts Added a new alert definition for 'DORMANT' with high severity and a description. Configured the alert to use the 'evaluateDormantAccount' function and target 'counterpartyId'. +10/-0    HttpExceptions.filter.ts Improve type safety in HttpExceptions filter                          services/workflows-service/src/common/filters/HttpExceptions.filter.ts Updated the type of error code status mapping to use HttpStatus type for better type safety. +1/-1      data-analytics.service.ts Support 'evaluateDormantAccount' in DataAnalyticsService  services/workflows-service/src/data-analytics/data-analytics.service.ts Added case for 'evaluateDormantAccount' in switch statement to handle specific project IDs. Refactored the SQL query in 'evaluateDormantAccount' for clarity and efficiency. +41/-51  types.ts Update type definitions for 'evaluateDormantAccount'          services/workflows-service/src/data-analytics/types.ts Added new type definitions related to the 'evaluateDormantAccount' function. Updated various type definitions to use 'TProjectId' for consistency. +8/-4      transaction-factory.ts Enhance transaction date generation in tests                          services/workflows-service/src/transaction/test-utils/transaction-factory.ts Modified transaction date generation to include both past and recent dates for testing. +1/-1
Tests	alert.service.intg.test.ts Integrate tests for 'DORMANT' alert rule                                  services/workflows-service/src/alert/alert.service.intg.test.ts Introduced cleanupDatabase function in beforeEach for better test isolation. Added extensive test cases for the new 'DORMANT' rule to check alert generation under various conditions. +120/-18

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

Summary by CodeRabbit

• New Features

  • Added a new alert definition for dormant accounts to enhance monitoring capabilities.
  • Introduced a new analysis method for evaluating dormant accounts in the Data Analytics service.

• Enhancements

  • Updated transaction date generation to include a broader range of dates, improving data variability for testing.

• Refactor

  • Improved type definitions in various services for better code clarity and maintenance.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 221610a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[coderabbitai]

Walkthrough

This update enriches the system with a new alert for dormant accounts, refines data analytics using CTE-based queries, enhances exception handling, and improves the generation of transaction dates. Type safety is bolstered with enums and custom types across modules.

Changes

Files	Summary
.../generate-alerts.ts	Added a new alert definition for DORMANT.
.../alert.service.intg.test.ts	Updated imports and added async function types.
.../data-analytics.service.ts, .../types.ts	Introduced CTE-based query for evaluating dormant accounts and updated type definitions.
.../transaction-factory.ts	Modified the logic for generating transaction dates.

🐇🌟
In the code's garden, changes bloom,
New alerts like flowers, dispel the gloom.
Data weaves like vines, precise and smart,
With every commit, a work of art.
Hop, skip, a joyful tweak and tune,
Under the moonlight, by the silvery moon. 🌙
🌟🐇

---

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?

Share

• X
• Mastodon
• Reddit
• LinkedIn

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[github-actions]

PR Description updated to latest commit (4521871)

[github-actions]

PR Review 🔍

(Review updated until commit 7c2cfe7)

⏱️ Estimated effort to review [1-5]	4, because the PR involves multiple complex changes across several files, including backend logic, database interactions, and testing. The changes are significant and require a deep understanding of the existing system and the new functionalities being introduced.
🧪 Relevant tests	Yes
⚡ Possible issues	Possible Bug: The evaluateDormantAccount function in data-analytics.service.ts uses a SQL query that might not correctly handle edge cases where transactions are exactly on the boundary of 180 days. This could lead to incorrect alert generation.
	Performance Concern: The SQL query in evaluateDormantAccount might not perform optimally on large datasets due to the lack of indexes on the transactionDate and counterpartyBeneficiaryId fields.
🔒 Security concerns	No

Code feedback:

---

relevant file	services/workflows-service/scripts/alerts/generate-alerts.ts
suggestion	Consider adding a validation or a guard clause to ensure that the evaluateDormantAccount function is only called when it is enabled in the alert definitions. This can prevent unnecessary function invocations and improve performance. [important]
relevant line	fnName: 'evaluateDormantAccount',

---

relevant file	services/workflows-service/src/data-analytics/data-analytics.service.ts
suggestion	Optimize the SQL query in evaluateDormantAccount by using a more efficient date comparison or by indexing relevant columns to improve query performance. [important]
relevant line	tr."counterpartyBeneficiaryId" AS "counterpartyId",

---

relevant file	services/workflows-service/src/alert/alert.service.intg.test.ts
suggestion	Refactor the test cases for the 'DORMANT' rule to use a mock or a stub for the date to control the environment and ensure consistent test results regardless of the actual date when the tests are run. [medium]
relevant line	await castedTransactionFactory.transactionDate(faker.date.recent(30)).count(1).create();

---

relevant file	services/workflows-service/src/common/filters/HttpExceptions.filter.ts
suggestion	Ensure that all possible HTTP status codes are handled in the ErrorCodesStatusMapping type to prevent runtime errors due to unhandled status codes. [medium]
relevant line	key: string]: (typeof HttpStatus)[keyof typeof HttpStatus];

---

[github-actions]

PR Code Suggestions ✨

Category	Suggestions
Enhancement	Improve type safety and maintainability by specifying valid subject types. Consider using a more specific type for the subjects array in the inlineRule definition to ensure that only valid subject types are used. This can help prevent runtime errors and improve code maintainability. services/workflows-service/scripts/alerts/generate-alerts.ts [422] -subjects: ['counterpartyId'], +subjects: Array<'counterpartyId' | 'otherValidSubject'>,
Security	Prevent SQL injection by using parameterized queries. Ensure that the SQL query avoids SQL injection by using parameterized queries instead of template literals for including variables like projectId. services/workflows-service/src/data-analytics/data-analytics.service.ts [338] -tr."projectId" = '${projectId}' +tr."projectId" = $1
Bug	Correct the SQL query syntax by placing JOIN before WHERE. The SQL JOIN operation should be placed before the WHERE clause to follow the correct SQL syntax and ensure the query executes without errors. services/workflows-service/src/data-analytics/data-analytics.service.ts [338-340] +JOIN "transactionsData" "td" ON +tr."counterpartyBeneficiaryId" = td."counterpartyBeneficiaryId" WHERE tr."projectId" = '${projectId}' -JOIN "transactionsData" "td" ON
Best practice	Enhance type safety by restricting ErrorCodesStatusMapping to valid HTTP status codes. Use a more specific type definition for the ErrorCodesStatusMapping to ensure that it only maps to valid HTTP status codes, enhancing type safety and reducing potential runtime errors. services/workflows-service/src/common/filters/HttpExceptions.filter.ts [7] -[key: string]: (typeof HttpStatus)[keyof typeof HttpStatus]; +[key: string]: 200 | 404 | 500 | (typeof HttpStatus)[keyof typeof HttpStatus];
Maintainability	Clarify the paymentMethod structure for better readability and maintainability. The paymentMethod option in the DORMANT alert definition uses an object structure that might be incorrect or unclear. Consider revising the structure or logic to clearly define how paymentMethod should be used in conditions. services/workflows-service/scripts/alerts/generate-alerts.ts [426-429] paymentMethod: { - value: PaymentMethod.credit_card, - operator: '!=', + type: PaymentMethod.credit_card, + condition: '!=', },

[github-actions]

PR Description updated to latest commit (7c2cfe7)

[github-actions]

Persistent review updated to latest commit 7c2cfe7

[github-actions]

PR Code Suggestions ✨

Category	Suggestions
Enhancement	Ensure the subjects array in the inlineRule accurately reflects all relevant subjects involved in the rule evaluation. The subjects array in the inlineRule for the DORMANT alert definition currently contains only 'counterpartyId'. If this is intended to be a placeholder or example, ensure that it accurately reflects the actual subjects involved in the rule evaluation. If more subjects are involved in the rule, they should be added to this array for clarity and completeness. services/workflows-service/scripts/alerts/generate-alerts.ts [423] -subjects: ['counterpartyId'], +subjects: ['counterpartyId', 'otherRelevantField'],
	Remove redundant database truncation if cleanupDatabase already clears the database. The cleanupDatabase function is being called before truncating tables in the beforeEach block. If cleanupDatabase is intended to clear the database, then the subsequent explicit truncation of tables might be redundant. Consider removing the explicit truncation if cleanupDatabase already handles this to make the test setup cleaner and more efficient. services/workflows-service/src/alert/alert.service.intg.test.ts [54-56] await cleanupDatabase(); -await prismaService.$executeRaw`TRUNCATE TABLE "public"."Alert" CASCADE;`;
	Parameterize the transactionDate to accept specific dates or ranges for more consistent and targeted testing. The getTransactionCreateData function uses faker.helpers.arrayElement to randomly select between a past or recent date for transactionDate. This could lead to inconsistent test results. If the intention is to test specific scenarios, consider parameterizing the function to accept a date or date range. services/workflows-service/src/transaction/test-utils/transaction-factory.ts [73] -transactionDate: faker.helpers.arrayElement([faker.date.past(1), faker.date.recent(30)]), +transactionDate: specificDate,
Security	Replace string interpolation with parameterized queries in SQL to prevent SQL injection. The SQL query in evaluateDormantAccount uses string interpolation for projectId, which can lead to SQL injection vulnerabilities. Use parameterized queries instead to enhance security. services/workflows-service/src/data-analytics/data-analytics.service.ts [325] -tr."projectId" = ${projectId} +tr."projectId" = $1
Best practice	Add error handling around the SQL query execution to manage exceptions gracefully. The evaluateDormantAccount function constructs a complex SQL query but does not handle potential exceptions that might occur during its execution. It's recommended to add error handling around the database query to manage exceptions gracefully. services/workflows-service/src/data-analytics/data-analytics.service.ts [339] -return await this._executeQuery<Array<Record<string, unknown>>>(query); +try { + return await this._executeQuery<Array<Record<string, unknown>>>(query); +} catch (error) { + console.error('Failed to execute dormant account query', error); + throw new Error('Database query failed'); +}

[coderabbitai]

Actionable comments posted: 0

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 64ac06c and 7c2cfe7.

Files selected for processing (6)

• services/workflows-service/scripts/alerts/generate-alerts.ts (1 hunks)
• services/workflows-service/src/alert/alert.service.intg.test.ts (6 hunks)
• services/workflows-service/src/common/filters/HttpExceptions.filter.ts (1 hunks)
• services/workflows-service/src/data-analytics/data-analytics.service.ts (2 hunks)
• services/workflows-service/src/data-analytics/types.ts (6 hunks)
• services/workflows-service/src/transaction/test-utils/transaction-factory.ts (1 hunks)

Additional comments not posted (14)

services/workflows-service/src/data-analytics/types.ts (5)

1-1: Import of TProjectId from '@/types' is correctly added to ensure type safety and consistency across the project.

---

40-40: Update of projectId to use TProjectId enhances type safety and consistency, aligning with best practices for TypeScript development.

---

56-56: The change to use TProjectId for projectId in HighTransactionTypePercentage is a good practice for maintaining type consistency and safety.

---

66-66: Using TProjectId for projectId in TCustomersTransactionTypeOptions correctly enforces type consistency across the service.

---

77-77: The update to use TProjectId for projectId in TransactionLimitHistoricAverageOptions is appropriate for ensuring type safety.

services/workflows-service/src/common/filters/HttpExceptions.filter.ts (1)

7-7: Refactoring ErrorCodesStatusMapping to use keys from HttpStatus directly simplifies the mapping and improves code clarity.

services/workflows-service/src/transaction/test-utils/transaction-factory.ts (1)

73-73: Changing transactionDate to randomly select dates from either the past year or the last 30 days introduces more variability and realism in test data generation.

services/workflows-service/src/data-analytics/data-analytics.service.ts (1)

314-336: The new CTE-based SQL query in evaluateDormantAccount enhances clarity and maintainability of the SQL operations. Ensure that the logic correctly identifies dormant accounts based on the specified conditions.

services/workflows-service/scripts/alerts/generate-alerts.ts (1)

416-425: The addition of the DORMANT alert definition with high severity and specific rule configurations is well-implemented to enhance monitoring of dormant accounts.

services/workflows-service/src/alert/alert.service.intg.test.ts (5)

11-11: Updated import statements to include cleanupDatabase.

Ensure that the new cleanupDatabase function is implemented correctly and tested.

---

27-27: Added import for AsyncFunction from 'type-fest/source/async-return-type'.

Verify that AsyncFunction is utilized appropriately in the tests to handle asynchronous operations.

---

54-54: Enhanced database cleanup process in beforeEach block.

Ensure that the additional cleanup commands do not affect the performance of the test suite negatively.

---

87-169: Added new test suite for the 'DORMANT' rule.

Ensure comprehensive coverage for the 'DORMANT' rule, including edge cases not currently tested.

---

1032-1063: Refactored createCounterparty function to handle optional project parameter.

Ensure that the refactoring handles cases where no project is provided and that related unit tests are updated accordingly.

[coderabbitai]

Actionable comments posted: 3

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 7c2cfe7 and a36344e.

Files selected for processing (4)

• services/workflows-service/scripts/alerts/generate-alerts.ts (1 hunks)
• services/workflows-service/src/alert/alert.service.intg.test.ts (6 hunks)
• services/workflows-service/src/data-analytics/data-analytics.service.ts (2 hunks)
• services/workflows-service/src/data-analytics/types.ts (5 hunks)

Files skipped from review as they are similar to previous changes (3)

• services/workflows-service/scripts/alerts/generate-alerts.ts
• services/workflows-service/src/data-analytics/data-analytics.service.ts
• services/workflows-service/src/data-analytics/types.ts

Additional comments not posted (3)

services/workflows-service/src/alert/alert.service.intg.test.ts (3)

58-58: Ensure cleanupDatabase is necessary.

The cleanupDatabase function is used to clean the database before each test. Verify that this function is necessary and correctly implemented to avoid potential issues with test isolation.

---

29-31: Type definition looks good.

The AsyncTransactionFactoryCallback type definition is correct and appropriately used in the file.

---

1256-1287: Helper function implementation looks good.

The createCounterparty helper function is correctly implemented and necessary for the tests.

[coderabbitai]

Actionable comments posted: 0

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between a36344e and b34934c.

Files selected for processing (1)

• services/workflows-service/src/alert/alert.service.intg.test.ts (7 hunks)

Files skipped from review as they are similar to previous changes (1)

• services/workflows-service/src/alert/alert.service.intg.test.ts