ballerine icon indicating copy to clipboard operation
ballerine copied to clipboard

Feature/implement ongoing report alert #1959 #1656

Open Blokh opened this issue 1 year ago • 11 comments

User description

User description

ongoing report alert


Type

enhancement, bug_fix


Description

  • Updated Jest configuration to use ES6 syntax and enhanced module resolution.
  • Refactored alert generation scripts to support transaction and merchant monitoring alerts.
  • Updated seeding script to use new alert seeding function.
  • Extended alert controller to support new endpoint for business report alerts.
  • Enhanced alert service to support different types of monitoring.
  • Added filtering and pagination support to business report service.
  • Implemented risk score evaluation method in data analytics service.
  • Integrated alert checking in the workflow hook callback handler.

Changes walkthrough

Relevant files
Enhancement
jest.config.ts
Update Jest configuration to ES6 and enhance module resolution

services/workflows-service/jest.config.ts

  • Converted module.exports to ES6 export default syntax.
  • Added configurations for modulePaths, moduleDirectories, and
    transform.
  • +18/-5   
    generate-alerts.ts
    Refactor alert generation and introduce merchant monitoring alerts

    services/workflows-service/scripts/alerts/generate-alerts.ts

  • Introduced new alert definitions for transaction monitoring and
    merchant monitoring.
  • Refactored alert generation functions to support different types of
    alert configurations.
  • Added new function to seed transaction alerts.
  • +119/-28
    seed.ts
    Update seeding script to use new alert seeding function   

    services/workflows-service/scripts/seed.ts

  • Replaced generateFakeAlertsAndDefinitions with seedTransactionsAlerts
    to seed transaction alerts.
  • +9/-9     
    alert.controller.external.ts
    Extend alert controller to support business report alerts

    services/workflows-service/src/alert/alert.controller.external.ts

  • Added new endpoint to list business report alerts.
  • Modified existing endpoints to filter alerts based on monitoring type.

  • +113/-34
    alert.service.ts
    Enhance alert service to support different types of monitoring

    services/workflows-service/src/alert/alert.service.ts

  • Added monitoringType parameter to getAlerts and getAllAlertDefinitions
    methods.
  • Implemented new method to check alerts for business based on
    businessId.
  • +17/-8   
    business-report.service.ts
    Add filtering and pagination support to business report service

    services/workflows-service/src/business-report/business-report.service.ts

  • Added methods to handle business reports with filters and pagination.
  • +87/-0   
    data-analytics.service.ts
    Implement risk score evaluation in data analytics service

    services/workflows-service/src/data-analytics/data-analytics.service.ts

  • Added method to check risk score changes and create alerts based on
    business reports.
  • +82/-7   
    hook-callback-handler.service.ts
    Integrate alert checking in workflow hook callback handler

    services/workflows-service/src/workflow/hook-callback-handler.service.ts

    • Integrated alert checking in the workflow hook callback handler.
    +13/-0   

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


    PR Type

    Enhancement, Bug fix


    Description

    • Updated Jest configuration to use ES6 syntax and enhanced module resolution.
    • Refactored alert generation scripts to support transaction and merchant monitoring alerts.
    • Updated seeding script to use new alert seeding function.
    • Extended alert controller to support new endpoint for business report alerts.
    • Enhanced alert service to support different types of monitoring.
    • Integrated alert checking in the workflow hook callback handler.

    Changes walkthrough 📝

    Relevant files
    Configuration changes
    jest.config.ts
    Update Jest Configuration to ES6 and Enhance Module Resolution

    services/workflows-service/jest.config.ts

  • Converted module.exports to ES6 export default syntax.
  • Added configurations for modulePaths, moduleDirectories, and
    transform.
  • +18/-5   
    alert.module.ts
    Include BusinessReportModule in Alert Module                         

    services/workflows-service/src/alert/alert.module.ts

    • Included BusinessReportModule in alert module imports.
    +2/-0     
    Enhancement
    generate-alerts.ts
    Refactor Alert Generation and Add Merchant Monitoring Alerts

    services/workflows-service/scripts/alerts/generate-alerts.ts

  • Introduced new alert definitions for transaction and merchant
    monitoring.
  • Refactored alert generation logic to support different monitoring
    types.
  • Added new functions for generating merchant monitoring alerts.
  • +153/-29
    seed.ts
    Update Seeding Script for Transaction and Merchant Monitoring Alerts

    services/workflows-service/scripts/seed.ts

  • Updated import paths and function names related to alert seeding.
  • Added seeding logic for transaction and merchant monitoring alerts.
  • +10/-10 
    alert.controller.external.ts
    Extend Alert Controller for Business Report Alerts             

    services/workflows-service/src/alert/alert.controller.external.ts

  • Extended the alert controller to support listing business report
    alerts.
  • Added new response types for transaction and merchant alerts.
  • +113/-34
    alert.service.ts
    Implement Ongoing Monitoring Alert Checks in Alert Service

    services/workflows-service/src/alert/alert.service.ts

  • Added new method to check ongoing monitoring alerts.
  • Refactored alert checking methods to support different monitoring
    types.
  • +94/-10 
    hook-callback-handler.service.ts
    Integrate Merchant Monitoring Alert Checks in Workflow Callback
    Handler

    services/workflows-service/src/workflow/hook-callback-handler.service.ts

  • Integrated alert checking for ongoing merchant monitoring in the
    workflow hook callback handler.
  • +32/-2   

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

    Blokh avatar May 05 '24 11:05 Blokh

    ⚠️ No Changeset found

    Latest commit: 6e0bf6956c8d6bc1714cc67f305e65e0fadf3e0c

    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

    changeset-bot[bot] avatar May 05 '24 11:05 changeset-bot[bot]

    [!IMPORTANT]

    Auto Review Skipped

    Auto reviews are disabled on base/target branches other than the default branch. Please add the base/target branch pattern to the list of additional branches to be reviewed in the settings.

    Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

    You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


    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
    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.

    coderabbitai[bot] avatar May 05 '24 11:05 coderabbitai[bot]

    PR Description updated to latest commit (https://github.com/ballerine-io/ballerine/commit/b19d4a7e6f23219b6db3404f55a0a3667605ce29)

    github-actions[bot] avatar May 05 '24 11:05 github-actions[bot]

    PR Description updated to latest commit (https://github.com/ballerine-io/ballerine/commit/b19d4a7e6f23219b6db3404f55a0a3667605ce29)

    github-actions[bot] avatar May 05 '24 11:05 github-actions[bot]

    PR Review

    (Review updated until commit https://github.com/ballerine-io/ballerine/commit/b19d4a7e6f23219b6db3404f55a0a3667605ce29)

    ⏱️ Estimated effort to review [1-5]

    4, because the PR involves multiple complex changes across various files including configuration updates, alert generation logic, and integration with existing services. The changes impact core functionalities such as alert definitions, alert generation, and alert handling, requiring a thorough review to ensure correctness and maintainability.

    🧪 Relevant tests

    No

    🔍 Possible issues

    Possible Bug: The checkRiskScore function in data-analytics.service.ts creates an alert without providing necessary parameters. This could lead to runtime errors or incorrect alert generation.

    Data Integrity: The migration script 20240502085516_add_risk_score_to_business_report adds a non-nullable column riskScore to BusinessReport without a default value. This will fail if the table is not empty.

    🔒 Security concerns

    No

    Code feedback:
    relevant fileservices/workflows-service/src/data-analytics/data-analytics.service.ts
    suggestion      

    Ensure that the createAlert method call within checkRiskScore function is provided with the necessary parameters. This is crucial to avoid runtime errors and ensure that alerts are generated correctly. [important]

    relevant linethis.alertService.createAlert({});

    relevant fileservices/workflows-service/prisma/migrations/20240502085516_add_risk_score_to_business_report/migration.sql
    suggestion      

    Modify the migration script to handle existing data by providing a default value for the riskScore or making the column nullable if applicable. This change will prevent deployment errors when the table contains existing data. [important]

    relevant lineALTER TABLE "BusinessReport" ADD COLUMN "riskScore" INTEGER NOT NULL;


    ✨ Review tool usage guide:

    Overview: The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

    The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

    • When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:
    /review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...
    
    [pr_reviewer]
    some_config1=...
    some_config2=...
    

    See the review usage page for a comprehensive guide on using this tool.

    github-actions[bot] avatar May 05 '24 11:05 github-actions[bot]

    Persistent review updated to latest commit https://github.com/ballerine-io/ballerine/commit/b19d4a7e6f23219b6db3404f55a0a3667605ce29

    github-actions[bot] avatar May 05 '24 11:05 github-actions[bot]

    PR Code Suggestions

    CategorySuggestions                                                                                                                                                       
    Enhancement
    Improve the regular expression escaping in testRegex to ensure it matches the intended file patterns accurately.

    The regular expression for testRegex should be properly escaped to avoid unintended
    matches or errors. Specifically, the period (.) should be escaped to match a literal
    period rather than any character.

    services/workflows-service/jest.config.ts [4]

    -testRegex: '(/__tests__/.*|(\\.|/)(unit|e2e|intg)\\.test)\\.ts$',
    +testRegex: '(/__tests__/.*|(\\.|\\/)(unit|e2e|intg)\\.test)\\.ts$',
     
    
    Improve code readability and performance by modifying how arguments are passed.

    Replace the spread operator with direct property assignment in the checkAlert function to
    improve readability and performance.

    services/workflows-service/src/alert/alert.service.ts [147-149]

    -return this.checkAlert(alertDefinition, businessId);
    +return this.checkAlert(alertDefinition, { businessId });
     
    
    Increase flexibility of getAlertDefinitions by allowing more generic options.

    Refactor the getAlertDefinitions function to accept a more generic options parameter to
    increase the function's flexibility.

    services/workflows-service/src/alert/alert.service.ts [119]

    -async getAlertDefinitions(options: { type: MonitoringType }): Promise<AlertDefinition[]> {
    +async getAlertDefinitions(options: { type?: MonitoringType, enabled?: boolean } = {}): Promise<AlertDefinition[]> {
     
    
    Add error handling for parseInt to ensure valid number inputs for pagination.

    Consider adding error handling for the parseInt function to ensure that the input is a
    valid number. If the input is not a valid number, the function returns NaN, which could
    lead to unexpected behavior in the pagination logic.

    services/workflows-service/src/business-report/business-report.service.ts [60-61]

     const size = parseInt(getTransactionsParameters.page.size as unknown as string, 10);
     const number = parseInt(getTransactionsParameters.page.number as unknown as string, 10);
    +if (isNaN(size) || isNaN(number)) {
    +  throw new Error('Invalid pagination parameters');
    +}
     
    
    Simplify property access using destructuring in pagination logic.

    Use destructuring to simplify the access to properties within
    getTransactionsParameters.page to enhance code readability and reduce redundancy.

    services/workflows-service/src/business-report/business-report.service.ts [60-61]

    -const size = parseInt(getTransactionsParameters.page.size as unknown as string, 10);
    -const number = parseInt(getTransactionsParameters.page.number as unknown as string, 10);
    +const { size: pageSize, number: pageNumber } = getTransactionsParameters.page;
    +const size = parseInt(pageSize as unknown as string, 10);
    +const number = parseInt(pageNumber as unknown as string, 10);
     
    
    Maintainability
    Use a more descriptive variable name for inlineRule.id.

    To improve the maintainability and readability of the code, consider using a more
    descriptive variable name than id for the inlineRule.id in the
    getAlertDefinitionCreateData function.

    services/workflows-service/scripts/alerts/generate-alerts.ts [476]

    -const id = inlineRule.id;
    +const ruleId = inlineRule.id;
     
    
    Simplify the conditional logic in generateFakeAlert for handling different ID types.

    Refactor the generateFakeAlert function to simplify the conditional logic by using a
    single object to handle both businessIds and counterpartyIds, reducing redundancy and
    improving code clarity.

    services/workflows-service/scripts/alerts/generate-alerts.ts [565-573]

    -if ('businessIds' in options) {
    -  businessId = faker.helpers.arrayElement(options.businessIds.map(id => ({ businessId: id })));
    -} else if ('counterpartyIds' in options) {
    -  counterpartyId = faker.helpers.arrayElement(
    -    options.counterpartyIds.map(id => ({ counterpartyId: id })),
    -  );
    -}
    +const entityIdType = 'businessIds' in options ? 'businessId' : 'counterpartyId';
    +const entityIds = options[entityIdType];
    +const entity = faker.helpers.arrayElement(
    +  entityIds.map(id => ({ [entityIdType]: id }))
    +);
     
    
    Add explicit return type to the checkAlert function for better maintainability.

    Ensure that the checkAlert function's return type is explicitly defined to improve code
    maintainability and clarity.

    services/workflows-service/src/alert/alert.service.ts [151]

    -private async checkAlert(alertDefinition: AlertDefinition, ...args: any[]) {
    +private async checkAlert(alertDefinition: AlertDefinition, ...args: any[]): Promise<ReturnType> {
     
    
    Refactor the switch statement to use a map for time unit conversions.

    Refactor the switch statement to use a map or object for cleaner and more maintainable
    code. This approach avoids the repetitive structure and makes it easier to manage the time
    unit conversions.

    services/workflows-service/src/business-report/business-report.service.ts [108-123]

    -switch (filterParams.timeUnit) {
    -  case TIME_UNITS.minutes:
    -    subtractValue = filterParams.timeValue * 60 * 1000;
    -    break;
    -  case TIME_UNITS.hours:
    -    subtractValue = filterParams.timeValue * 60 * 60 * 1000;
    -    break;
    -  case TIME_UNITS.days:
    -    subtractValue = filterParams.timeValue * 24 * 60 * 60 * 1000;
    -    break;
    -  case TIME_UNITS.months:
    -    now.setMonth(now.getMonth() - filterParams.timeValue);
    -    break;
    -  case TIME_UNITS.years:
    -    now.setFullYear(now.getFullYear() - filterParams.timeValue);
    -    break;
    +const timeUnitMultipliers = {
    +  [TIME_UNITS.minutes]: 60 * 1000,
    +  [TIME_UNITS.hours]: 60 * 60 * 1000,
    +  [TIME_UNITS.days]: 24 * 60 * 60 * 1000,
    +  [TIME_UNITS.months]: (value) => now.setMonth(now.getMonth() - value),
    +  [TIME_UNITS.years]: (value) => now.setFullYear(now.getFullYear() - value),
    +};
    +const multiplier = timeUnitMultipliers[filterParams.timeUnit];
    +if (typeof multiplier === 'function') {
    +  multiplier(filterParams.timeValue);
    +} else {
    +  subtractValue = filterParams.timeValue * multiplier;
     }
     
    
    Extract logic for updating date filters into a separate function for better readability.

    To improve code readability and maintainability, consider extracting the logic for
    updating the whereClause.createdAt into a separate function. This will make the
    buildFilters function cleaner and easier to understand.

    services/workflows-service/src/business-report/business-report.service.ts [90-101]

    -if (filterParams.startDate) {
    -  whereClause.createdAt = {
    -    ...(whereClause.createdAt as DateTimeFilter),
    -    gte: filterParams.startDate,
    +function updateCreatedAtFilter(filter, value, operator) {
    +  return {
    +    ...(filter.createdAt as DateTimeFilter),
    +    [operator]: value,
       };
     }
    +if (filterParams.startDate) {
    +  whereClause.createdAt = updateCreatedAtFilter(whereClause, filterParams.startDate, 'gte');
    +}
     if (filterParams.endDate) {
    -  whereClause.createdAt = {
    -    ...(whereClause.createdAt as DateTimeFilter),
    -    lte: filterParams.endDate,
    -  };
    +  whereClause.createdAt = updateCreatedAtFilter(whereClause, filterParams.endDate, 'lte');
     }
     
    
    Add a comment explaining the default value choice for the new column.

    Consider adding a comment to explain the choice of the default value
    'transaction_monitoring' for the new column 'monitoringType'. This helps maintainers
    understand why this default was chosen, especially if other values are equally valid.

    services/workflows-service/prisma/migrations/20240415161621_add_monitoring_type/migration.sql [5]

    +-- Default to 'transaction_monitoring' because it is the most common monitoring type
     ALTER TABLE "AlertDefinition" ADD COLUMN "monitoringType" "MonitoringType" NOT NULL DEFAULT 'transaction_monitoring';
     
    
    Ensure the ENUM 'MonitoringType' is comprehensive to minimize future schema changes.

    Ensure that the ENUM 'MonitoringType' includes all necessary monitoring types at this
    stage to avoid frequent modifications which can be disruptive. Consider future use cases
    or consult with stakeholders.

    services/workflows-service/prisma/migrations/20240415161621_add_monitoring_type/migration.sql [2]

    -CREATE TYPE "MonitoringType" AS ENUM ('transaction_monitoring', 'ongoing_merchant_monitoring');
    +CREATE TYPE "MonitoringType" AS ENUM ('transaction_monitoring', 'ongoing_merchant_monitoring', 'new_type_here');
     
    
    Best practice
    Add error handling for the getAlerts method calls to manage exceptions and ensure robustness.

    To ensure type safety and proper handling, add error handling for the getAlerts method
    calls within the list and listBusniessReportAlerts methods to manage potential exceptions
    or empty results.

    services/workflows-service/src/alert/alert.controller.external.ts [59]

    -const alerts = await this.alertService.getAlerts(
    +let alerts;
    +try {
    +  alerts = await this.alertService.getAlerts(
    +} catch (error) {
    +  // handle error appropriately
    +}
     
    
    Enhance type safety by specifying a more detailed type for the args parameter.

    Use a more specific type than any[] for the args parameter in the checkAlert function to
    enhance type safety.

    services/workflows-service/src/alert/alert.service.ts [151]

    -private async checkAlert(alertDefinition: AlertDefinition, ...args: any[]) {
    +private async checkAlert(alertDefinition: AlertDefinition, ...args: SpecificType[]) {
     
    
    Add error handling in the checkAlert function to manage exceptions.

    Consider handling exceptions for the checkAlert function to ensure that errors are managed
    properly.

    services/workflows-service/src/alert/alert.service.ts [151]

    -private async checkAlert(alertDefinition: AlertDefinition, ...args: any[]) {
    +private async checkAlert(alertDefinition: AlertDefinition, ...args: any[]): Promise<void> {
    +  try {
    +    // Existing implementation
    +  } catch (error) {
    +    // Error handling logic
    +  }
    +}
     
    
    Avoid modifying the original date object to ensure function purity.

    Avoid directly modifying the now Date object inside the switch statement. Instead,
    calculate the new date based on the original date to prevent side effects and make the
    function pure.

    services/workflows-service/src/business-report/business-report.service.ts [105-123]

     const now = new Date(); // UTC time by default
    +let newDate = new Date(now);
     switch (filterParams.timeUnit) {
       case TIME_UNITS.months:
    -    now.setMonth(now.getMonth() - filterParams.timeValue);
    +    newDate.setMonth(now.getMonth() - filterParams.timeValue);
         break;
       case TIME_UNITS.years:
    -    now.setFullYear(now.getFullYear() - filterParams.timeValue);
    +    newDate.setFullYear(now.getFullYear() - filterParams.timeValue);
         break;
     }
     
    

    ✨ Improve tool usage guide:

    Overview: The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

    • When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:
    /improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...
    
    [pr_code_suggestions]
    some_config1=...
    some_config2=...
    

    See the improve usage page for a comprehensive guide on using this tool.

    github-actions[bot] avatar May 05 '24 11:05 github-actions[bot]

    PR Code Suggestions

    CategorySuggestions                                                                                                                                                       
    Enhancement
    Simplify and enhance the type safety of conditional logic in the function.

    Refactor the generateFakeAlert function to use a more concise and type-safe approach for
    handling the conditional logic of businessIds and counterpartyIds.

    services/workflows-service/scripts/alerts/generate-alerts.ts [565-573]

    -if ('businessIds' in options) {
    -  businessId = faker.helpers.arrayElement(options.businessIds.map(id => ({ businessId: id })));
    -} else if ('counterpartyIds' in options) {
    -  counterpartyId = faker.helpers.arrayElement(
    -    options.counterpartyIds.map(id => ({ counterpartyId: id })),
    -  );
    +const key = 'businessIds' in options ? 'businessIds' : 'counterpartyIds';
    +const idKey = key === 'businessIds' ? 'businessId' : 'counterpartyId';
    +const result = faker.helpers.arrayElement(options[key].map(id => ({ [idKey]: id })));
    +if (key === 'businessIds') {
    +  businessId = result;
    +} else {
    +  counterpartyId = result;
     }
     
    
    Implement error handling in the API method to improve reliability and error reporting.

    Add error handling for the list method to manage exceptions that might occur during the
    database operations, improving the robustness of the API.

    services/workflows-service/src/alert/alert.controller.external.ts [59-101]

    -const alerts = await this.alertService.getAlerts(
    -  findAlertsDto,
    -  MonitoringType.transaction_monitoring,
    -  projectIds,
    -  {
    -    include: {
    -      alertDefinition: {
    -        select: {
    -          label: true,
    -          description: true,
    +try {
    +  const alerts = await this.alertService.getAlerts(
    +    findAlertsDto,
    +    MonitoringType.transaction_monitoring,
    +    projectIds,
    +    {
    +      include: {
    +        alertDefinition: {
    +          select: {
    +            label: true,
    +            description: true,
    +          },
             },
    -      },
    -      assignee: {
    -        select: {
    -          id: true,
    -          firstName: true,
    -          lastName: true,
    -          avatarUrl: true,
    +        assignee: {
    +          select: {
    +            id: true,
    +            firstName: true,
    +            lastName: true,
    +            avatarUrl: true,
    +          },
             },
    -      },
    -      counterparty: {
    -        select: {
    -          id: true,
    -          business: {
    -            select: {
    -              id: true,
    -              correlationId: true,
    -              companyName: true,
    +        counterparty: {
    +          select: {
    +            id: true,
    +            business: {
    +              select: {
    +                id: true,
    +                correlationId: true,
    +                companyName: true,
    +              },
                 },
    -          },
    -          endUser: {
    -            select: {
    -              id: true,
    -              correlationId: true,
    -              firstName: true,
    -              lastName: true,
    +            endUser: {
    +              select: {
    +                id: true,
    +                correlationId: true,
    +                firstName: true,
    +                lastName: true,
    +              },
                 },
               },
             },
           },
    -    },
    -);
    +  );
    +} catch (error) {
    +  throw new common.HttpException('Failed to retrieve alerts', common.HttpStatus.INTERNAL_SERVER_ERROR);
    +}
     
    
    Improve type safety by verifying string types before parsing integers.

    Consider using a more robust type assertion or type guard when casting
    getTransactionsParameters.page.size and getTransactionsParameters.page.number to strings.
    This ensures that the values are indeed strings before attempting to parse them, which can
    prevent runtime errors if unexpected types are passed.

    services/workflows-service/src/business-report/business-report.service.ts [60-61]

    -const size = parseInt(getTransactionsParameters.page.size as unknown as string, 10);
    -const number = parseInt(getTransactionsParameters.page.number as unknown as string, 10);
    +const size = typeof getTransactionsParameters.page.size === 'string' ? parseInt(getTransactionsParameters.page.size, 10) : 0;
    +const number = typeof getTransactionsParameters.page.number === 'string' ? parseInt(getTransactionsParameters.page.number, 10) : 0;
     
    
    Simplify property access using destructuring in the buildFilters method.

    Use destructuring to simplify the access to properties from getTransactionsParameters in
    the buildFilters method, which improves readability and reduces redundancy.

    services/workflows-service/src/business-report/business-report.service.ts [86-102]

    -if (filterParams.businessId) {
    -  whereClause.businessId = filterParams.businessId;
    +const { businessId, startDate, endDate } = filterParams;
    +if (businessId) {
    +  whereClause.businessId = businessId;
     }
    -if (filterParams.startDate) {
    +if (startDate) {
       whereClause.createdAt = {
         ...(whereClause.createdAt as DateTimeFilter),
    -    gte: filterParams.startDate,
    +    gte: startDate,
       };
     }
    -if (filterParams.endDate) {
    +if (endDate) {
       whereClause.createdAt = {
         ...(whereClause.createdAt as DateTimeFilter),
    -    lte: filterParams.endDate,
    +    lte: endDate,
       };
     }
     
    
    Consider future-proofing the monitoring types to avoid frequent database migrations.

    Ensure that the ENUM 'MonitoringType' includes all necessary monitoring types or consider
    future-proofing by allowing additional types without requiring a migration. This could
    involve using a different approach, such as a reference table or configuration file, which
    can be modified without a full migration.

    services/workflows-service/prisma/migrations/20240415161621_add_monitoring_type/migration.sql [2]

    -CREATE TYPE "MonitoringType" AS ENUM ('transaction_monitoring', 'ongoing_merchant_monitoring');
    +-- Consider future monitoring types and the need to avoid frequent migrations
    +-- CREATE TYPE "MonitoringType" AS ENUM ('transaction_monitoring', 'ongoing_merchant_monitoring');
    +-- Alternatively, use a reference table for monitoring types
     
    
    Best practice
    Add input validation to ensure all necessary parameters are provided and valid.

    The function seedTransactionsAlerts should validate the input parameters to ensure they
    are not undefined or empty, as they are crucial for the operation of the function.

    services/workflows-service/scripts/alerts/generate-alerts.ts [999-1006]

    +if (!project1 || !businessRiskIds.length || !agentUserIds.length) {
    +  throw new Error('Required parameters for seeding transaction alerts are missing or invalid.');
    +}
     await seedTransactionsAlerts(client, {
       project: project1,
       businessIds: businessRiskIds,
       counterpartyIds: [...ids1, ...ids2]
         .map(({ counterpartyOriginatorId }) => counterpartyOriginatorId)
         .filter(Boolean) as string[],
       agentUserIds: agentUsers.map(({ id }) => id),
     });
     
    
    Store repeated calculations in variables to enhance code readability and reduce errors.

    Use a variable to store the result of the amountBetween.min + 1 calculation to avoid
    repeating the same calculation in multiple places.

    services/workflows-service/src/alert/alert.service.intg.test.ts [106]

    -.amount(TRANSACTIONS_ALERT_DEFINITIONS.STRUC_CC.inlineRule.options.amountBetween.min + 1)
    +const minAmountPlusOne = TRANSACTIONS_ALERT_DEFINITIONS.STRUC_CC.inlineRule.options.amountBetween.min + 1;
    +.amount(minAmountPlusOne)
     
    
    Improve type safety by specifying a more detailed type than any.

    Consider using a more specific type than any[] for the args parameter in the checkAlert
    method to improve type safety.

    services/workflows-service/src/alert/alert.service.ts [151]

    -private async checkAlert(alertDefinition: AlertDefinition, ...args: any[]) {
    +private async checkAlert(alertDefinition: AlertDefinition, ...args: SpecificType[]) {
     
    
    Ensure consistent use of async/await for database operations.

    To avoid potential issues with asynchronous execution and ensure that the database
    operations are properly awaited, refactor the method to use async/await consistently
    instead of mixing it with then-catch or other promise handling approaches.

    services/workflows-service/src/business-report/business-report.service.ts [71-80]

    -return await this.businessReportRepository.findMany(
    -  {
    -    ...options,
    -    where: {
    -      ...this.buildFilters(getTransactionsParameters),
    -    },
    -    ...args,
    +const queryOptions = {
    +  ...options,
    +  where: {
    +    ...this.buildFilters(getTransactionsParameters),
       },
    -  [projectId],
    -);
    +  ...args,
    +};
    +try {
    +  const result = await this.businessReportRepository.findMany(queryOptions, [projectId]);
    +  return result;
    +} catch (error) {
    +  // appropriate error handling
    +  throw error;
    +}
     
    
    Maintainability
    Replace hardcoded values with constants for better maintainability.

    Replace the hardcoded threshold values in the test assertions with variables or constants
    to avoid duplication and improve maintainability.

    services/workflows-service/src/alert/alert.service.intg.test.ts [93-95]

    +const MIN_AMOUNT_THRESHOLD = 5;
     expect(
       TRANSACTIONS_ALERT_DEFINITIONS.STRUC_CC.inlineRule.options.amountThreshold,
    -).toBeGreaterThanOrEqual(5);
    +).toBeGreaterThanOrEqual(MIN_AMOUNT_THRESHOLD);
     
    
    Replace switch statement with object lookup for cleaner code.

    Refactor the switch statement for filterParams.timeUnit to use a map or object lookup
    instead of multiple case statements. This will make the code cleaner and easier to
    maintain.

    services/workflows-service/src/business-report/business-report.service.ts [108-123]

    -switch (filterParams.timeUnit) {
    -  case TIME_UNITS.minutes:
    -    subtractValue = filterParams.timeValue * 60 * 1000;
    -    break;
    -  case TIME_UNITS.hours:
    -    subtractValue = filterParams.timeValue * 60 * 60 * 1000;
    -    break;
    -  case TIME_UNITS.days:
    -    subtractValue = filterParams.timeValue * 24 * 60 * 60 * 1000;
    -    break;
    -  case TIME_UNITS.months:
    -    now.setMonth(now.getMonth() - filterParams.timeValue);
    -    break;
    -  case TIME_UNITS.years:
    -    now.setFullYear(now.getFullYear() - filterParams.timeValue);
    -    break;
    +const timeUnitMultipliers = {
    +  [TIME_UNITS.minutes]: 60 * 1000,
    +  [TIME_UNITS.hours]: 60 * 60 * 1000,
    +  [TIME_UNITS.days]: 24 * 60 * 60 * 1000,
    +  [TIME_UNITS.months]: (value) => now.setMonth(now.getMonth() - value),
    +  [TIME_UNITS.years]: (value) => now.setFullYear(now.getFullYear() - value),
    +};
    +const action = timeUnitMultipliers[filterParams.timeUnit];
    +if (typeof action === 'function') {
    +  action(filterParams.timeValue);
    +} else {
    +  subtractValue = filterParams.timeValue * action;
     }
     
    
    Refactor risk score calculation into a separate method for better maintainability.

    Refactor the risk score calculation logic to a separate method to improve code readability
    and maintainability. This also makes it easier to unit test the risk score calculation
    independently.

    services/workflows-service/src/data-analytics/data-analytics.service.ts [91-123]

    -if (currentRiskScore < previousRiskScore) {
    -  return false;
    -}
    -if (increaseRiskScore > currentRiskScore - previousRiskScore) {
    -  return false;
    -}
    -let severity: AlertSeverity = AlertSeverity.low;
    -const riskScorePercentage = (currentRiskScore - previousRiskScore) / previousRiskScore;
    -if (15 <= riskScorePercentage || currentRiskScore < 40) {
    -  severity = AlertSeverity.low;
    -}
    -if (
    -  (15 > riskScorePercentage && riskScorePercentage >= 30) ||
    -  (currentRiskScore >= 40 && currentRiskScore < 55)
    -) {
    -  severity = AlertSeverity.medium;
    -}
    -if (
    -  (30 > riskScorePercentage && riskScorePercentage >= 45) ||
    -  (currentRiskScore >= 55 && currentRiskScore < 75)
    -) {
    -  severity = AlertSeverity.high;
    -}
    -if ((45 > riskScorePercentage && riskScorePercentage >= 60) || currentRiskScore > 75) {
    -  severity = AlertSeverity.critical;
    +const severity = this.calculateRiskSeverity(currentRiskScore, previousRiskScore, increaseRiskScore);
    +if (!severity) return false;
    +
    +// New method
    +calculateRiskSeverity(currentRiskScore, previousRiskScore, increaseRiskScore) {
    +  if (currentRiskScore < previousRiskScore || increaseRiskScore > currentRiskScore - previousRiskScore) {
    +    return false;
    +  }
    +  const riskScorePercentage = (currentRiskScore - previousRiskScore) / previousRiskScore;
    +  if (riskScorePercentage >= 60 || currentRiskScore > 75) return AlertSeverity.critical;
    +  if (riskScorePercentage >= 45) return AlertSeverity.high;
    +  if (riskScorePercentage >= 30) return AlertSeverity.medium;
    +  return AlertSeverity.low;
     }
     
    
    Add a comment explaining the default value for the new column to aid in future maintenance.

    Consider adding a comment to explain the choice of the default value
    'transaction_monitoring' for the new column 'monitoringType'. This will help future
    maintainers understand why this specific default was chosen, especially in the context of
    existing data.

    services/workflows-service/prisma/migrations/20240415161621_add_monitoring_type/migration.sql [5]

    +-- Default to 'transaction_monitoring' to maintain compatibility with existing alerts
     ALTER TABLE "AlertDefinition" ADD COLUMN "monitoringType" "MonitoringType" NOT NULL DEFAULT 'transaction_monitoring';
     
    
    Bug
    Refactor method call to pass parameters correctly.

    Refactor the checkAlertForBusiness method to directly pass the businessId to the
    checkAlert method without wrapping it in an array.

    services/workflows-service/src/alert/alert.service.ts [148-149]

    -return this.checkAlert(alertDefinition, businessId);
    +return this.checkAlert(alertDefinition, [businessId]);
     
    
    Possible issue
    Correct the use of TypeScript's type assertions to ensure proper type checking.

    Ensure that the satisfies clause in the object creation is correctly validating against
    InputJsonValue to prevent runtime errors.

    services/workflows-service/src/alert/alert.service.ts [255]

    -} satisfies TExecutionDetails as InputJsonValue,
    +} as InputJsonValue,
     
    
    Performance
    Improve query performance by adding an index on the new 'monitoringType' column.

    Add an index on the new column 'monitoringType' in the 'AlertDefinition' table to improve
    query performance, especially if this column will be frequently used in WHERE clauses or
    joins.

    services/workflows-service/prisma/migrations/20240415161621_add_monitoring_type/migration.sql [5]

     ALTER TABLE "AlertDefinition" ADD COLUMN "monitoringType" "MonitoringType" NOT NULL DEFAULT 'transaction_monitoring';
    +CREATE INDEX idx_monitoring_type ON "AlertDefinition"("monitoringType");
     
    

    ✨ Improve tool usage guide:

    Overview: The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

    • When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:
    /improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...
    
    [pr_code_suggestions]
    some_config1=...
    some_config2=...
    

    See the improve usage page for a comprehensive guide on using this tool.

    github-actions[bot] avatar May 05 '24 11:05 github-actions[bot]

    PR Description updated to latest commit (https://github.com/ballerine-io/ballerine/commit/0dc86f64c059e544902ec5cfa12ed413c6ae6fa1)

    github-actions[bot] avatar May 07 '24 13:05 github-actions[bot]

    PR Review 🔍

    ⏱️ Estimated effort to review [1-5]

    4, because the PR involves multiple files and significant changes across various services including alert generation, configuration updates, and integration with existing services. The complexity is increased by the introduction of new types and the integration of these types across different modules.

    🧪 Relevant tests

    No

    ⚡ Possible issues

    Possible Bug: The checkMerchantOngoingAlert method in DataAnalyticsService might not handle cases where the previousReport is undefined or does not contain the expected structure. This could lead to runtime errors if the data is not as expected.

    Data Integrity: The checkOngoingMonitoringAlert method in AlertService does not handle cases where no alert definitions are found or all definitions are filtered out. This could lead to unhandled promises or lack of alert generation when expected.

    🔒 Security concerns

    No

    Code feedback:
    relevant fileservices/workflows-service/src/alert/alert.service.ts
    suggestion      

    Consider adding error handling for cases where previousReport data might be missing or malformed in the checkMerchantOngoingAlert method. This will prevent potential runtime errors and ensure the application's robustness. [important]

    relevant lineif (previousReportType !== BusinessReportType.ONGOING_MERCHANT_REPORT_T1) {

    relevant fileservices/workflows-service/src/alert/alert.service.ts
    suggestion      

    Implement error handling or a default case in the checkOngoingMonitoringAlert method for scenarios where no alert definitions are applicable or returned. This ensures that the function's promise is always resolved or rejected appropriately, preventing potential issues in the promise chain. [important]

    relevant lineconst alertCreateArgs = (await Promise.all(alertDefinitionsCheck))

    relevant fileservices/workflows-service/src/data-analytics/data-analytics.service.ts
    suggestion      

    Add validation to ensure that the previousReport exists and has the necessary structure before accessing its properties in the checkMerchantOngoingAlert method. This will enhance the reliability of the data handling process. [important]

    relevant lineif (!(maxRiskScoreThreshold || increaseRiskScore || increaseRiskScorePercentage)) {

    relevant fileservices/workflows-service/src/workflow/hook-callback-handler.service.ts
    suggestion      

    Ensure that the checkOngoingMonitoringAlert method's promise is properly handled. Consider using await with try-catch for better error management or ensure that errors are logged and do not cause unhandled rejections. [important]

    relevant linethis.alertService

    github-actions[bot] avatar May 07 '24 13:05 github-actions[bot]

    PR Code Suggestions ✨

    CategorySuggestions                                                                                                                                                       
    Maintainability
    Improve readability and maintainability of regex patterns.

    Replace the regex string in testRegex with a more readable and maintainable format using
    RegExp constructor.

    services/workflows-service/jest.config.ts [4]

    -testRegex: '(/__tests__/.*|(\\.|/)(unit|e2e|intg)\\.test)\\.ts$',
    +testRegex: new RegExp('(/__tests__/.*|(\\.|/)(unit|e2e|intg)\\.test)\\.ts$'),
     
    
    Simplify the function signature by using a single object type for options.

    Refactor the generateFakeAlert function to use a single object type for options instead of
    a union type, simplifying the function signature and usage.

    services/workflows-service/scripts/alerts/generate-alerts.ts [581-591]

     options: {
       severity: AlertSeverity;
       agentUserIds: string[];
    -} & (
    -  | {
    -      counterpartyIds: string[];
    -    }
    -  | {
    -      businessIds: string[];
    -    }
    -),
    +  entityIds: string[];
    +  entityType: 'business' | 'counterparty';
    +},
     
    
    Refactor the checkOngoingMonitoringAlert method to improve readability and maintainability.

    Refactor the checkOngoingMonitoringAlert method to separate concerns for better
    readability and maintainability. Specifically, the logic for processing alert definitions
    and creating alerts should be moved to separate methods.

    services/workflows-service/src/alert/alert.service.ts [154-201]

     async checkOngoingMonitoringAlert(
       checkRiskScoreSubject: CheckRiskScoreSubject,
       businessCompanyName: string,
     ) {
    -  const alertDefinitions = await this.alertDefinitionRepository.findMany(
    -    {
    -      where: {
    -        enabled: true,
    -        monitoringType: MonitoringType.ongoing_merchant_monitoring,
    -        projectId: checkRiskScoreSubject.projectId,
    -      },
    -    },
    -    [checkRiskScoreSubject.projectId],
    -  );
    +  const alertDefinitions = await this.fetchEnabledAlertDefinitions(checkRiskScoreSubject);
    +  const alertResults = await this.processAlertDefinitions(alertDefinitions, checkRiskScoreSubject, businessCompanyName);
    +  return this.createAlertsFromResults(alertResults);
    +}
     
    +private async fetchEnabledAlertDefinitions(checkRiskScoreSubject: CheckRiskScoreSubject): Promise<AlertDefinition[]> {
    +  // Fetch logic here
    +}
    +
    +private async processAlertDefinitions(alertDefinitions: AlertDefinition[], checkRiskScoreSubject: CheckRiskScoreSubject, businessCompanyName: string): Promise<any[]> {
    +  // Process logic here
    +}
    +
    +private async createAlertsFromResults(alertResults: any[]): Promise<Alert[]> {
    +  // Create logic here
    +}
    +
    
    Improve code modularity by extracting pagination logic into a separate function.

    Refactor the method to handle pagination logic in a separate function to improve
    modularity and readability.

    services/workflows-service/src/business-report/business-report.service.ts [56-62]

    -if (getTransactionsParameters.page?.number && getTransactionsParameters.page?.size) {
    -  const size = parseInt(getTransactionsParameters.page.size as unknown as string, 10);
    -  const number = parseInt(getTransactionsParameters.page.number as unknown as string, 10);
    -  args.take = size;
    -  args.skip = size * (number - 1);
    +this.applyPagination(args, getTransactionsParameters);
    +// Elsewhere in the class
    +private applyPagination(args: Prisma.BusinessReportFindManyArgs, params: GetBusinessReportDto): void {
    +  if (params.page?.number && params.page?.size) {
    +    const size = parseInt(params.page.size as unknown as string, 10);
    +    const number = parseInt(params.page.number as unknown as string, 10);
    +    args.take = size;
    +    args.skip = size * (number - 1);
    +  }
     }
     
    
    Reduce redundancy and improve maintainability by refactoring repeated type assertions.

    Refactor the repeated type assertion of reportRiskScore and other similar variables to a
    single validated assignment to enhance code maintainability and reduce redundancy.

    services/workflows-service/src/workflow/hook-callback-handler.service.ts [161]

    -riskScore: reportRiskScore as number,
    +const validatedReportRiskScore: number = typeof reportRiskScore === 'number' ? reportRiskScore : 0;
    +riskScore: validatedReportRiskScore,
     
    
    Error handling
    Add error handling for missing entity IDs in the generateFakeAlert function.

    Ensure that the generateFakeAlert function handles the case where neither businessIds nor
    counterpartyIds are provided, to avoid runtime errors.

    services/workflows-service/scripts/alerts/generate-alerts.ts [598-606]

    -if ('businessIds' in options) {
    +if ('businessIds' in options && options.businessIds.length > 0) {
       businessId = faker.helpers.arrayElement(options.businessIds.map(id => ({ businessId: id })));
    -} else if ('counterpartyIds' in options) {
    +} else if ('counterpartyIds' in options && options.counterpartyIds.length > 0) {
       counterpartyId = faker.helpers.arrayElement(
         options.counterpartyIds.map(id => ({ counterpartyId: id })),
       );
    +} else {
    +  throw new Error('No valid entity IDs provided.');
     }
     
    
    Readability
    Improve variable naming for clarity.

    Use a more descriptive variable name than id for the inlineRule.id in
    getAlertDefinitionCreateData to enhance code readability.

    services/workflows-service/scripts/alerts/generate-alerts.ts [509]

    -const id = inlineRule.id;
    +const ruleId = inlineRule.id;
     
    
    Best practice
    Replace magic numbers with named constants to improve code readability and maintainability.

    Avoid using magic numbers directly in the code. Define a constant for the number range
    used in faker.datatype.number({ min: 1, max: 5 }).

    services/workflows-service/scripts/alerts/generate-alerts.ts [609-610]

    -const assigneeId = faker.datatype.number({ min: 1, max: 5 }) === 1
    +const ASSIGN_CHANCE_RANGE = { min: 1, max: 5 };
    +const assigneeId = faker.datatype.number(ASSIGN_CHANCE_RANGE) === 1;
     
    
    Define a type for the parameters of the createAlert function.

    To ensure that the createAlert function is type-safe and adheres to TypeScript's best
    practices, consider defining a type for its parameters. This will help in maintaining the
    integrity of the data being passed to the function.

    services/workflows-service/src/alert/alert.service.ts [292-293]

    -async createAlert(
    -  alertDef: AlertDefinition,
    -  subject: Array<{ [key: string]: unknown }>,
    -  executionRow: Record<string, unknown>,
    -  additionalInfo?: Record<string, unknown>,
    +type CreateAlertParams = {
    +  alertDef: AlertDefinition;
    +  subject: Array<{ [key: string]: unknown }>;
    +  executionRow: Record<string, unknown>;
    +  additionalInfo?: Record<string, unknown>;
    +};
     
    +async createAlert({
    +  alertDef,
    +  subject,
    +  executionRow,
    +  additionalInfo,
    +}: CreateAlertParams) {
    +  // Function implementation
    +}
    +
    
    Replace the TODO comment with a specific action or ticket reference in the listBusinessReportAlerts method.

    It's recommended to avoid using the TODO comment without a specific action or ticket
    reference. Ensure that the ApiOkResponse type is set correctly for the
    listBusinessReportAlerts method or create a ticket to address this later.

    services/workflows-service/src/alert/alert.controller.external.ts [137]

    [email protected]({ type: Array<TAlertMerchantResponse> }) // TODO: Set type
    [email protected]({ type: Array<TAlertMerchantResponse> }) // Ensure correct response type is set or refer to TICKET-1234
     
    
    Use a utility function to handle pagination logic for better code reuse and error handling.

    Instead of manually constructing the pagination logic with parseInt, consider using a
    utility function or library that handles pagination parameters safely and efficiently.

    services/workflows-service/src/business-report/business-report.service.ts [58-62]

    -const size = parseInt(getTransactionsParameters.page.size as unknown as string, 10);
    -const number = parseInt(getTransactionsParameters.page.number as unknown as string, 10);
    -args.take = size;
    -args.skip = size * (number - 1);
    +const pagination = this.getPagination(getTransactionsParameters.page);
    +args = { ...args, ...pagination };
    +// Elsewhere in the class
    +private getPagination(page: PageDto): { take: number; skip: number } {
    +  const size = parseInt(page.size as unknown as string, 10);
    +  const number = parseInt(page.number as unknown as string, 10);
    +  return {
    +    take: size,
    +    skip: size * (number - 1)
    +  };
    +}
     
    
    Improve type safety and error handling when accessing properties of potentially undefined objects.

    Consider using TypeScript's non-null assertion operator or a type guard to ensure
    reportData is of type TReportWithRiskScore before accessing summary.riskScore. This will
    prevent runtime errors if reportData is not the expected type.

    services/workflows-service/src/workflow/hook-callback-handler.service.ts [138]

    -const reportRiskScore = (reportData as TReportWithRiskScore).summary.riskScore;
    +const reportRiskScore = (reportData as TReportWithRiskScore)?.summary.riskScore ?? 0;
     
    
    Ensure type safety by validating types before asserting them.

    Avoid using as for type assertions without prior validation. Instead, validate
    currentReportId to ensure it is a string before using it in your database queries to
    prevent potential runtime type errors.

    services/workflows-service/src/workflow/hook-callback-handler.service.ts [146]

    -const currentReportId = reportId as string;
    +if (typeof reportId !== 'string') {
    +  throw new Error('reportId must be a string');
    +}
    +const currentReportId = reportId;
     
    
    Enhancement
    Add error handling to the getAlerts method to manage exceptions.

    Consider adding error handling for the getAlerts method to manage exceptions that may
    occur during the database query operation. This will improve the robustness of your
    application by preventing it from crashing due to unhandled exceptions.

    services/workflows-service/src/alert/alert.service.ts [79-84]

     async getAlerts(
       findAlertsDto: FindAlertsDto,
       monitoringType: MonitoringType,
       projectIds: TProjectId[],
       args?: Omit<
         Parameters<typeof this.alertRepository.findMany>[0],
    +) {
    +  try {
    +    // Existing code here
    +  } catch (error) {
    +    // Handle error appropriately
    +  }
    +}
     
    
    Simplify property access and assignment using optional chaining.

    Use TypeScript's optional chaining when accessing deeply nested properties to simplify the
    code and enhance readability.

    services/workflows-service/src/business-report/business-report.service.ts [65-67]

    -if (getTransactionsParameters.orderBy) {
    -  args.orderBy = toPrismaOrderByGeneric(getTransactionsParameters.orderBy);
    -}
    +args.orderBy = getTransactionsParameters.orderBy ? toPrismaOrderByGeneric(getTransactionsParameters.orderBy) : undefined;
     
    
    Improve code readability and error handling by using async/await.

    Use async/await for handling promises instead of .then and .catch for better readability
    and error handling.

    services/workflows-service/src/workflow/hook-callback-handler.service.ts [188-202]

    -this.alertService
    -  .checkOngoingMonitoringAlert(
    +try {
    +  await this.alertService.checkOngoingMonitoringAlert(
         {
           businessId: business.id,
           projectId: currentProjectId,
           reportId: currentReportId,
         },
         business.companyName,
    -  )
    -  .then(() => {
    -    this.logger.debug(`Alert Tested for ${currentReportId}}`);
    -  })
    -  .catch(error => {
    -    this.logger.error(error);
    -  });
    +  );
    +  this.logger.debug(`Alert Tested for ${currentReportId}`);
    +} catch (error) {
    +  this.logger.error(error);
    +}
     
    
    Performance
    Optimize the sortBySeverity method using a map for severity values.

    To improve the performance of the sortBySeverity method, consider using a map to store the
    severity values instead of a switch statement. This will reduce the complexity and
    potentially enhance the performance of the method.

    services/workflows-service/src/alert/alert.service.ts [407-426]

     sortBySeverity(a: AlertSeverity, b: AlertSeverity) {
    -  const alertSeverityToNumber = (severity: AlertSeverity) => {
    -    switch (severity) {
    -      case AlertSeverity.high:
    -        return 3;
    -      case AlertSeverity.medium:
    -        return 2;
    -      case AlertSeverity.low:
    -        return 1;
    -      default:
    -        return 0;
    -    }
    +  const severityMap = {
    +    [AlertSeverity.high]: 3,
    +    [AlertSeverity.medium]: 2,
    +    [AlertSeverity.low]: 1,
       };
     
    +  const alertSeverityToNumber = (severity: AlertSeverity) => severityMap[severity] || 0;
    +
    +  if (a === b) {
    +    return 0;
    +  }
    +
    +  return alertSeverityToNumber(a) < alertSeverityToNumber(b) ? 1 : -1;
    +}
    +
    
    Possible issue
    Add null checks for nested properties access to prevent runtime errors.

    Consider checking for null or undefined values when accessing nested properties of
    getTransactionsParameters.page to prevent runtime errors if page is not provided.

    services/workflows-service/src/business-report/business-report.service.ts [58-59]

    -const size = parseInt(getTransactionsParameters.page.size as unknown as string, 10);
    -const number = parseInt(getTransactionsParameters.page.number as unknown as string, 10);
    +const size = getTransactionsParameters.page ? parseInt(getTransactionsParameters.page.size as unknown as string, 10) : defaultSize;
    +const number = getTransactionsParameters.page ? parseInt(getTransactionsParameters.page.number as unknown as string, 10) : defaultNumber;
     
    
    Prevent accessing properties of null objects by ensuring the object is not null.

    To avoid potential null reference errors, ensure that business is not null before
    accessing its properties like id and companyName.

    services/workflows-service/src/workflow/hook-callback-handler.service.ts [144-196]

     if (!business) throw new BadRequestException('Business not found.');
    +const { id: businessId, companyName } = business;
     this.alertService
       .checkOngoingMonitoringAlert(
         {
    -      businessId: business.id,
    +      businessId,
           projectId: currentProjectId,
           reportId: currentReportId,
         },
    -    business.companyName,
    +    companyName,
       )
     
    
    Robustness
    Add error handling for parsing pagination parameters to prevent runtime exceptions.

    To ensure the robustness of the application, add error handling for potential parsing
    errors when converting pagination parameters.

    services/workflows-service/src/business-report/business-report.service.ts [58-59]

    -const size = parseInt(getTransactionsParameters.page.size as unknown as string, 10);
    -const number = parseInt(getTransactionsParameters.page.number as unknown as string, 10);
    +let size, number;
    +try {
    +  size = parseInt(getTransactionsParameters.page.size as unknown as string, 10);
    +  number = parseInt(getTransactionsParameters.page.number as unknown as string, 10);
    +} catch (error) {
    +  this.logger.error('Error parsing pagination parameters', { error });
    +  throw new Error('Invalid pagination parameters');
    +}
     
    

    github-actions[bot] avatar May 07 '24 13:05 github-actions[bot]