Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(application-system): Refactor env config #15096

Merged
merged 43 commits into from
Jun 13, 2024

Conversation

Halli1414
Copy link
Contributor

@Halli1414 Halli1414 commented Jun 4, 2024

...

Attach a link to issue if relevant

What

Refactor clients to use the nestjs config module instead of using dynamic modules and register config from there.

Why

Some clients are using the dynamic method and others are using the config module method, this will hopefully unify the approch taken in the applicaiton system.

Some clients are also used in other projects, but the functionality should remain the same.

Feature deploy link: https://choreapplication-system-update-env-config-beta.dev01.devland.is/

Screenshots / Gifs

Attach Screenshots / Gifs to help reviewers understand the scope of the pull request

Checklist:

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • Formatting passes locally with my changes
  • I have rebased against main before asking for a review

Summary by CodeRabbit

  • Refactor

    • Streamlined module imports and configurations for various services, including criminal records, health insurance, and VMST.
    • Simplified module setups by removing redundant configuration methods and directly importing necessary modules.
  • New Features

    • Introduced new configuration settings for the VmstClient, enhancing flexibility and control over API interactions.

@Halli1414 Halli1414 added the deploy-feature Deploys features to dev label Jun 6, 2024
@Halli1414 Halli1414 marked this pull request as ready for review June 6, 2024 13:09
@Halli1414 Halli1414 requested review from a team as code owners June 6, 2024 13:09
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 12

Outside diff range and nitpick comments (2)
libs/application/template-api-modules/src/lib/modules/templates/criminal-record-submission/criminal-record-submission.module.ts (1)

Line range hint 11-24: Refactor the class to a simple function to improve maintainability.

- export class CriminalRecordSubmissionModule {
-   static register(baseConfig: BaseTemplateAPIModuleConfig): DynamicModule {
-     return {
-       module: CriminalRecordSubmissionModule,
-       imports: [
-         SharedTemplateAPIModule.register(baseConfig),
-         CriminalRecordModule,
-         SyslumennClientModule,
-       ],
-       providers: [CriminalRecordSubmissionService],
-       exports: [CriminalRecordSubmissionService],
-     }
-   }
- }
+ export function registerCriminalRecordSubmissionModule(baseConfig: BaseTemplateAPIModuleConfig): DynamicModule {
+   return {
+     module: CriminalRecordSubmissionModule,
+     imports: [
+       SharedTemplateAPIModule.register(baseConfig),
+       CriminalRecordModule,
+       SyslumennClientModule,
+     ],
+     providers: [CriminalRecordSubmissionService],
+     exports: [CriminalRecordSubmissionService],
+   }
+ }
libs/application/template-api-modules/src/lib/modules/templates/operating-license/operatingLicense.module.ts (1)

Line range hint 10-25: Refactor the class to a simple function to improve maintainability.

- export class OperatingLicenseModule {
-   static register(config: BaseTemplateAPIModuleConfig): DynamicModule {
-     return {
-       module: OperatingLicenseModule,
-       imports: [
-         SyslumennClientModule,
-         SharedTemplateAPIModule.register(config),
-         CriminalRecordModule,
-         FinanceClientModule,
-         JudicialAdministrationClientModule,
-       ],
-       providers: [OperatingLicenseService],
-       exports: [OperatingLicenseService],
-     }
-   }
- }
+ export function registerOperatingLicenseModule(config: BaseTemplateAPIModuleConfig): DynamicModule {
+   return {
+     module: OperatingLicenseModule,
+     imports: [
+       SyslumennClientModule,
+       SharedTemplateAPIModule.register(config),
+       CriminalRecordModule,
+       FinanceClientModule,
+       JudicialAdministrationClientModule,
+     ],
+     providers: [OperatingLicenseService],
+     exports: [OperatingLicenseService],
+   }
+ }
Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between d380505 and f6969fb.

Files selected for processing (37)
  • apps/api/src/app/app.module.ts (4 hunks)
  • apps/api/src/app/environments/environment.ts (4 hunks)
  • apps/application-system/api/src/app/app.module.ts (2 hunks)
  • apps/application-system/api/src/environments/environment.ts (2 hunks)
  • libs/api/domains/criminal-record/src/lib/criminalRecord.module.ts (1 hunks)
  • libs/api/domains/criminal-record/src/lib/criminalRecord.spec.ts (2 hunks)
  • libs/api/domains/directorate-of-labour/src/lib/directorate-of-labour.module.ts (2 hunks)
  • libs/api/domains/health-insurance/src/lib/api-domains-health-insurance.spec.ts (2 hunks)
  • libs/api/domains/health-insurance/src/lib/healthInsurance.module.ts (1 hunks)
  • libs/application/template-api-modules/src/lib/modules/templates/accident-notification/accident-notification.module.ts (2 hunks)
  • libs/application/template-api-modules/src/lib/modules/templates/criminal-record-submission/criminal-record-submission.module.ts (1 hunks)
  • libs/application/template-api-modules/src/lib/modules/templates/data-protection-complaint/data-protection-complaint.module.ts (1 hunks)
  • libs/application/template-api-modules/src/lib/modules/templates/health-insurance/health-insurance.module.ts (1 hunks)
  • libs/application/template-api-modules/src/lib/modules/templates/operating-license/operatingLicense.module.ts (1 hunks)
  • libs/application/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.module.ts (1 hunks)
  • libs/application/template-api-modules/src/lib/types/index.ts (2 hunks)
  • libs/clients/althingi-ombudsman/src/lib/apiConfiguration.ts (3 hunks)
  • libs/clients/criminal-record/src/index.ts (1 hunks)
  • libs/clients/criminal-record/src/lib/apiConfiguration.ts (1 hunks)
  • libs/clients/criminal-record/src/lib/criminalRecordApi.config.ts (1 hunks)
  • libs/clients/criminal-record/src/lib/criminalRecordApi.module.ts (1 hunks)
  • libs/clients/data-protection-complaint/src/index.ts (1 hunks)
  • libs/clients/data-protection-complaint/src/lib/apiConfiguration.ts (1 hunks)
  • libs/clients/data-protection-complaint/src/lib/apis.ts (1 hunks)
  • libs/clients/data-protection-complaint/src/lib/config/config.ts (1 hunks)
  • libs/clients/data-protection-complaint/src/lib/data-protection-complaint-client.config.ts (1 hunks)
  • libs/clients/data-protection-complaint/src/lib/data-protection-complaint.module.ts (1 hunks)
  • libs/clients/icelandic-health-insurance/health-insurance/src/index.ts (1 hunks)
  • libs/clients/icelandic-health-insurance/health-insurance/src/lib/apiConfiguration.ts (1 hunks)
  • libs/clients/icelandic-health-insurance/health-insurance/src/lib/apis.ts (1 hunks)
  • libs/clients/icelandic-health-insurance/health-insurance/src/lib/clients-health-insurance-v2.config.ts (1 hunks)
  • libs/clients/icelandic-health-insurance/health-insurance/src/lib/clients-health-insurance-v2.module.ts (1 hunks)
  • libs/clients/vmst/src/index.ts (1 hunks)
  • libs/clients/vmst/src/lib/apiConfiguration.ts (1 hunks)
  • libs/clients/vmst/src/lib/apis.ts (1 hunks)
  • libs/clients/vmst/src/lib/vmst.config.ts (1 hunks)
  • libs/clients/vmst/src/lib/vmst.module.ts (1 hunks)
Files skipped from review due to trivial changes (3)
  • apps/application-system/api/src/environments/environment.ts
  • libs/clients/althingi-ombudsman/src/lib/apiConfiguration.ts
  • libs/clients/vmst/src/index.ts
Additional context used
Path-based instructions (34)
libs/clients/data-protection-complaint/src/lib/config/config.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/icelandic-health-insurance/health-insurance/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/criminal-record/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/icelandic-health-insurance/health-insurance/src/lib/clients-health-insurance-v2.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."
libs/clients/icelandic-health-insurance/health-insurance/src/lib/apis.ts (1)

Pattern libs/**/*: "Confirm that the code adheres to the following:

  • Reusability of components and hooks across different NextJS apps.
  • TypeScript usage for defining props and exporting types.
  • Effective tree-shaking and bundling practices."
libs/api/domains/criminal-record/src/lib/criminalRecord.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."
libs/clients/vmst/src/lib/vmst.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."
libs/clients/data-protection-complaint/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/criminal-record/src/lib/criminalRecordApi.config.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/data-protection-complaint/src/lib/apis.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/vmst/src/lib/apis.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/criminal-record/src/lib/criminalRecordApi.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."
libs/api/domains/directorate-of-labour/src/lib/directorate-of-labour.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."
libs/api/domains/health-insurance/src/lib/healthInsurance.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."
libs/clients/data-protection-complaint/src/lib/data-protection-complaint-client.config.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/icelandic-health-insurance/health-insurance/src/lib/clients-health-insurance-v2.config.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/modules/templates/criminal-record-submission/criminal-record-submission.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."
libs/clients/vmst/src/lib/vmst.config.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/data-protection-complaint/src/lib/data-protection-complaint.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."
libs/application/template-api-modules/src/lib/modules/templates/operating-license/operatingLicense.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."
libs/clients/criminal-record/src/lib/apiConfiguration.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/modules/templates/health-insurance/health-insurance.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."
libs/clients/data-protection-complaint/src/lib/apiConfiguration.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/vmst/src/lib/apiConfiguration.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/icelandic-health-insurance/health-insurance/src/lib/apiConfiguration.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/modules/templates/parental-leave/parental-leave.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."
libs/application/template-api-modules/src/lib/modules/templates/data-protection-complaint/data-protection-complaint.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."
libs/application/template-api-modules/src/lib/modules/templates/accident-notification/accident-notification.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."
libs/api/domains/health-insurance/src/lib/api-domains-health-insurance.spec.ts (1)

Pattern libs/**/*: "Confirm that the code adheres to the following:

  • Reusability of components and hooks across different NextJS apps.
  • TypeScript usage for defining props and exporting types.
  • Effective tree-shaking and bundling practices."
libs/api/domains/criminal-record/src/lib/criminalRecord.spec.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/types/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."
apps/application-system/api/src/app/app.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/api/src/app/environments/environment.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/api/src/app/app.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."
Biome
libs/api/domains/directorate-of-labour/src/lib/directorate-of-labour.module.ts

[error] 8-21: Avoid classes that contain only static members. (lint/complexity/noStaticOnlyClass)

Prefer using simple functions instead of classes with only static members.

libs/application/template-api-modules/src/lib/modules/templates/criminal-record-submission/criminal-record-submission.module.ts

[error] 11-24: Avoid classes that contain only static members. (lint/complexity/noStaticOnlyClass)

Prefer using simple functions instead of classes with only static members.

libs/application/template-api-modules/src/lib/modules/templates/operating-license/operatingLicense.module.ts

[error] 10-25: Avoid classes that contain only static members. (lint/complexity/noStaticOnlyClass)

Prefer using simple functions instead of classes with only static members.

libs/application/template-api-modules/src/lib/modules/templates/health-insurance/health-insurance.module.ts

[error] 15-27: Avoid classes that contain only static members. (lint/complexity/noStaticOnlyClass)

Prefer using simple functions instead of classes with only static members.

libs/application/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.module.ts

[error] 21-44: Avoid classes that contain only static members. (lint/complexity/noStaticOnlyClass)

Prefer using simple functions instead of classes with only static members.

libs/application/template-api-modules/src/lib/modules/templates/data-protection-complaint/data-protection-complaint.module.ts

[error] 18-36: Avoid classes that contain only static members. (lint/complexity/noStaticOnlyClass)

Prefer using simple functions instead of classes with only static members.

libs/application/template-api-modules/src/lib/modules/templates/accident-notification/accident-notification.module.ts

[error] 23-53: Avoid classes that contain only static members. (lint/complexity/noStaticOnlyClass)

Prefer using simple functions instead of classes with only static members.

Additional comments not posted (23)
libs/clients/icelandic-health-insurance/health-insurance/src/index.ts (1)

1-3: Exports are correctly set up for modular architecture.

The file correctly exports the necessary modules and configurations, aligning with the PR's objectives to streamline client configurations.

libs/clients/criminal-record/src/index.ts (1)

6-6: Good addition of centralized configuration.

The export of CriminalRecordClientConfig enhances the maintainability and scalability of the configuration management.

libs/clients/icelandic-health-insurance/health-insurance/src/lib/clients-health-insurance-v2.module.ts (1)

5-9: Module setup is efficient and scalable.

The use of spread operators for including APIs in the exports and providers list is a good practice, enhancing the module's scalability and maintainability.

libs/clients/icelandic-health-insurance/health-insurance/src/lib/apis.ts (1)

1-9: The dynamic API configuration approach using a factory pattern enhances modularity and maintainability.

libs/api/domains/criminal-record/src/lib/criminalRecord.module.ts (1)

1-11: The refactoring of CriminalRecordModule aligns with best practices for modularity and simplicity in service configuration.

libs/clients/vmst/src/lib/vmst.module.ts (1)

1-14: The configuration of VMSTModule is well-structured, promoting clear dependency management and ease of maintenance.

libs/clients/data-protection-complaint/src/index.ts (1)

12-12: Correctly exporting DataProtectionComplaintClientConfig ensures that configuration settings are accessible where needed, aligning with best practices for modular architecture.

libs/clients/criminal-record/src/lib/criminalRecordApi.config.ts (1)

1-17: Ensure the CriminalRecordClientConfig adheres to the new configuration standards.

This configuration setup using defineConfig and zod for schema validation aligns with the project's goal of standardizing configuration management. The explicit declaration of environment variables and default values enhances maintainability and clarity.

libs/clients/vmst/src/lib/apis.ts (1)

1-23: Standardize API factory implementations.

The use of a factory function for API instantiation is a good practice as it encapsulates the creation logic and makes the module more adaptable to changes in the configuration or the instantiation process.

libs/api/domains/health-insurance/src/lib/healthInsurance.module.ts (1)

11-11: Ensure all necessary services are exported.

Please verify that all services that need to be available externally are properly exported.

libs/clients/criminal-record/src/lib/apiConfiguration.ts (1)

6-25: Configuration setup using NestJS config module looks correct and well-structured.

libs/clients/data-protection-complaint/src/lib/apiConfiguration.ts (1)

6-26: Configuration setup using NestJS config module looks correct and well-structured.

libs/clients/vmst/src/lib/apiConfiguration.ts (1)

10-30: Configuration setup using NestJS config module looks correct and well-structured.

libs/clients/icelandic-health-insurance/health-insurance/src/lib/apiConfiguration.ts (1)

6-29: Configuration setup for Health Insurance V2 client looks good.

Ensure that sensitive information such as usernames and passwords are handled securely, possibly using environment variables or secure vault solutions.

libs/api/domains/health-insurance/src/lib/api-domains-health-insurance.spec.ts (3)

5-6: Imports for HealthInsuranceV2ClientConfig and HealthInsuranceV2ClientModule are correctly added to support the new module configuration.


9-10: Proper use of ConfigModule and XRoadConfig to ensure configurations are globally available and correctly set up for the Health Insurance service.


22-28: Configuration setup in the beforeEach block is correctly implemented, ensuring that all necessary settings are available for testing the Health Insurance service.

libs/api/domains/criminal-record/src/lib/criminalRecord.spec.ts (2)

10-11: Imports for ConfigModule and defineConfig are correctly added to support the new configuration setup for the Criminal Record service.


21-38: Configuration setup in the beforeEach block is correctly implemented, ensuring that all necessary settings are available for testing the Criminal Record service.

libs/application/template-api-modules/src/lib/types/index.ts (1)

1-1: Removal of imports for configurations that are no longer used directly aligns with the refactoring efforts and simplifies the type definitions.

apps/application-system/api/src/app/app.module.ts (1)

41-44: Addition of new client configurations (DataProtectionComplaintClientConfig, CriminalRecordClientConfig, HealthInsuranceV2ClientConfig, VmstClientConfig) is correctly implemented, ensuring that all necessary settings are available for the application system.

apps/api/src/app/environments/environment.ts (1)

Line range hint 1-406: Configuration cleanup aligns with PR objectives.

The removal of specific service configurations for criminal records, health insurance, VMST, and data protection complaints is consistent with the PR's goal to refactor and simplify the environment configuration. Ensure that all dependent modules and services are updated to use the new configuration approach.

apps/api/src/app/app.module.ts (1)

188-190: Updated imports and configurations align with PR objectives.

The changes in imports from specific modules to client configurations for criminal records, health insurance, VMST, and data protection complaints are consistent with the PR's goal to refactor and simplify module imports. This should help in maintaining a cleaner and more manageable codebase. Ensure that all dependent services and modules are properly adjusted to these changes.

Also applies to: 252-252, 307-307, 406-408

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Outside diff range and nitpick comments (1)
libs/application/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.module.ts (1)

Line range hint 15-38: Refactor the static class to a simple function to enhance maintainability.

- export class ParentalLeaveModule {
-   static register(config: BaseTemplateAPIModuleConfig): DynamicModule {
+ export function registerParentalLeaveModule(config: BaseTemplateAPIModuleConfig): DynamicModule {
      return {
        module: ParentalLeaveModule,
        imports: [
          VMSTModule,
          SharedTemplateAPIModule.register(config),
          SmsModule.register(config.smsOptions),
          ApplicationApiCoreModule,
          NationalRegistryClientModule,
        ],
        providers: [
          ChildrenService,
          ParentalLeaveService,
          NationalRegistryClientService,
          {
            provide: APPLICATION_ATTACHMENT_BUCKET,
            useFactory: () => config.attachmentBucket,
          },
        ],
        exports: [ParentalLeaveService],
-     }
-   }
+   };
+ }
Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between f6969fb and bf7db30.

