refactor(alert): uses a single table for all alerts#1320
Conversation
WalkthroughThese changes refactor and unify the notifications alert system. The schema for alerts is consolidated into a single, extensible model supporting multiple alert types. Old raw and deployment balance alert modules, repositories, and controllers are removed. New schemas, repositories, and a REST controller are introduced for the unified alert model, with all code, tests, and migrations updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AlertController
participant AlertRepository
participant Database
Client->>AlertController: POST /alerts (create alert)
AlertController->>AlertRepository: create(input)
AlertRepository->>Database: INSERT INTO alerts ...
Database-->>AlertRepository: alert record
AlertRepository-->>AlertController: created alert
AlertController-->>Client: alert response
Client->>AlertController: GET /alerts/:id
AlertController->>AlertRepository: findOneById(id)
AlertRepository->>Database: SELECT * FROM alerts WHERE id=...
Database-->>AlertRepository: alert record or null
AlertRepository-->>AlertController: alert or undefined
AlertController-->>Client: alert response or 404
Client->>AlertController: PATCH /alerts/:id
AlertController->>AlertRepository: updateById(id, patch)
AlertRepository->>Database: UPDATE alerts SET ... WHERE id=...
Database-->>AlertRepository: updated alert or null
AlertRepository-->>AlertController: alert or undefined
AlertController-->>Client: alert response or 404
Client->>AlertController: DELETE /alerts/:id
AlertController->>AlertRepository: deleteOneById(id)
AlertRepository->>Database: DELETE FROM alerts WHERE id=...
Database-->>AlertRepository: deleted alert or null
AlertRepository-->>AlertController: alert or undefined
AlertController-->>Client: alert response or 404
Suggested reviewers
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (39)
💤 Files with no reviewable changes (13)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (21)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (29)
✨ 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 (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1320 +/- ##
==========================================
- Coverage 32.81% 32.78% -0.04%
==========================================
Files 758 757 -1
Lines 18890 18876 -14
Branches 3538 3537 -1
==========================================
- Hits 6199 6188 -11
+ Misses 11939 11936 -3
Partials 752 752
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
ygrishajev
force-pushed
the
feature/notifications
branch
from
5513bb6 to
bf4abd4
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
apps/notifications/src/interfaces/alert-events/handlers/chain-events/chain-events.handler.spec.ts (1)
19-27: 🛠️ Refactor suggestionUpdate variable names to match the new service type
The variable name
rawAlertsServiceis still being used in the test body, but it now refers to an instance ofChainMessageAlertService. For clarity and consistency, you should update all references to match the new service type.- const { controller, rawAlertsService, brokerService } = await setup(); + const { controller, chainMessageAlertService, brokerService } = await setup(); const mockEvent = generateMock(MsgCloseDeploymentDto.schema); const alertMessage = generateAlertMessage({}); - rawAlertsService.alertFor.mockImplementation((_, callback) => callback(alertMessage)); + chainMessageAlertService.alertFor.mockImplementation((_, callback) => callback(alertMessage)); await controller.processDeploymentClosed(mockEvent); - expect(rawAlertsService.alertFor).toHaveBeenCalledWith(mockEvent, expect.any(Function)); + expect(chainMessageAlertService.alertFor).toHaveBeenCalledWith(mockEvent, expect.any(Function));
🧹 Nitpick comments (14)
apps/notifications/src/main.ts (2)
6-6: Avoid shadowing the built-inmodule
Naming the constantmodulecan be misleading since Node.js exposes a globalmoduleobject. Consider a more descriptive name (e.g.interfaceNameorinterfaceKey) to improve readability and avoid potential confusion.
12-13: Improve missing-module detection
Relying onerror.message.includes("Cannot find module")is brittle across Node versions/locales. A more robust approach is to checkerror.code === "ERR_MODULE_NOT_FOUND"(or inspecterror.code) when detecting a missing dynamic import.apps/notifications/src/interfaces/rest/rest.module.ts (1)
8-8: Inconsistent directory and controller naming.The controller has been renamed to
AlertControllerbut is still located in thecontrollers/raw-alert/directory. Consider renaming the directory tocontrollers/alert/to maintain consistency with the new unified alert structure.apps/notifications/src/interfaces/alert-events/handlers/chain-events/chain-events.handler.ts (1)
13-13: Property name doesn't match its type.The property is still named
rawAlertsServicebut its type is nowChainMessageAlertService. For clarity and consistency, consider renaming the property to match its type.- private readonly rawAlertsService: ChainMessageAlertService, + private readonly chainMessageAlertService: ChainMessageAlertService,Don't forget to update all references to this property in the class methods.
packages/http-sdk/src/deployment/deployment-http.service.ts (1)
144-145: Consider using a structured logging approach instead of console.logWhile adding debug logging is helpful, using direct
console.logstatements is generally not recommended for production code. Consider using a proper logging framework or a centralized logging utility that supports:
- Log levels (debug, info, warn, error)
- Conditional logging based on environment
- Structured log format for easier parsing
This would make logs more consistent and manageable across the codebase.
- console.log("DEBUG owner", owner); - console.log("DEBUG dseq", dseq); + logger.debug('Finding deployment', { owner, dseq });apps/notifications/src/modules/alert/model-schemas/alert.schema.ts (1)
26-28: Consider an index onmin_block_height,type,status,enabledThese columns are used as filters in the repository (
lte(min_block_height, …),eq(type, …)etc.).
Adding a composite or individual indexes will keep alert polling snappy when the table grows.apps/notifications/src/modules/alert/repositories/alert/alert-json-fields.schema.ts (1)
38-41: Nit: export a reusableAlertTypeSchemaYou already need the union of alert types in multiple places (HTTP schema, repository, etc.).
Exporting a single constant prevents drift:-const chainMessageTypeSchema = z.literal("CHAIN_MESSAGE"); -const deploymentBalanceTypeSchema = z.literal("DEPLOYMENT_BALANCE"); -export const alertTypeSchema = z.union([chainMessageTypeSchema, deploymentBalanceTypeSchema]); +export const chainMessageTypeSchema = z.literal("CHAIN_MESSAGE"); +export const deploymentBalanceTypeSchema = z.literal("DEPLOYMENT_BALANCE"); +export const alertTypeSchema = z.union([ + chainMessageTypeSchema, + deploymentBalanceTypeSchema +]);apps/notifications/src/modules/alert/repositories/alert/alert.repository.ts (1)
141-147:toOutputListdouble-injectstype; avoid data mutation
{ ...alert, type }overwrites the DB value and might mask data corruption (e.g., record stored with wrong type).
Safer:-return alerts.map(alert => this.toOutput({ ...alert, type })); +return alerts.map(alert => this.toOutput(alert as InternalAlertOutput & { type: T }));That way the method trusts the source of truth (DB) and preserves defensive checks inside
toOutput.apps/notifications/drizzle/0000_striped_vin_gonzales.sql (3)
4-4: Consider future extensibility of contact point types.The
contact_point_typeenum currently only has one value ('email'). While this works for the immediate implementation, consider whether additional contact methods (like SMS, push notifications, webhooks, etc.) might be needed in the future.
5-19: Good consolidation of alert tables with appropriate structure.The unified
alertstable properly captures the shared fields between different alert types while allowing type-specific data in theconditionsandparamsfields. This consolidation will simplify schema management and queries.Consider adding an index on
user_idfor performance if users will frequently query their own alerts:CREATE TABLE "alerts" ( "id" uuid PRIMARY KEY DEFAULT uuid_generate_v4() NOT NULL, "user_id" uuid NOT NULL, "contact_point_id" uuid NOT NULL, "summary" text NOT NULL, "description" text NOT NULL, "conditions" jsonb NOT NULL, "enabled" boolean DEFAULT true NOT NULL, "type" "alert_type" NOT NULL, "status" "alert_status" DEFAULT 'NORMAL' NOT NULL, "params" jsonb, "min_block_height" integer DEFAULT 0 NOT NULL, "created_at" timestamp DEFAULT now() NOT NULL, "updated_at" timestamp DEFAULT now() NOT NULL ); +CREATE INDEX alerts_user_id_idx ON "alerts" ("user_id");
38-38: Consider ON DELETE CASCADE behavior for contact_point_id.The current foreign key relationship uses
ON DELETE no action, which prevents deletion of contact points that have alerts. If the intent is to delete alerts when their contact point is deleted, consider usingON DELETE CASCADEinstead.-ALTER TABLE "alerts" ADD CONSTRAINT "alerts_contact_point_id_contact_points_id_fk" FOREIGN KEY ("contact_point_id") REFERENCES "public"."contact_points"("id") ON DELETE no action ON UPDATE no action; +ALTER TABLE "alerts" ADD CONSTRAINT "alerts_contact_point_id_contact_points_id_fk" FOREIGN KEY ("contact_point_id") REFERENCES "public"."contact_points"("id") ON DELETE CASCADE ON UPDATE no action;apps/notifications/src/interfaces/rest/http-schemas/alert.http-schema.ts (2)
35-39: Consider type-specific patch schemas for better validation.The current patch schema combines conditions from both alert types using a union. Consider creating separate patch schemas for each alert type to maintain type-specific validations and ensure consistency.
-export const alertPatchInputSchema = alertCreateCommonInputSchema - .extend({ - conditions: z.union([chainMessageConditionsSchema, deploymentBalanceConditionsSchema]) - }) - .partial(); +export const chainMessagePatchInputSchema = chainMessageCreateInputSchema.partial(); +export const deploymentBalancePatchInputSchema = deploymentBalanceCreateInputSchema.partial(); +export const alertPatchInputSchema = z.discriminatedUnion("type", [ + chainMessagePatchInputSchema, + deploymentBalancePatchInputSchema +]);
44-44: Consider using enum for status instead of string.The
statusfield in the output schema is defined asz.string()but should match thealert_statusenum defined in the database schema. Consider using a more specific validation.- status: z.string(), + status: z.enum(['NORMAL', 'FIRING', 'FIRED']),apps/notifications/drizzle/meta/0000_snapshot.json (1)
99-112: Consider index on alerts.user_id for query performance.While the foreign key relationship is correctly set up, consider adding an index on the
user_idcolumn in the alerts table if users will frequently query their own alerts. This would improve query performance for those operations.
🛑 Comments failed to post (6)
apps/notifications/src/interfaces/alert-events/handlers/chain-events/chain-events.handler.spec.ts (1)
47-62: 🛠️ Refactor suggestion
Update the return object property name to match the variable name in tests
The setup function returns an object with a property named
rawAlertsServicethat contains theChainMessageAlertServicemock. For consistency with the return type annotation and the test usage, update the property name.async function setup(): Promise<{ controller: ChainEventsHandler; rawAlertsService: MockProxy<ChainMessageAlertService>; deploymentBalanceAlertsService: MockProxy<DeploymentBalanceAlertsService>; brokerService: MockProxy<BrokerService>; }> { const module: TestingModule = await Test.createTestingModule({ providers: [MockProvider(BrokerService), ChainEventsHandler, MockProvider(ChainMessageAlertService), MockProvider(DeploymentBalanceAlertsService)] }).compile(); return { controller: module.get<ChainEventsHandler>(ChainEventsHandler), - rawAlertsService: module.get<MockProxy<ChainMessageAlertService>>(ChainMessageAlertService), + rawAlertsService: module.get<MockProxy<ChainMessageAlertService>>(ChainMessageAlertService), // Either rename this to chainMessageAlertService... deploymentBalanceAlertsService: module.get<MockProxy<DeploymentBalanceAlertsService>>(DeploymentBalanceAlertsService), brokerService: module.get<MockProxy<BrokerService>>(BrokerService) }; }Alternatively, you could update the return type annotation instead:
async function setup(): Promise<{ controller: ChainEventsHandler; - rawAlertsService: MockProxy<ChainMessageAlertService>; + chainMessageAlertService: MockProxy<ChainMessageAlertService>; // ...or update the type annotation to match the existing property name deploymentBalanceAlertsService: MockProxy<DeploymentBalanceAlertsService>; brokerService: MockProxy<BrokerService>; }> {Committable suggestion skipped: line range outside the PR's diff.
apps/notifications/src/modules/alert/services/chain-message-alert/chain-message-alert.service.spec.ts (1)
35-35: 💡 Verification agent
🧩 Analysis chain
Type casting could be improved for better type safety
The
as anytype casting is used multiple times to work around type compatibility issues between the repository callback and the test data. Consider creating a proper type guard or interface alignment to avoid usingany.- options.callback(alerts as any); + options.callback(alerts as AlertOutput[]);Also applies to: 70-70, 97-97, 158-158, 213-213
🌐 Web query:
TypeScript type assertion best practices vs using 'any'💡 Result:
When working with TypeScript, choosing between type assertion and using the
anytype has important implications for code safety and maintainability. Here’s a comparison and best practices for each:Type Assertion
Type assertion is a way to tell the TypeScript compiler to treat a value as a certain type, without performing type checking or restructuring. It’s done using the
askeyword or angle-bracket syntax.Best Practices:
- Use sparingly and only when certain: Use type assertion when you are confident about the type of a value but TypeScript cannot infer it correctly, such as when interacting with third-party libraries or DOM elements.
- Avoid forcing incorrect types: Don’t use assertions to silence type errors by forcing incompatible types; this can lead to runtime errors.
- Prefer more specific types: Assert the most specific type possible rather than a broad type.
- Validate data externally: When data comes from outside TypeScript’s control (like JSON APIs), validate it first before asserting its type.
- Avoid
anyfallback: Using assertions is better than usinganyblindly, as it keeps type information more explicit.const inputElement = document.querySelector('input') as HTMLInputElement;Using
any
anydisables type checking for that variable, essentially opting out of TypeScript’s type system.Best Practices:
- Avoid using
anyif possible: Usinganydefeats the purpose of TypeScript, making your code less safe and more prone to bugs.- Use for gradual typing:
anycan be useful temporarily during migration from JavaScript to TypeScript or in prototypes.- Replace
anyASAP: Aim to narrowanydown to a more specific type as soon as you can.- Consider alternatives: Use
unknownif you want to accept any value but enforce type checks before use.- Use linters: Configure linters to warn on undesired use of
anyto maintain code quality.let data: any = fetchData();Summary
Aspect Type Assertion anyCompiler Checks Bypasses checks on asserted type No checks; disables safety Safety Safer if used carefully Unsafe, can hide bugs Code Documentation Provides explicit type info No type info Use Cases Interfacing with external APIs, DOM Temporary, gradual typing Best Practice Validate data, be specific Minimize use, prefer unknown
If you want, I can help you with example scenarios or guidelines for your specific project!
🏁 Script executed:
#!/bin/bash # Search for the AlertOutput type definition across the repo rg 'type AlertOutput|interface AlertOutput' -n # Inspect the test file for alerts declaration and usage of options.callback rg -n 'chain-message-alert.service.spec.ts' -C3 -F 'alerts' # Locate the callback signature in the service file to confirm its expected parameter type rg -n 'callback(' -C3 -F 'callback' apps/notifications/src/modules/alert/services/chain-message-alert/chain-message-alert.service.tsLength of output: 763
🏁 Script executed:
#!/bin/bash # Locate the spec file spec_file=$(fd chain-message-alert.service.spec.ts) # Show lines 1-100 of the spec to inspect alerts declaration and usage sed -n '1,100p' "$spec_file" # Locate the service file service_file=$(fd chain-message-alert.service.ts) # Show callback signature and surrounding context in the service file grep -R -n 'callback' -C3 "$service_file"Length of output: 5310
Remove unnecessary
anycasts in testsThe
alertsarray is already declared asAlertOutput[], so there’s no need to bypass type checking withas any. You can invoke the callback directly and preserve full type safety:- alertRepository.paginate.mockImplementation(async options => { - options.callback(alerts as any); - }); + alertRepository.paginate.mockImplementation(async options => { + options.callback(alerts); + });Please update all occurrences in
apps/notifications/src/modules/alert/services/chain-message-alert/chain-message-alert.service.spec.ts
at lines 35, 70, 97, 158, and 213.apps/notifications/src/modules/alert/model-schemas/alert.schema.ts (1)
15-18: 🛠️ Refactor suggestion
Add explicit FK & cascade on
userId/contactPointIdfor referential integrity
userIdis declared as a plainuuidwithout a FK;contactPointIdhas a FK but noonDelete('cascade').
If either a user / contact point is removed you may orphan alerts or run into FK-violations at runtime.- userId: uuid("user_id").notNull(), + userId: uuid("user_id") + .notNull() + .references(() => Users.id, { onDelete: "cascade" }), + - contactPointId: uuid("contact_point_id") - .notNull() - .references(() => ContactPoint.id), + contactPointId: uuid("contact_point_id") + .notNull() + .references(() => ContactPoint.id, { onDelete: "cascade" }),(Replace
Userswith the actual users table.)
This prevents dangling records and eliminates manual cleanup.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.userId: uuid("user_id") .notNull() .references(() => Users.id, { onDelete: "cascade" }), contactPointId: uuid("contact_point_id") .notNull() .references(() => ContactPoint.id, { onDelete: "cascade" }),apps/notifications/src/modules/alert/repositories/alert/alert-json-fields.schema.ts (1)
4-15:
⚠️ Potential issue
toCompoundis not truly recursive – nested AND/OR clauses fail validationInside the generated
"and" | "or"branch the array elements are restricted to the baseschema, so a payload like
{ operator: "and", value: [{ operator: "or", value:[…] }, { … }] }will be rejected.A classic trick is a lazy schema:
-const toCompound = <T extends { operator: any; value: any; field: any }>(schema: ZodObject<T>) => - z.union([ - z.object({ operator: z.literal("and"), value: z.array(schema).min(2) }), - z.object({ operator: z.literal("or"), value: z.array(schema).min(2) }), - schema - ]); +const toCompound = <T extends ZodRawShape>(base: ZodObject<T>) => { + const lazy: z.ZodTypeAny = z.lazy(() => + z.union([ + z.object({ operator: z.literal("and"), value: z.array(lazy).min(2) }), + z.object({ operator: z.literal("or"), value: z.array(lazy).min(2) }), + base + ]) + ); + return lazy as unknown as z.ZodType<T>; +};This permits arbitrarily deep logical trees while keeping type-safety.
apps/notifications/src/modules/alert/repositories/alert/alert.repository.ts (1)
120-124:
⚠️ Potential issue
paginatebypasses CASL checks – potential privilege escalationAll other CRUD methods apply
whereAccessibleBy, butpaginatebuilds its ownclausesand never invokes the ability filter.
A user could call the polling endpoint and retrieve alerts they are not authorised to see.- const where = lastId ? and(...clauses, gt(schema.Alert.id, lastId)) : and(...clauses); + const base = lastId ? and(...clauses, gt(schema.Alert.id, lastId)) : and(...clauses); + const where = this.whereAccessibleBy(base);Failing to do so undermines the RBAC layer.
apps/notifications/src/interfaces/rest/http-schemas/alert.http-schema.ts (1)
9-16: 💡 Verification agent
🧩 Analysis chain
Address the TODO comment about user authentication.
The code has a TODO comment about getting the userId from authentication instead of from the request body. This would improve security by ensuring users can only create alerts for themselves.
🏁 Script executed:
#!/bin/bash # Check if there's authentication middleware that could be used # Look for auth-related code rg -A 3 "auth|authentication|middleware" --type ts | grep -v node_modules # Check if the controller extracts user info from request rg -A 5 "controller.*alert" --type tsLength of output: 118897
Enforce authentication and source userId from auth context
The alert‐creation endpoint must not accept a
userIdin the request body—clients could create alerts on behalf of other users. RemoveuserIdfrom the input schema, apply your auth middleware, and pullcurrentUser.idin the controller.• Modify apps/notifications/src/interfaces/rest/http-schemas/alert.http-schema.ts:
export const alertCreateCommonInputSchema = z.object({ - userId: z.string().uuid(), contactPointId: z.string().uuid(), enabled: z.boolean().optional().default(true), summary: z.string().min(3), description: z.string().min(3) });• In the alert controller (e.g. src/interfaces/rest/http-controllers/alert.controller.ts), register and invoke your authentication middleware, then use
context.currentUser.id(or equivalent) instead ofbody.userId.
• Ensure the notifications service’s app entrypoint (e.g. src/app.ts) mounts the auth middleware before the alert routes.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export const alertCreateCommonInputSchema = z.object({ // TODO: receive user from the auth instead contactPointId: z.string().uuid(), enabled: z.boolean().optional().default(true), summary: z.string().min(3), description: z.string().min(3) });
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
apps/notifications/drizzle/meta/0000_snapshot.json (1)
120-180:⚠️ Potential issueRevisit the unique constraint on
contact_points.user_id.
Enforcinguser_idas unique prevents a user from having multiple contact points (e.g. email, SMS, webhook). If multi‐channel support is intended, change this to a composite unique key on(user_id, type).
🧹 Nitpick comments (1)
apps/notifications/drizzle/meta/0000_snapshot.json (1)
7-119: Optimize the newalertstable schema.
– Consider adding B-tree indexes onuser_idandcontact_point_idfor faster lookups by user or contact point.
– Evaluate altering the foreign‐key toON DELETE CASCADEif you want alerts cleaned up automatically when a contact point is removed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (40)
apps/notifications/drizzle.config.ts(1 hunks)apps/notifications/drizzle/0000_cultured_kingpin.sql(0 hunks)apps/notifications/drizzle/0000_striped_vin_gonzales.sql(1 hunks)apps/notifications/drizzle/0001_foamy_amazoness.sql(0 hunks)apps/notifications/drizzle/0002_tiresome_power_man.sql(0 hunks)apps/notifications/drizzle/0003_regular_ezekiel.sql(0 hunks)apps/notifications/drizzle/meta/0000_snapshot.json(5 hunks)apps/notifications/drizzle/meta/0001_snapshot.json(0 hunks)apps/notifications/drizzle/meta/0002_snapshot.json(0 hunks)apps/notifications/drizzle/meta/0003_snapshot.json(0 hunks)apps/notifications/drizzle/meta/_journal.json(1 hunks)apps/notifications/src/interfaces/alert-events/handlers/chain-events/chain-events.handler.spec.ts(3 hunks)apps/notifications/src/interfaces/alert-events/handlers/chain-events/chain-events.handler.ts(2 hunks)apps/notifications/src/interfaces/rest/controllers/raw-alert/alert.controller.spec.ts(1 hunks)apps/notifications/src/interfaces/rest/controllers/raw-alert/alert.controller.ts(1 hunks)apps/notifications/src/interfaces/rest/controllers/raw-alert/raw-alert.controller.ts(0 hunks)apps/notifications/src/interfaces/rest/http-schemas/alert.http-schema.ts(1 hunks)apps/notifications/src/interfaces/rest/rest.module.ts(1 hunks)apps/notifications/src/main.ts(1 hunks)apps/notifications/src/modules/alert/alert.module.ts(1 hunks)apps/notifications/src/modules/alert/model-schemas/alert-base.schema.ts(0 hunks)apps/notifications/src/modules/alert/model-schemas/alert.schema.ts(1 hunks)apps/notifications/src/modules/alert/model-schemas/deployment-balance-alert.schema.ts(0 hunks)apps/notifications/src/modules/alert/model-schemas/index.ts(1 hunks)apps/notifications/src/modules/alert/model-schemas/raw-alert.schema.ts(0 hunks)apps/notifications/src/modules/alert/repositories/alert/alert-json-fields.schema.ts(1 hunks)apps/notifications/src/modules/alert/repositories/alert/alert.repository.ts(1 hunks)apps/notifications/src/modules/alert/repositories/deployment-balance-alert/deployment-balance-alert.repository.ts(0 hunks)apps/notifications/src/modules/alert/repositories/raw-alert/raw-alert.repository.ts(0 hunks)apps/notifications/src/modules/alert/services/chain-message-alert/chain-message-alert.service.spec.ts(12 hunks)apps/notifications/src/modules/alert/services/chain-message-alert/chain-message-alert.service.ts(2 hunks)apps/notifications/src/modules/alert/services/deployment-balance-alerts/deployment-balance-alerts.service.spec.ts(11 hunks)apps/notifications/src/modules/alert/services/deployment-balance-alerts/deployment-balance-alerts.service.ts(5 hunks)apps/notifications/test/functional/alert-crud.spec.ts(3 hunks)apps/notifications/test/functional/balance-alert.spec.ts(2 hunks)apps/notifications/test/functional/chain-message-alert.spec.ts(4 hunks)apps/notifications/test/seeders/akash-address.seeder.ts(1 hunks)apps/notifications/test/seeders/chain-message-alert.seeder.ts(2 hunks)apps/notifications/test/seeders/deployment-balance-alert.seeder.ts(3 hunks)packages/http-sdk/src/deployment/deployment-http.service.ts(1 hunks)
💤 Files with no reviewable changes (13)
- apps/notifications/src/modules/alert/model-schemas/raw-alert.schema.ts
- apps/notifications/drizzle/0001_foamy_amazoness.sql
- apps/notifications/drizzle/0002_tiresome_power_man.sql
- apps/notifications/src/modules/alert/model-schemas/deployment-balance-alert.schema.ts
- apps/notifications/src/modules/alert/model-schemas/alert-base.schema.ts
- apps/notifications/src/interfaces/rest/controllers/raw-alert/raw-alert.controller.ts
- apps/notifications/drizzle/0000_cultured_kingpin.sql
- apps/notifications/drizzle/0003_regular_ezekiel.sql
- apps/notifications/drizzle/meta/0002_snapshot.json
- apps/notifications/src/modules/alert/repositories/raw-alert/raw-alert.repository.ts
- apps/notifications/src/modules/alert/repositories/deployment-balance-alert/deployment-balance-alert.repository.ts
- apps/notifications/drizzle/meta/0003_snapshot.json
- apps/notifications/drizzle/meta/0001_snapshot.json
🚧 Files skipped from review as they are similar to previous changes (26)
- apps/notifications/drizzle.config.ts
- apps/notifications/drizzle/meta/_journal.json
- apps/notifications/test/functional/chain-message-alert.spec.ts
- apps/notifications/src/modules/alert/alert.module.ts
- apps/notifications/test/seeders/akash-address.seeder.ts
- apps/notifications/src/interfaces/rest/rest.module.ts
- apps/notifications/src/interfaces/alert-events/handlers/chain-events/chain-events.handler.ts
- apps/notifications/src/modules/alert/model-schemas/index.ts
- apps/notifications/src/interfaces/alert-events/handlers/chain-events/chain-events.handler.spec.ts
- apps/notifications/test/functional/balance-alert.spec.ts
- packages/http-sdk/src/deployment/deployment-http.service.ts
- apps/notifications/src/main.ts
- apps/notifications/test/seeders/chain-message-alert.seeder.ts
- apps/notifications/src/interfaces/rest/controllers/raw-alert/alert.controller.spec.ts
- apps/notifications/src/modules/alert/model-schemas/alert.schema.ts
- apps/notifications/src/modules/alert/services/deployment-balance-alerts/deployment-balance-alerts.service.ts
- apps/notifications/src/modules/alert/repositories/alert/alert.repository.ts
- apps/notifications/test/seeders/deployment-balance-alert.seeder.ts
- apps/notifications/src/modules/alert/services/deployment-balance-alerts/deployment-balance-alerts.service.spec.ts
- apps/notifications/test/functional/alert-crud.spec.ts
- apps/notifications/src/modules/alert/repositories/alert/alert-json-fields.schema.ts
- apps/notifications/src/modules/alert/services/chain-message-alert/chain-message-alert.service.ts
- apps/notifications/src/modules/alert/services/chain-message-alert/chain-message-alert.service.spec.ts
- apps/notifications/src/interfaces/rest/controllers/raw-alert/alert.controller.ts
- apps/notifications/drizzle/0000_striped_vin_gonzales.sql
- apps/notifications/src/interfaces/rest/http-schemas/alert.http-schema.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: validate-api
- GitHub Check: validate-notifications
- GitHub Check: test-deploy-web-build
🔇 Additional comments (3)
apps/notifications/drizzle/meta/0000_snapshot.json (3)
2-5: Verify migration snapshot version alignment.
Theversionis bumped to"7"and the snapshotidchanged—please confirm these values match your newest migration files and ordering to avoid drift in schema versioning.
181-220: Block_cursor table design looks solid.
Use of a single-row primary key (id = 'latest') plus timestampedlast_processed_blockeffectively tracks state. No changes needed here.
223-246: Validate enum coverage for alert and contact point types.
Make surepublic.alert_typeandpublic.alert_statusinclude all runtime values your code handles, and confirmpublic.contact_point_typecovers every contact channel (e.g. SMS, webhook) if those are supported.
ygrishajev
force-pushed
the
feature/notifications
branch
from
bf4abd4 to
ebf7e6e
Compare
2 participants
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores