feat: base code for providerChains and chainMembers #449
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kshitij-k-osmosys
added
ready for review
Fallback Provider Feature
WalkthroughThis change introduces a fallback provider chain feature by adding two new database tables ( Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant NotificationService
participant ProviderChainsService
participant ProviderChainMembersService
participant Database
Client->>NotificationService: Request notification send
NotificationService->>ProviderChainsService: Get provider chain by name
ProviderChainsService->>Database: Query notify_provider_chains
ProviderChainsService-->>NotificationService: Return ProviderChain
NotificationService->>ProviderChainMembersService: Get first priority provider by chainId
ProviderChainMembersService->>Database: Query notify_provider_chain_members
ProviderChainMembersService-->>NotificationService: Return providerId
NotificationService->>Database: Create notification with providerChainId
NotificationService-->>Client: Acknowledge/send notification
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
apps/api/src/modules/provider-chains/provider-chains.service.spec.ts (1)
1-18: Tests assert existence only – flesh out behaviour-level cases.The spec stops at “should be defined”. With business logic already in
ProviderChainsService(e.g.,getByProviderChainNamefiltering byStatus.ACTIVE), missing tests means:• No guarantee that the active-status filter remains intact.
• No safety net for edge cases (non-existent name, duplicates, etc.).Add repository mocks and cover at least:
- Resolves active chain by name.
- Returns
undefined/ throws on inactive or absent chain.- Respects default chain selection (once implemented).
apps/api/src/modules/provider-chain-members/provider-chain-members.service.spec.ts (1)
1-18: Placeholder test offers no value – add real scenarios or remove until logic exists.As with the provider-chains spec, a lone “defined” assertion neither documents intent nor guards behaviour.
When CRUD or query helpers land inProviderChainMembersService, accompany them with:• Happy-path retrieval/creation.
• FK-constraint / uniqueness violations.
• Membership ordering / weighting logic (if applicable).Until then, this file adds CI noise without coverage gain.
apps/api/src/modules/notifications/notifications.module.ts (1)
50-52: Module-list inconsistency – keep all provider-chain modules together for maintainability.
ProviderChainsModule&ProviderChainMembersModuleare imported directly rather than added toproviderModules, breaking the “single list” pattern used for all other provider-related modules.const providerModules = [ ... SmsSnsModule, + ProviderChainsModule, + ProviderChainMembersModule, ]; @Module({ imports: [ TypeOrmModule.forFeature([Notification, RetryNotification]), - ...providerModules, - WebhookModule, - ArchivedNotificationsModule, - ProviderChainsModule, - ProviderChainMembersModule, + ...providerModules, + WebhookModule, + ArchivedNotificationsModule, ],This keeps the array authoritative, avoids drift, and makes future additions trivial.
Also applies to: 93-95
apps/api/src/modules/providers/entities/provider.entity.ts (1)
84-89: Fix parameter naming consistency in relationship mapping.The relationship mapping parameter should use camelCase to maintain consistency with other relationship mappings in the entity.
Apply this diff to fix the parameter naming:
@OneToMany( () => ProviderChainMember, - (ProviderChainMember) => ProviderChainMember.providerDetails, + (providerChainMember) => providerChainMember.providerDetails, )apps/api/src/modules/provider-chains/entities/provider-chain.entity.ts (1)
39-46: Add nullable option to GraphQL field decorator for consistency.The
descriptionfield is defined as nullable in the database column but the GraphQL field decorator doesn't specify{ nullable: true }.@Column({ name: 'description', type: 'text', nullable: true, }) @IsOptional() - @Field() + @Field({ nullable: true }) description: string;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/api/src/common/constants/database.ts(1 hunks)apps/api/src/database/migrations/1752904253632-FallbackProviderChanges.ts(1 hunks)apps/api/src/modules/applications/entities/application.entity.ts(2 hunks)apps/api/src/modules/archived-notifications/entities/archived-notification.entity.ts(3 hunks)apps/api/src/modules/notifications/entities/notification.entity.ts(3 hunks)apps/api/src/modules/notifications/notifications.module.ts(2 hunks)apps/api/src/modules/provider-chain-members/entities/provider-chain-member.entity.ts(1 hunks)apps/api/src/modules/provider-chain-members/provider-chain-members.module.ts(1 hunks)apps/api/src/modules/provider-chain-members/provider-chain-members.service.spec.ts(1 hunks)apps/api/src/modules/provider-chain-members/provider-chain-members.service.ts(1 hunks)apps/api/src/modules/provider-chains/entities/provider-chain.entity.ts(1 hunks)apps/api/src/modules/provider-chains/provider-chains.module.ts(1 hunks)apps/api/src/modules/provider-chains/provider-chains.service.spec.ts(1 hunks)apps/api/src/modules/provider-chains/provider-chains.service.ts(1 hunks)apps/api/src/modules/providers/entities/provider.entity.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (14)
apps/api/src/modules/provider-chain-members/provider-chain-members.service.spec.ts (2)
Learnt from: LakshayaT
PR: OsmosysSoftware/osmo-x#283
File: apps/api/src/modules/providers/vc-twilio/vc-twilio.service.spec.ts:1-18
Timestamp: 2024-07-17T12:19:36.667Z
Learning: LakshayaT and their team use a separate Postman collection for testing and do not modify autogenerated test files like `vc-twilio.service.spec.ts`.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#443
File: apps/api/src/common/guards/api-key/api-key.guard.ts:161-175
Timestamp: 2025-07-13T14:37:39.774Z
Learning: The `getByProviderChainName` method in `ProviderChainsService` already filters by `status: Status.ACTIVE` in its TypeORM query, making additional status checks in calling code redundant.
apps/api/src/modules/notifications/notifications.module.ts (5)
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#332
File: apps/portal/src/app/views/notifications/notifications.component.ts:0-0
Timestamp: 2024-10-08T17:27:45.631Z
Learning: When the code in `notifications.component.ts` is outdated, previous suggestions related to it may not apply.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#332
File: apps/portal/src/app/views/notifications/notifications.component.ts:0-0
Timestamp: 2024-09-27T12:15:47.625Z
Learning: When the code in `notifications.component.ts` is outdated, previous suggestions related to it may not apply.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#360
File: apps/api/src/modules/notifications/notifications.service.ts:0-0
Timestamp: 2024-11-20T06:08:32.855Z
Learning: The `findNotificationsToArchive` function has been removed from `notifications.service.ts`, and archiving tasks now use `QueryRunner` directly in `archived-notifications.service.ts`.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#368
File: apps/portal/src/app/views/notifications/notification.model.ts:21-32
Timestamp: 2024-11-28T12:52:01.779Z
Learning: In the TypeScript file `apps/portal/src/app/views/notifications/notification.model.ts`, the `ArchivedNotification` interface intentionally has a different structure from the `Notification` interface, specifically using `notificationId` instead of `id`.
Learnt from: ayushnvs
PR: OsmosysSoftware/osmo-x#262
File: apps/api/src/jobs/consumers/notifications/mailgun-notifications.job.consumer.ts:0-0
Timestamp: 2024-07-03T13:27:46.550Z
Learning: The `notificationSendResponse` variable in the `processMailgunNotificationConfirmationQueue` method of the `MailgunNotificationConsumer` class was changed from `let` to `const` to reflect that it is never reassigned.
apps/api/src/modules/provider-chains/provider-chains.service.spec.ts (3)
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#443
File: apps/api/src/common/guards/api-key/api-key.guard.ts:161-175
Timestamp: 2025-07-13T14:37:39.774Z
Learning: The `getByProviderChainName` method in `ProviderChainsService` already filters by `status: Status.ACTIVE` in its TypeORM query, making additional status checks in calling code redundant.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#443
File: apps/api/src/common/guards/api-key/api-key.guard.ts:161-175
Timestamp: 2025-07-13T14:37:39.774Z
Learning: The `getByProviderChainName` method in `ProviderChainsService` already filters for active status, so additional status checks in calling code are unnecessary.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#443
File: apps/api/src/common/guards/api-key/api-key.guard.ts:161-175
Timestamp: 2025-07-13T14:37:39.774Z
Learning: The `getByProviderChainName` method in `ProviderChainsService` already filters for active status at the service layer, making additional status checks in calling code redundant. This follows the pattern where service methods handle status validation internally.
apps/api/src/modules/provider-chain-members/provider-chain-members.service.ts (5)
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#443
File: apps/api/src/common/guards/api-key/api-key.guard.ts:161-175
Timestamp: 2025-07-13T14:37:39.774Z
Learning: The `getByProviderChainName` method in `ProviderChainsService` already filters by `status: Status.ACTIVE` in its TypeORM query, making additional status checks in calling code redundant.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#443
File: apps/api/src/common/guards/api-key/api-key.guard.ts:161-175
Timestamp: 2025-07-13T14:37:39.774Z
Learning: The `getByProviderChainName` method in `ProviderChainsService` already filters for active status at the service layer, making additional status checks in calling code redundant. This follows the pattern where service methods handle status validation internally.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#443
File: apps/api/src/common/guards/api-key/api-key.guard.ts:161-175
Timestamp: 2025-07-13T14:37:39.774Z
Learning: The `getByProviderChainName` method in `ProviderChainsService` already filters for active status, so additional status checks in calling code are unnecessary.
Learnt from: Harish-osmosys
PR: OsmosysSoftware/osmo-x#278
File: apps/api/src/modules/webhook/webhook.service.ts:0-0
Timestamp: 2024-10-08T17:27:45.632Z
Learning: The `WebhookService` class in `apps/api/src/modules/webhook/webhook.service.ts` now uses the `Logger` from `@nestjs/common` for logging instead of `console.error`.
Learnt from: Harish-osmosys
PR: OsmosysSoftware/osmo-x#278
File: apps/api/src/modules/webhook/webhook.service.ts:0-0
Timestamp: 2024-07-12T09:48:15.458Z
Learning: The `WebhookService` class in `apps/api/src/modules/webhook/webhook.service.ts` now uses the `Logger` from `@nestjs/common` for logging instead of `console.error`.
apps/api/src/modules/applications/entities/application.entity.ts (10)
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#443
File: apps/api/src/common/guards/api-key/api-key.guard.ts:161-175
Timestamp: 2025-07-13T14:37:39.774Z
Learning: The `getByProviderChainName` method in `ProviderChainsService` already filters by `status: Status.ACTIVE` in its TypeORM query, making additional status checks in calling code redundant.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#400
File: apps/api/src/modules/applications/entities/application.entity.ts:42-46
Timestamp: 2025-03-07T06:13:20.929Z
Learning: In the OsmoX codebase, JSON data fields in entity classes are often typed as `string` even when they store structured data, with TypeORM handling the conversion between string representation and JSON database type. This pattern is used for consistency across the codebase, particularly for provider configurations.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#368
File: apps/portal/src/app/views/notifications/notification.model.ts:21-32
Timestamp: 2024-11-28T12:52:01.779Z
Learning: In the TypeScript file `apps/portal/src/app/views/notifications/notification.model.ts`, the `ArchivedNotification` interface intentionally has a different structure from the `Notification` interface, specifically using `notificationId` instead of `id`.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#368
File: apps/portal/src/app/graphql/graphql.queries.ts:50-50
Timestamp: 2024-11-28T12:52:15.785Z
Learning: In `apps/portal/src/app/graphql/graphql.queries.ts`, the `GetArchivedNotifications` query uses the field `notificationId` intentionally instead of `id` because `archivedNotifications` has a different structure than `notifications`.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#360
File: apps/api/src/modules/archived-notifications/entities/archived-notification.entity.ts:32-35
Timestamp: 2024-11-20T05:58:43.756Z
Learning: In the codebase, the `Notification` and `ArchivedNotification` entities use `default: null` in the `@Column` decorators for nullable fields, and `@Field()` without `{ nullable: true }`. This pattern is accepted in this project.
Learnt from: Harish-osmosys
PR: OsmosysSoftware/osmo-x#330
File: apps/api/src/modules/applications/applications.module.ts:12-13
Timestamp: 2024-09-24T10:33:24.415Z
Learning: The `ServerApiKeysService` is still used in notification creation via the REST controller.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#360
File: apps/api/src/modules/notifications/notifications.service.ts:0-0
Timestamp: 2024-11-20T06:08:32.855Z
Learning: The `findNotificationsToArchive` function has been removed from `notifications.service.ts`, and archiving tasks now use `QueryRunner` directly in `archived-notifications.service.ts`.
Learnt from: LakshayaT
PR: OsmosysSoftware/osmo-x#218
File: apps/api/src/database/migrations/1692870736645-migration.ts:0-0
Timestamp: 2024-10-08T17:27:45.632Z
Learning: The foreign key for `channel_type` in `notify_notifications` references `master_id` from `notify_master_providers` by design, to keep naming consistent with the table name.
Learnt from: LakshayaT
PR: OsmosysSoftware/osmo-x#218
File: apps/api/src/database/migrations/1692870736645-migration.ts:0-0
Timestamp: 2024-06-10T19:12:20.809Z
Learning: The foreign key for `channel_type` in `notify_notifications` references `master_id` from `notify_master_providers` by design, to keep naming consistent with the table name.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#367
File: apps/api/src/modules/archived-notifications/dtos/archived-notification-response.dto.ts:18-23
Timestamp: 2024-11-28T12:49:15.719Z
Learning: In the TypeScript classes `ArchivedNotificationResponse` and `NotificationResponse` within our NestJS application, we do not include input validation in constructors for response DTOs. It's acceptable to proceed without input validation to maintain consistency.
apps/api/src/modules/archived-notifications/entities/archived-notification.entity.ts (13)
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#368
File: apps/portal/src/app/views/notifications/notification.model.ts:21-32
Timestamp: 2024-11-28T12:52:01.779Z
Learning: In the TypeScript file `apps/portal/src/app/views/notifications/notification.model.ts`, the `ArchivedNotification` interface intentionally has a different structure from the `Notification` interface, specifically using `notificationId` instead of `id`.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#368
File: apps/portal/src/app/graphql/graphql.queries.ts:50-50
Timestamp: 2024-11-28T12:52:15.785Z
Learning: In `apps/portal/src/app/graphql/graphql.queries.ts`, the `GetArchivedNotifications` query uses the field `notificationId` intentionally instead of `id` because `archivedNotifications` has a different structure than `notifications`.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#360
File: apps/api/src/modules/archived-notifications/entities/archived-notification.entity.ts:32-35
Timestamp: 2024-11-20T05:58:43.756Z
Learning: In the codebase, the `Notification` and `ArchivedNotification` entities use `default: null` in the `@Column` decorators for nullable fields, and `@Field()` without `{ nullable: true }`. This pattern is accepted in this project.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#360
File: apps/api/src/modules/notifications/notifications.service.ts:0-0
Timestamp: 2024-11-20T06:08:32.855Z
Learning: The `findNotificationsToArchive` function has been removed from `notifications.service.ts`, and archiving tasks now use `QueryRunner` directly in `archived-notifications.service.ts`.
Learnt from: osm-vishnukyatannawar
PR: OsmosysSoftware/osmo-x#354
File: apps/api/src/database/migrations/1730724383210-ArchiveCompletedNotifications.ts:25-27
Timestamp: 2024-11-08T05:23:26.338Z
Learning: In the 'archived_notifications' table, we do not add a foreign key constraint on 'notification_id' to 'notify_notifications' because the original notifications may have been deleted, and we want to retain archived notifications even if the originals are removed.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#408
File: apps/api/src/database/migrations/1743145236864-AddNotificationSentOnColumn.ts:0-0
Timestamp: 2025-04-02T04:39:38.272Z
Learning: In the Osmo-X project, the `notification_sent_on` column in both `notify_archived_notifications` and `notify_notifications` tables is designed to be nullable, as explicitly defined in the migration file with `isNullable: true`.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#408
File: apps/api/src/database/migrations/1743145236864-AddNotificationSentOnColumn.ts:0-0
Timestamp: 2025-04-02T04:39:34.338Z
Learning: In the 'AddNotificationSentOnColumn1743145236864' migration file, the 'notification_sent_on' column is intentionally defined as nullable (isNullable: true) for both the 'notify_archived_notifications' and 'notify_notifications' tables.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#408
File: apps/api/src/database/migrations/1743145236864-AddNotificationSentOnColumn.ts:0-0
Timestamp: 2025-04-02T04:39:34.338Z
Learning: In the migration file for adding the 'notification_sent_on' column to 'notify_archived_notifications' and 'notify_notifications' tables, the column is defined as nullable.
Learnt from: LakshayaT
PR: OsmosysSoftware/osmo-x#218
File: apps/api/src/database/migrations/1692870736645-migration.ts:0-0
Timestamp: 2024-10-08T17:27:45.632Z
Learning: The foreign key for `channel_type` in `notify_notifications` references `master_id` from `notify_master_providers` by design, to keep naming consistent with the table name.
Learnt from: LakshayaT
PR: OsmosysSoftware/osmo-x#218
File: apps/api/src/database/migrations/1692870736645-migration.ts:0-0
Timestamp: 2024-06-10T19:12:20.809Z
Learning: The foreign key for `channel_type` in `notify_notifications` references `master_id` from `notify_master_providers` by design, to keep naming consistent with the table name.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#400
File: apps/api/src/modules/applications/entities/application.entity.ts:42-46
Timestamp: 2025-03-07T06:13:20.929Z
Learning: In the OsmoX codebase, JSON data fields in entity classes are often typed as `string` even when they store structured data, with TypeORM handling the conversion between string representation and JSON database type. This pattern is used for consistency across the codebase, particularly for provider configurations.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#367
File: apps/api/src/modules/archived-notifications/dtos/archived-notification-response.dto.ts:18-23
Timestamp: 2024-11-28T12:49:15.719Z
Learning: In the TypeScript classes `ArchivedNotificationResponse` and `NotificationResponse` within our NestJS application, we do not include input validation in constructors for response DTOs. It's acceptable to proceed without input validation to maintain consistency.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#443
File: apps/api/src/common/guards/api-key/api-key.guard.ts:161-175
Timestamp: 2025-07-13T14:37:39.774Z
Learning: The `getByProviderChainName` method in `ProviderChainsService` already filters by `status: Status.ACTIVE` in its TypeORM query, making additional status checks in calling code redundant.
apps/api/src/modules/provider-chain-members/provider-chain-members.module.ts (3)
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#443
File: apps/api/src/common/guards/api-key/api-key.guard.ts:161-175
Timestamp: 2025-07-13T14:37:39.774Z
Learning: The `getByProviderChainName` method in `ProviderChainsService` already filters by `status: Status.ACTIVE` in its TypeORM query, making additional status checks in calling code redundant.
Learnt from: Harish-osmosys
PR: OsmosysSoftware/osmo-x#278
File: apps/api/src/modules/webhook/webhook.service.ts:0-0
Timestamp: 2024-10-08T17:27:45.632Z
Learning: The `WebhookService` class in `apps/api/src/modules/webhook/webhook.service.ts` now uses the `Logger` from `@nestjs/common` for logging instead of `console.error`.
Learnt from: Harish-osmosys
PR: OsmosysSoftware/osmo-x#278
File: apps/api/src/modules/webhook/webhook.service.ts:0-0
Timestamp: 2024-07-12T09:48:15.458Z
Learning: The `WebhookService` class in `apps/api/src/modules/webhook/webhook.service.ts` now uses the `Logger` from `@nestjs/common` for logging instead of `console.error`.
apps/api/src/modules/provider-chains/provider-chains.service.ts (3)
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#443
File: apps/api/src/common/guards/api-key/api-key.guard.ts:161-175
Timestamp: 2025-07-13T14:37:39.774Z
Learning: The `getByProviderChainName` method in `ProviderChainsService` already filters by `status: Status.ACTIVE` in its TypeORM query, making additional status checks in calling code redundant.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#443
File: apps/api/src/common/guards/api-key/api-key.guard.ts:161-175
Timestamp: 2025-07-13T14:37:39.774Z
Learning: The `getByProviderChainName` method in `ProviderChainsService` already filters for active status at the service layer, making additional status checks in calling code redundant. This follows the pattern where service methods handle status validation internally.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#443
File: apps/api/src/common/guards/api-key/api-key.guard.ts:161-175
Timestamp: 2025-07-13T14:37:39.774Z
Learning: The `getByProviderChainName` method in `ProviderChainsService` already filters for active status, so additional status checks in calling code are unnecessary.
apps/api/src/modules/provider-chain-members/entities/provider-chain-member.entity.ts (5)
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#443
File: apps/api/src/common/guards/api-key/api-key.guard.ts:161-175
Timestamp: 2025-07-13T14:37:39.774Z
Learning: The `getByProviderChainName` method in `ProviderChainsService` already filters by `status: Status.ACTIVE` in its TypeORM query, making additional status checks in calling code redundant.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#400
File: apps/api/src/modules/applications/entities/application.entity.ts:42-46
Timestamp: 2025-03-07T06:13:20.929Z
Learning: In the OsmoX codebase, JSON data fields in entity classes are often typed as `string` even when they store structured data, with TypeORM handling the conversion between string representation and JSON database type. This pattern is used for consistency across the codebase, particularly for provider configurations.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#360
File: apps/api/src/modules/archived-notifications/entities/archived-notification.entity.ts:32-35
Timestamp: 2024-11-20T05:58:43.756Z
Learning: In the codebase, the `Notification` and `ArchivedNotification` entities use `default: null` in the `@Column` decorators for nullable fields, and `@Field()` without `{ nullable: true }`. This pattern is accepted in this project.
Learnt from: LakshayaT
PR: OsmosysSoftware/osmo-x#218
File: apps/api/src/database/migrations/1692870736645-migration.ts:0-0
Timestamp: 2024-10-08T17:27:45.632Z
Learning: The foreign key for `channel_type` in `notify_notifications` references `master_id` from `notify_master_providers` by design, to keep naming consistent with the table name.
Learnt from: LakshayaT
PR: OsmosysSoftware/osmo-x#218
File: apps/api/src/database/migrations/1692870736645-migration.ts:0-0
Timestamp: 2024-06-10T19:12:20.809Z
Learning: The foreign key for `channel_type` in `notify_notifications` references `master_id` from `notify_master_providers` by design, to keep naming consistent with the table name.
apps/api/src/database/migrations/1752904253632-FallbackProviderChanges.ts (8)
Learnt from: LakshayaT
PR: OsmosysSoftware/osmo-x#218
File: apps/api/src/database/migrations/1692870736645-migration.ts:0-0
Timestamp: 2024-06-10T19:12:20.809Z
Learning: The foreign key for `channel_type` in `notify_notifications` references `master_id` from `notify_master_providers` by design, to keep naming consistent with the table name.
Learnt from: LakshayaT
PR: OsmosysSoftware/osmo-x#218
File: apps/api/src/database/migrations/1692870736645-migration.ts:0-0
Timestamp: 2024-10-08T17:27:45.632Z
Learning: The foreign key for `channel_type` in `notify_notifications` references `master_id` from `notify_master_providers` by design, to keep naming consistent with the table name.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#408
File: apps/api/src/database/migrations/1743145236864-AddNotificationSentOnColumn.ts:0-0
Timestamp: 2025-04-02T04:39:34.338Z
Learning: In the 'AddNotificationSentOnColumn1743145236864' migration file, the 'notification_sent_on' column is intentionally defined as nullable (isNullable: true) for both the 'notify_archived_notifications' and 'notify_notifications' tables.
Learnt from: LakshayaT
PR: OsmosysSoftware/osmo-x#217
File: apps/api/src/database/migrations/1692870736646-seeddata.ts:0-0
Timestamp: 2024-06-10T19:12:20.809Z
Learning: The SQL queries in the migration file `1692870736646-seeddata.ts` have been updated to use parameterized queries as of commit `01a1c05b4edf08fbdca84ccb9b6d8a9af67badc9` in PR #217.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#408
File: apps/api/src/database/migrations/1743145236864-AddNotificationSentOnColumn.ts:0-0
Timestamp: 2025-04-02T04:39:34.338Z
Learning: In the migration file for adding the 'notification_sent_on' column to 'notify_archived_notifications' and 'notify_notifications' tables, the column is defined as nullable.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#360
File: apps/api/src/modules/notifications/notifications.service.ts:0-0
Timestamp: 2024-11-20T06:08:32.855Z
Learning: The `findNotificationsToArchive` function has been removed from `notifications.service.ts`, and archiving tasks now use `QueryRunner` directly in `archived-notifications.service.ts`.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#304
File: apps/api/src/database/migrations/1692870736645-migration.ts:0-0
Timestamp: 2024-08-06T09:08:42.664Z
Learning: In the `down` method of migration classes, ensure to add null checks when retrieving foreign keys using `queryRunner.getTable()`.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#304
File: apps/api/src/database/migrations/1692870736645-migration.ts:0-0
Timestamp: 2024-10-08T17:27:45.632Z
Learning: In the `down` method of migration classes, ensure to add null checks when retrieving foreign keys using `queryRunner.getTable()`.
apps/api/src/modules/provider-chains/provider-chains.module.ts (3)
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#443
File: apps/api/src/common/guards/api-key/api-key.guard.ts:161-175
Timestamp: 2025-07-13T14:37:39.774Z
Learning: The `getByProviderChainName` method in `ProviderChainsService` already filters by `status: Status.ACTIVE` in its TypeORM query, making additional status checks in calling code redundant.
Learnt from: Harish-osmosys
PR: OsmosysSoftware/osmo-x#278
File: apps/api/src/modules/webhook/webhook.service.ts:0-0
Timestamp: 2024-10-08T17:27:45.632Z
Learning: The `WebhookService` class in `apps/api/src/modules/webhook/webhook.service.ts` now uses the `Logger` from `@nestjs/common` for logging instead of `console.error`.
Learnt from: Harish-osmosys
PR: OsmosysSoftware/osmo-x#278
File: apps/api/src/modules/webhook/webhook.service.ts:0-0
Timestamp: 2024-07-12T09:48:15.458Z
Learning: The `WebhookService` class in `apps/api/src/modules/webhook/webhook.service.ts` now uses the `Logger` from `@nestjs/common` for logging instead of `console.error`.
apps/api/src/modules/notifications/entities/notification.entity.ts (11)
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#360
File: apps/api/src/modules/archived-notifications/entities/archived-notification.entity.ts:32-35
Timestamp: 2024-11-20T05:58:43.756Z
Learning: In the codebase, the `Notification` and `ArchivedNotification` entities use `default: null` in the `@Column` decorators for nullable fields, and `@Field()` without `{ nullable: true }`. This pattern is accepted in this project.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#368
File: apps/portal/src/app/views/notifications/notification.model.ts:21-32
Timestamp: 2024-11-28T12:52:01.779Z
Learning: In the TypeScript file `apps/portal/src/app/views/notifications/notification.model.ts`, the `ArchivedNotification` interface intentionally has a different structure from the `Notification` interface, specifically using `notificationId` instead of `id`.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#368
File: apps/portal/src/app/graphql/graphql.queries.ts:50-50
Timestamp: 2024-11-28T12:52:15.785Z
Learning: In `apps/portal/src/app/graphql/graphql.queries.ts`, the `GetArchivedNotifications` query uses the field `notificationId` intentionally instead of `id` because `archivedNotifications` has a different structure than `notifications`.
Learnt from: LakshayaT
PR: OsmosysSoftware/osmo-x#218
File: apps/api/src/database/migrations/1692870736645-migration.ts:0-0
Timestamp: 2024-06-10T19:12:20.809Z
Learning: The foreign key for `channel_type` in `notify_notifications` references `master_id` from `notify_master_providers` by design, to keep naming consistent with the table name.
Learnt from: LakshayaT
PR: OsmosysSoftware/osmo-x#218
File: apps/api/src/database/migrations/1692870736645-migration.ts:0-0
Timestamp: 2024-10-08T17:27:45.632Z
Learning: The foreign key for `channel_type` in `notify_notifications` references `master_id` from `notify_master_providers` by design, to keep naming consistent with the table name.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#408
File: apps/api/src/database/migrations/1743145236864-AddNotificationSentOnColumn.ts:0-0
Timestamp: 2025-04-02T04:39:38.272Z
Learning: In the Osmo-X project, the `notification_sent_on` column in both `notify_archived_notifications` and `notify_notifications` tables is designed to be nullable, as explicitly defined in the migration file with `isNullable: true`.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#360
File: apps/api/src/modules/notifications/notifications.service.ts:0-0
Timestamp: 2024-11-20T06:08:32.855Z
Learning: The `findNotificationsToArchive` function has been removed from `notifications.service.ts`, and archiving tasks now use `QueryRunner` directly in `archived-notifications.service.ts`.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#408
File: apps/api/src/database/migrations/1743145236864-AddNotificationSentOnColumn.ts:0-0
Timestamp: 2025-04-02T04:39:34.338Z
Learning: In the 'AddNotificationSentOnColumn1743145236864' migration file, the 'notification_sent_on' column is intentionally defined as nullable (isNullable: true) for both the 'notify_archived_notifications' and 'notify_notifications' tables.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#408
File: apps/api/src/database/migrations/1743145236864-AddNotificationSentOnColumn.ts:0-0
Timestamp: 2025-04-02T04:39:34.338Z
Learning: In the migration file for adding the 'notification_sent_on' column to 'notify_archived_notifications' and 'notify_notifications' tables, the column is defined as nullable.
Learnt from: osm-vishnukyatannawar
PR: OsmosysSoftware/osmo-x#354
File: apps/api/src/database/migrations/1730724383210-ArchiveCompletedNotifications.ts:25-27
Timestamp: 2024-11-08T05:23:26.338Z
Learning: In the 'archived_notifications' table, we do not add a foreign key constraint on 'notification_id' to 'notify_notifications' because the original notifications may have been deleted, and we want to retain archived notifications even if the originals are removed.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#443
File: apps/api/src/common/guards/api-key/api-key.guard.ts:161-175
Timestamp: 2025-07-13T14:37:39.774Z
Learning: The `getByProviderChainName` method in `ProviderChainsService` already filters by `status: Status.ACTIVE` in its TypeORM query, making additional status checks in calling code redundant.
apps/api/src/modules/providers/entities/provider.entity.ts (9)
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#443
File: apps/api/src/common/guards/api-key/api-key.guard.ts:161-175
Timestamp: 2025-07-13T14:37:39.774Z
Learning: The `getByProviderChainName` method in `ProviderChainsService` already filters by `status: Status.ACTIVE` in its TypeORM query, making additional status checks in calling code redundant.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#400
File: apps/api/src/modules/applications/entities/application.entity.ts:42-46
Timestamp: 2025-03-07T06:13:20.929Z
Learning: In the OsmoX codebase, JSON data fields in entity classes are often typed as `string` even when they store structured data, with TypeORM handling the conversion between string representation and JSON database type. This pattern is used for consistency across the codebase, particularly for provider configurations.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#368
File: apps/portal/src/app/views/notifications/notification.model.ts:21-32
Timestamp: 2024-11-28T12:52:01.779Z
Learning: In the TypeScript file `apps/portal/src/app/views/notifications/notification.model.ts`, the `ArchivedNotification` interface intentionally has a different structure from the `Notification` interface, specifically using `notificationId` instead of `id`.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#360
File: apps/api/src/modules/archived-notifications/entities/archived-notification.entity.ts:32-35
Timestamp: 2024-11-20T05:58:43.756Z
Learning: In the codebase, the `Notification` and `ArchivedNotification` entities use `default: null` in the `@Column` decorators for nullable fields, and `@Field()` without `{ nullable: true }`. This pattern is accepted in this project.
Learnt from: LakshayaT
PR: OsmosysSoftware/osmo-x#218
File: apps/api/src/database/migrations/1692870736645-migration.ts:0-0
Timestamp: 2024-10-08T17:27:45.632Z
Learning: The foreign key for `channel_type` in `notify_notifications` references `master_id` from `notify_master_providers` by design, to keep naming consistent with the table name.
Learnt from: LakshayaT
PR: OsmosysSoftware/osmo-x#218
File: apps/api/src/database/migrations/1692870736645-migration.ts:0-0
Timestamp: 2024-06-10T19:12:20.809Z
Learning: The foreign key for `channel_type` in `notify_notifications` references `master_id` from `notify_master_providers` by design, to keep naming consistent with the table name.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#360
File: apps/api/src/modules/notifications/notifications.service.ts:0-0
Timestamp: 2024-11-20T06:08:32.855Z
Learning: The `findNotificationsToArchive` function has been removed from `notifications.service.ts`, and archiving tasks now use `QueryRunner` directly in `archived-notifications.service.ts`.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#367
File: apps/api/src/modules/archived-notifications/dtos/archived-notification-response.dto.ts:18-23
Timestamp: 2024-11-28T12:49:15.719Z
Learning: In the TypeScript classes `ArchivedNotificationResponse` and `NotificationResponse` within our NestJS application, we do not include input validation in constructors for response DTOs. It's acceptable to proceed without input validation to maintain consistency.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#368
File: apps/portal/src/app/graphql/graphql.queries.ts:50-50
Timestamp: 2024-11-28T12:52:15.785Z
Learning: In `apps/portal/src/app/graphql/graphql.queries.ts`, the `GetArchivedNotifications` query uses the field `notificationId` intentionally instead of `id` because `archivedNotifications` has a different structure than `notifications`.
apps/api/src/modules/provider-chains/entities/provider-chain.entity.ts (10)
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#443
File: apps/api/src/common/guards/api-key/api-key.guard.ts:161-175
Timestamp: 2025-07-13T14:37:39.774Z
Learning: The `getByProviderChainName` method in `ProviderChainsService` already filters by `status: Status.ACTIVE` in its TypeORM query, making additional status checks in calling code redundant.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#400
File: apps/api/src/modules/applications/entities/application.entity.ts:42-46
Timestamp: 2025-03-07T06:13:20.929Z
Learning: In the OsmoX codebase, JSON data fields in entity classes are often typed as `string` even when they store structured data, with TypeORM handling the conversion between string representation and JSON database type. This pattern is used for consistency across the codebase, particularly for provider configurations.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#360
File: apps/api/src/modules/archived-notifications/entities/archived-notification.entity.ts:32-35
Timestamp: 2024-11-20T05:58:43.756Z
Learning: In the codebase, the `Notification` and `ArchivedNotification` entities use `default: null` in the `@Column` decorators for nullable fields, and `@Field()` without `{ nullable: true }`. This pattern is accepted in this project.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#368
File: apps/portal/src/app/views/notifications/notification.model.ts:21-32
Timestamp: 2024-11-28T12:52:01.779Z
Learning: In the TypeScript file `apps/portal/src/app/views/notifications/notification.model.ts`, the `ArchivedNotification` interface intentionally has a different structure from the `Notification` interface, specifically using `notificationId` instead of `id`.
Learnt from: LakshayaT
PR: OsmosysSoftware/osmo-x#218
File: apps/api/src/database/migrations/1692870736645-migration.ts:0-0
Timestamp: 2024-06-10T19:12:20.809Z
Learning: The foreign key for `channel_type` in `notify_notifications` references `master_id` from `notify_master_providers` by design, to keep naming consistent with the table name.
Learnt from: LakshayaT
PR: OsmosysSoftware/osmo-x#218
File: apps/api/src/database/migrations/1692870736645-migration.ts:0-0
Timestamp: 2024-10-08T17:27:45.632Z
Learning: The foreign key for `channel_type` in `notify_notifications` references `master_id` from `notify_master_providers` by design, to keep naming consistent with the table name.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#360
File: apps/api/src/modules/notifications/notifications.service.ts:0-0
Timestamp: 2024-11-20T06:08:32.855Z
Learning: The `findNotificationsToArchive` function has been removed from `notifications.service.ts`, and archiving tasks now use `QueryRunner` directly in `archived-notifications.service.ts`.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#443
File: apps/api/src/common/guards/api-key/api-key.guard.ts:161-175
Timestamp: 2025-07-13T14:37:39.774Z
Learning: The `getByProviderChainName` method in `ProviderChainsService` already filters for active status at the service layer, making additional status checks in calling code redundant. This follows the pattern where service methods handle status validation internally.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#367
File: apps/api/src/modules/archived-notifications/dtos/archived-notification-response.dto.ts:18-23
Timestamp: 2024-11-28T12:49:15.719Z
Learning: In the TypeScript classes `ArchivedNotificationResponse` and `NotificationResponse` within our NestJS application, we do not include input validation in constructors for response DTOs. It's acceptable to proceed without input validation to maintain consistency.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#443
File: apps/api/src/common/guards/api-key/api-key.guard.ts:161-175
Timestamp: 2025-07-13T14:37:39.774Z
Learning: The `getByProviderChainName` method in `ProviderChainsService` already filters for active status, so additional status checks in calling code are unnecessary.
🧬 Code Graph Analysis (4)
apps/api/src/modules/provider-chain-members/provider-chain-members.module.ts (2)
apps/api/src/modules/provider-chains/provider-chains.module.ts (1)
Module(6-11)apps/api/src/modules/notifications/notifications.module.ts (1)
Module(87-132)
apps/api/src/modules/provider-chains/provider-chains.service.ts (1)
apps/api/src/common/constants/database.ts (1)
Status(1-4)
apps/api/src/modules/provider-chain-members/entities/provider-chain-member.entity.ts (3)
apps/api/src/modules/provider-chains/entities/provider-chain.entity.ts (1)
Entity(19-101)apps/api/src/modules/providers/entities/provider.entity.ts (1)
Entity(18-94)apps/api/src/common/constants/database.ts (1)
Status(1-4)
apps/api/src/modules/provider-chains/provider-chains.module.ts (2)
apps/api/src/modules/provider-chain-members/provider-chain-members.module.ts (1)
Module(6-11)apps/api/src/modules/notifications/notifications.module.ts (1)
Module(87-132)
🔇 Additional comments (8)
apps/api/src/modules/provider-chains/provider-chains.module.ts (1)
1-11: LGTM! Standard NestJS module implementation.The module follows the established NestJS patterns correctly, with proper TypeORM integration, service provision, and exports for dependency injection in other modules.
apps/api/src/modules/provider-chains/provider-chains.service.ts (1)
1-24: LGTM! Well-implemented service with proper error handling.The service correctly implements dependency injection, includes appropriate null checks, and filters for active status. The implementation follows established patterns in the codebase.
apps/api/src/modules/applications/entities/application.entity.ts (1)
80-82: LGTM! Properly implemented entity relationship.The one-to-many relationship to ProviderChain is correctly implemented with proper TypeORM and GraphQL decorators, following the established patterns in the entity.
apps/api/src/modules/provider-chain-members/provider-chain-members.module.ts (1)
1-11: LGTM! Consistent module implementation.The module follows the exact same pattern as
ProviderChainsModule, maintaining consistency across the provider chain feature modules. The implementation correctly uses NestJS and TypeORM patterns.apps/api/src/modules/notifications/entities/notification.entity.ts (1)
19-19: LGTM! Provider chain integration follows established patterns.The addition of
providerChainIdandproviderChainDetailsis well-implemented:
- Nullable field pattern matches the codebase convention
- ManyToOne relationship correctly references
chainIdproperty- GraphQL field decorators properly mark the relation as nullable
Also applies to: 96-118
apps/api/src/modules/archived-notifications/entities/archived-notification.entity.ts (1)
17-17: LGTM! Consistent implementation with Notification entity.The provider chain integration in
ArchivedNotificationcorrectly mirrors the implementation inNotificationentity, maintaining consistency between the two entities as per the established pattern.Also applies to: 98-119
apps/api/src/modules/provider-chains/entities/provider-chain.entity.ts (1)
81-85: Verify ifproviderChainMembersshould be exposed as a GraphQL field.The
providerChainMembersrelationship is not decorated with@Field()unlike the other OneToMany relationships (notificationsandarchivedNotifications). This might prevent querying chain members through GraphQL.If this is intentional for security or design reasons, please confirm. Otherwise, consider adding:
@OneToMany( () => ProviderChainMember, (providerChainMember) => providerChainMember.providerChainDetails, ) + @Field(() => [ProviderChainMember], { nullable: true }) providerChainMembers: ProviderChainMember[];apps/api/src/database/migrations/1752904253632-FallbackProviderChanges.ts (1)
313-354: Well-structured migration with proper rollback implementation.The migration is comprehensive and follows best practices:
- Tables are dropped in reverse order to respect dependencies
- All constraints, indexes, and foreign keys are properly removed
- The organization makes it easy to understand and maintain
apps/api/src/database/migrations/1752904253632-FallbackProviderChanges.ts
Outdated
Show resolved
Hide resolved
apps/api/src/modules/provider-chain-members/entities/provider-chain-member.entity.ts
Show resolved
Hide resolved
apps/api/src/modules/provider-chain-members/entities/provider-chain-member.entity.ts
Outdated
Show resolved
Hide resolved
apps/api/src/modules/provider-chain-members/provider-chain-members.service.ts
Outdated
Show resolved
Hide resolved
kshitij-k-osmosys
changed the title
kshitij-k-osmosys
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/api/src/database/migrations/1752904253632-FallbackProviderChanges.ts (1)
134-177: Foreign key constraints are well-designed with one consideration.The foreign key constraints are logically structured and the RESTRICT behavior on provider_type correctly prevents data integrity issues. However, consider whether
ON DELETE CASCADEfor the provider_id foreign key (line 174) is the desired behavior - you may want to preserve chain member records and mark them inactive instead when a provider is deleted.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/api/src/database/migrations/1752904253632-FallbackProviderChanges.ts(1 hunks)apps/api/src/modules/provider-chain-members/entities/provider-chain-member.entity.ts(1 hunks)apps/api/src/modules/provider-chains/entities/provider-chain.entity.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/api/src/modules/provider-chain-members/entities/provider-chain-member.entity.ts
- apps/api/src/modules/provider-chains/entities/provider-chain.entity.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#360
File: apps/api/src/modules/notifications/notifications.service.ts:0-0
Timestamp: 2024-11-20T06:08:32.855Z
Learning: The `findNotificationsToArchive` function has been removed from `notifications.service.ts`, and archiving tasks now use `QueryRunner` directly in `archived-notifications.service.ts`.
Learnt from: LakshayaT
PR: OsmosysSoftware/osmo-x#218
File: apps/api/src/database/migrations/1692870736645-migration.ts:0-0
Timestamp: 2024-06-10T19:12:20.809Z
Learning: The foreign key for `channel_type` in `notify_notifications` references `master_id` from `notify_master_providers` by design, to keep naming consistent with the table name.
Learnt from: LakshayaT
PR: OsmosysSoftware/osmo-x#218
File: apps/api/src/database/migrations/1692870736645-migration.ts:0-0
Timestamp: 2024-10-08T17:27:45.632Z
Learning: The foreign key for `channel_type` in `notify_notifications` references `master_id` from `notify_master_providers` by design, to keep naming consistent with the table name.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#360
File: apps/api/src/modules/archived-notifications/entities/archived-notification.entity.ts:32-35
Timestamp: 2024-11-20T05:58:43.756Z
Learning: In the codebase, the `Notification` and `ArchivedNotification` entities use `default: null` in the `@Column` decorators for nullable fields, and `@Field()` without `{ nullable: true }`. This pattern is accepted in this project.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#443
File: apps/api/src/common/guards/api-key/api-key.guard.ts:161-175
Timestamp: 2025-07-13T14:37:39.774Z
Learning: The `getByProviderChainName` method in `ProviderChainsService` already filters by `status: Status.ACTIVE` in its TypeORM query, making additional status checks in calling code redundant.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#408
File: apps/api/src/database/migrations/1743145236864-AddNotificationSentOnColumn.ts:0-0
Timestamp: 2025-04-02T04:39:38.272Z
Learning: In the Osmo-X project, the `notification_sent_on` column in both `notify_archived_notifications` and `notify_notifications` tables is designed to be nullable, as explicitly defined in the migration file with `isNullable: true`.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#368
File: apps/portal/src/app/views/notifications/notification.model.ts:21-32
Timestamp: 2024-11-28T12:52:01.779Z
Learning: In the TypeScript file `apps/portal/src/app/views/notifications/notification.model.ts`, the `ArchivedNotification` interface intentionally has a different structure from the `Notification` interface, specifically using `notificationId` instead of `id`.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#408
File: apps/api/src/database/migrations/1743145236864-AddNotificationSentOnColumn.ts:0-0
Timestamp: 2025-04-02T04:39:34.338Z
Learning: In the 'AddNotificationSentOnColumn1743145236864' migration file, the 'notification_sent_on' column is intentionally defined as nullable (isNullable: true) for both the 'notify_archived_notifications' and 'notify_notifications' tables.
Learnt from: osm-vishnukyatannawar
PR: OsmosysSoftware/osmo-x#354
File: apps/api/src/database/migrations/1730724383210-ArchiveCompletedNotifications.ts:25-27
Timestamp: 2024-11-08T05:23:26.338Z
Learning: In the 'archived_notifications' table, we do not add a foreign key constraint on 'notification_id' to 'notify_notifications' because the original notifications may have been deleted, and we want to retain archived notifications even if the originals are removed.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#408
File: apps/api/src/database/migrations/1743145236864-AddNotificationSentOnColumn.ts:0-0
Timestamp: 2025-04-02T04:39:34.338Z
Learning: In the migration file for adding the 'notification_sent_on' column to 'notify_archived_notifications' and 'notify_notifications' tables, the column is defined as nullable.
apps/api/src/database/migrations/1752904253632-FallbackProviderChanges.ts (12)
Learnt from: LakshayaT
PR: OsmosysSoftware/osmo-x#218
File: apps/api/src/database/migrations/1692870736645-migration.ts:0-0
Timestamp: 2024-10-08T17:27:45.632Z
Learning: The foreign key for `channel_type` in `notify_notifications` references `master_id` from `notify_master_providers` by design, to keep naming consistent with the table name.
Learnt from: LakshayaT
PR: OsmosysSoftware/osmo-x#218
File: apps/api/src/database/migrations/1692870736645-migration.ts:0-0
Timestamp: 2024-06-10T19:12:20.809Z
Learning: The foreign key for `channel_type` in `notify_notifications` references `master_id` from `notify_master_providers` by design, to keep naming consistent with the table name.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#408
File: apps/api/src/database/migrations/1743145236864-AddNotificationSentOnColumn.ts:0-0
Timestamp: 2025-04-02T04:39:34.338Z
Learning: In the 'AddNotificationSentOnColumn1743145236864' migration file, the 'notification_sent_on' column is intentionally defined as nullable (isNullable: true) for both the 'notify_archived_notifications' and 'notify_notifications' tables.
Learnt from: LakshayaT
PR: OsmosysSoftware/osmo-x#217
File: apps/api/src/database/migrations/1692870736646-seeddata.ts:0-0
Timestamp: 2024-06-10T19:12:20.809Z
Learning: The SQL queries in the migration file `1692870736646-seeddata.ts` have been updated to use parameterized queries as of commit `01a1c05b4edf08fbdca84ccb9b6d8a9af67badc9` in PR #217.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#408
File: apps/api/src/database/migrations/1743145236864-AddNotificationSentOnColumn.ts:0-0
Timestamp: 2025-04-02T04:39:34.338Z
Learning: In the migration file for adding the 'notification_sent_on' column to 'notify_archived_notifications' and 'notify_notifications' tables, the column is defined as nullable.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#360
File: apps/api/src/modules/notifications/notifications.service.ts:0-0
Timestamp: 2024-11-20T06:08:32.855Z
Learning: The `findNotificationsToArchive` function has been removed from `notifications.service.ts`, and archiving tasks now use `QueryRunner` directly in `archived-notifications.service.ts`.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#408
File: apps/api/src/database/migrations/1743145236864-AddNotificationSentOnColumn.ts:0-0
Timestamp: 2025-04-02T04:39:38.272Z
Learning: In the Osmo-X project, the `notification_sent_on` column in both `notify_archived_notifications` and `notify_notifications` tables is designed to be nullable, as explicitly defined in the migration file with `isNullable: true`.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#304
File: apps/api/src/database/migrations/1692870736645-migration.ts:0-0
Timestamp: 2024-10-08T17:27:45.632Z
Learning: In the `down` method of migration classes, ensure to add null checks when retrieving foreign keys using `queryRunner.getTable()`.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#304
File: apps/api/src/database/migrations/1692870736645-migration.ts:0-0
Timestamp: 2024-08-06T09:08:42.664Z
Learning: In the `down` method of migration classes, ensure to add null checks when retrieving foreign keys using `queryRunner.getTable()`.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#443
File: apps/api/src/common/guards/api-key/api-key.guard.ts:161-175
Timestamp: 2025-07-13T14:37:39.774Z
Learning: The `getByProviderChainName` method in `ProviderChainsService` already filters by `status: Status.ACTIVE` in its TypeORM query, making additional status checks in calling code redundant.
Learnt from: osm-vishnukyatannawar
PR: OsmosysSoftware/osmo-x#354
File: apps/api/src/database/migrations/1730724383210-ArchiveCompletedNotifications.ts:25-27
Timestamp: 2024-11-08T05:23:26.338Z
Learning: In the 'archived_notifications' table, we do not add a foreign key constraint on 'notification_id' to 'notify_notifications' because the original notifications may have been deleted, and we want to retain archived notifications even if the originals are removed.
Learnt from: kshitij-k-osmosys
PR: OsmosysSoftware/osmo-x#360
File: apps/api/src/modules/archived-notifications/entities/archived-notification.entity.ts:32-35
Timestamp: 2024-11-20T05:58:43.756Z
Learning: In the codebase, the `Notification` and `ArchivedNotification` entities use `default: null` in the `@Column` decorators for nullable fields, and `@Field()` without `{ nullable: true }`. This pattern is accepted in this project.
🔇 Additional comments (5)
apps/api/src/database/migrations/1752904253632-FallbackProviderChanges.ts (5)
14-74: Well-structured table creation following codebase patterns.The
notify_provider_chainstable structure is correctly implemented with appropriate column types, constraints, and follows the established patterns in the codebase for timestamps and status fields.
77-131: Consistent table structure with appropriate fallback ordering support.The
notify_provider_chain_memberstable correctly implements the member structure with priority ordering capabilities, following the same established patterns as the chains table.
180-227: Comprehensive indexing strategy supporting key query patterns.The indexes are well-designed to support efficient queries for provider chain lookups, fallback ordering, and status filtering. The composite indexes particularly support the core use cases effectively.
230-252: Essential unique constraints enforcing critical business rules.The unique constraints properly enforce important business logic: preventing duplicate chain names per application and ensuring proper fallback ordering without conflicts. These are crucial for the fallback provider system to function correctly.
254-353: Proper table modifications with correct rollback implementation.The addition of
provider_chain_idcolumns follows established codebase patterns for nullable foreign keys. TheSET NULLdeletion behavior appropriately preserves notification records when provider chains are deleted. The down method correctly reverses all changes in proper dependency order.
osm-vishnukyatannawar
approved these changes
Jul 21, 2025
osm-vishnukyatannawar
deleted the
feat/fallback-provider-migration-and-base-code
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
API PR Checklist
Task Link
Osmosys Developers must include the Pinestem task link in the PR.
OSXT-349
Pre-requisites
.env.examplefile with the required values as applicable.PR Details
PR details have been updated as per the given format (see below)
feat: add admin login endpoint)Additional Information
ready for reviewshould be added if the PR is ready to be reviewed)Note: Reviewer should ensure that the checklist and description have been populated and followed correctly, and the PR should be merged only after resolving all conversations and verifying that CI checks pass.
Description:
Fallback Provider Feature: Add migration, module & entity files for providerChains and providerChainMembers
Related changes:
Screenshots:
Summary by CodeRabbit
New Features
Tests