Files selected for processing (6)
  • apps/api/src/app/app.module.ts (5 hunks)
  • libs/api/domains/directorate-of-labour/src/lib/directorate-of-labour.module.ts (1 hunks)
  • libs/application/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.module.ts (2 hunks)
  • libs/clients/data-protection-complaint/src/lib/data-protection-complaint.module.ts (1 hunks)
  • libs/clients/vmst/src/lib/apiConfiguration.ts (1 hunks)
  • libs/clients/vmst/src/lib/vmst.config.ts (1 hunks)
Files skipped from review as they are similar to previous changes (5)
  • apps/api/src/app/app.module.ts
  • libs/api/domains/directorate-of-labour/src/lib/directorate-of-labour.module.ts
  • libs/clients/data-protection-complaint/src/lib/data-protection-complaint.module.ts
  • libs/clients/vmst/src/lib/apiConfiguration.ts
  • libs/clients/vmst/src/lib/vmst.config.ts
Additional context used
Path-based instructions (1)
libs/application/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.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
libs/application/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.module.ts

[error] 15-38: Avoid classes that contain only static members. (lint/complexity/noStaticOnlyClass)

Prefer using simple functions instead of classes with only static members.

@Halli1414 Halli1414 marked this pull request as draft June 6, 2024 16:12
@Halli1414 Halli1414 marked this pull request as ready for review June 13, 2024 09:52
Copy link
Member

@baering baering left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Core files LGTM

Copy link
Member

@joningi98 joningi98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Member

@berglindoma13 berglindoma13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@jonnigs jonnigs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Halli1414 Halli1414 added the deprecated:automerge (Disabled) Merge this PR as soon as all checks pass label Jun 13, 2024
@kodiakhq kodiakhq bot merged commit be82e15 into main Jun 13, 2024
44 of 48 checks passed
@kodiakhq kodiakhq bot deleted the chore/application-system-update-env-config branch June 13, 2024 12:53
sigruntg pushed a commit that referenced this pull request Jun 14, 2024
* Data Protection config refactor

* remove unused variable

* Criminal Record client config refactor

* HealthInsurance client config refactor

* remove logging

* Vmst client config refactor

* Data Protection config refactor

* remove unused variable

* Criminal Record client config refactor

* Vmst refactor

* clean up

* Health insurance client config refactor

* client path fix

* Revert "Vmst client config refactor"

This reverts commit d2855b7.

* basePath update

* vmst client refactor round 2

* Data Protection config refactor

* remove unused variable

* Criminal Record client config refactor

* Vmst refactor

* clean up

* Health insurance client config refactor

* client path fix

* Data Protection config refactor

* remove unused variable

* Criminal Record client config refactor

* HealthInsurance client config refactor

* Vmst client config refactor

* Revert "Vmst client config refactor"

This reverts commit d2855b7.

* vmst client refactor round 2

* Doublicate key clean up

* fallback value for env

* Fallback for env variables

* chore: nx format:write update dirty files

* Update tests to use new module config

* Propper mock values

* Force feature deploy

* clean up

* typo fix and other minor clean up

---------

Co-authored-by: andes-it <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
lodmfjord pushed a commit that referenced this pull request Jun 14, 2024
* Data Protection config refactor

* remove unused variable

* Criminal Record client config refactor

* HealthInsurance client config refactor

* remove logging

* Vmst client config refactor

* Data Protection config refactor

* remove unused variable

* Criminal Record client config refactor

* Vmst refactor

* clean up

* Health insurance client config refactor

* client path fix

* Revert "Vmst client config refactor"

This reverts commit d2855b7.

* basePath update

* vmst client refactor round 2

* Data Protection config refactor

* remove unused variable

* Criminal Record client config refactor

* Vmst refactor

* clean up

* Health insurance client config refactor

* client path fix

* Data Protection config refactor

* remove unused variable

* Criminal Record client config refactor

* HealthInsurance client config refactor

* Vmst client config refactor

* Revert "Vmst client config refactor"

This reverts commit d2855b7.

* vmst client refactor round 2

* Doublicate key clean up

* fallback value for env

* Fallback for env variables

* chore: nx format:write update dirty files

* Update tests to use new module config

* Propper mock values

* Force feature deploy

* clean up

* typo fix and other minor clean up

---------

Co-authored-by: andes-it <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy-feature Deploys features to dev deprecated:automerge (Disabled) Merge this PR as soon as all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants