feat: 🎸 persist webhook state with repository #282
Conversation
|
Warning Rate limit exceeded@polymath-eric has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 14 minutes and 57 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe recent updates enhance the codebase by aligning webhook parameter names with the updated specifications, enriching local storage repositories and their PostgreSQL counterparts with new entities and migration scripts, and refining test utilities for mock operations. Additionally, the README and decorator functionalities have been adjusted to reflect the new webhook terminology. These changes improve maintainability, data handling processes, and testing robustness across the application. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WebhookService
participant NotificationRepo
participant SubscriptionRepo
participant PostgresDB
User->>WebhookService: Trigger submitWithCallback
WebhookService->>NotificationRepo: Create Notification
WebhookService->>SubscriptionRepo: Retrieve Subscription Data
NotificationRepo->>PostgresDB: Save Notification
SubscriptionRepo->>PostgresDB: Fetch Subscription Data
PostgresDB->>WebhookService: Return Subscription Data
WebhookService->>User: Send Callback with Notification Status
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration 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: 4
Outside diff range and nitpick comments (11)
src/datastore/local-store/repos/subscription.repo.ts (1)
8-58: Local Repository Implementation: Correct but consider concurrency issues.The implementation of
LocalSubscriptionRepocorrectly adheres to theSubscriptionRepointerface. Methods are well-implemented with appropriate error handling. However, consider potential concurrency issues with the in-memory storage approach, especially in environments with multiple instances or heavy load.Consider implementing locking mechanisms or exploring other concurrency control techniques if this module is used in a production environment with high concurrency requirements.
README.md (10)
Line range hint
111-111: Typographical Correction: "block chain" should be "blockchain".The term "blockchain" is conventionally spelled as a single word.
- block chain + blockchainTools
LanguageTool
[uncategorized] ~229-~229: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...g. ### With docker To pass in the env variables you can use-eto pass them individua...
Line range hint
130-130: Grammar Correction: Use "a" instead of "an" before "transaction".The article "an" is used before words that start with a vowel sound, not just a vowel. Here, "transaction" starts with a consonant sound, so it should be "a transaction".
- an transaction + a transactionTools
LanguageTool
[uncategorized] ~229-~229: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...g. ### With docker To pass in the env variables you can use-eto pass them individua...
Line range hint
136-136: Grammar Correction: Add a comma after "given".When listing conditions or clauses, it's clear and grammatically correct to use a comma to separate them.
- If args for multiple are given the precedence order is Vault over Fireblocks over Local. + If args for multiple are given, the precedence order is Vault over Fireblocks over Local.Tools
LanguageTool
[uncategorized] ~229-~229: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...g. ### With docker To pass in the env variables you can use-eto pass them individua...
Line range hint
143-143: Verb Agreement Correction: Use "uses" instead of "use".The subject "Vault" is singular, so the verb should also be singular ("uses").
- To refer to a key when signing use the Vault name and version `${name}-${version}`. + To refer to a key when signing, uses the Vault name and version `${name}-${version}`.Tools
LanguageTool
[uncategorized] ~229-~229: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...g. ### With docker To pass in the env variables you can use-eto pass them individua...
Line range hint
148-148: Grammar and Punctuation Corrections: Change "correspond" to "corresponds" and add a comma.The verb should agree with the singular subject "This", and a comma is needed after "out".
- This correspond to `account`, `change` and `address_index` from the BIP-44 standard. If `change` and `address` portion are left out they will default to `0`. + This corresponds to `account`, `change`, and `address_index` from the BIP-44 standard. If `change` and `address` portions are left out, they will default to `0`.Tools
LanguageTool
[uncategorized] ~229-~229: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...g. ### With docker To pass in the env variables you can use-eto pass them individua...
Line range hint
165-165: Grammar and Punctuation Corrections: Replace "on" with "of" and add a comma after "Currently".The preposition "on" is incorrect in this context, and a comma should follow "Currently" when it starts a sentence.
- AMQP is a form on offline processing where the payload will be published on an AMQP topic, instead of being returned. Currently there are a set of "offline" modules, that setup listeners to the different queues. + AMQP is a form of offline processing where the payload will be published on an AMQP topic, instead of being returned. Currently, there are a set of "offline" modules that set up listeners to the different queues.Tools
LanguageTool
[uncategorized] ~229-~229: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...g. ### With docker To pass in the env variables you can use-eto pass them individua...
Line range hint
173-173: Typographical Correction: Change "i.e" to "i.e.".The abbreviation "i.e." should always be followed by a period.
- i.e postgres + i.e. postgresTools
LanguageTool
[uncategorized] ~229-~229: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...g. ### With docker To pass in the env variables you can use-eto pass them individua...
Line range hint
177-177: Grammar Correction: Change "project's" to "projects".The context suggests a plural form, not possessive.
- If using the project's compose file, an Artemis console will be exposed on `:8181` with `artemis` being both username and password. + If using the projects compose file, an Artemis console will be exposed on `:8181` with `artemis` being both username and password.Tools
LanguageTool
[uncategorized] ~229-~229: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...g. ### With docker To pass in the env variables you can use-eto pass them individua...
Line range hint
185-185: Grammar Correction: Use "an" instead of "a" before "endpoint".The word "endpoint" starts with a vowel sound, so "an" is the correct article to use.
- which will enabled a endpoint at `/developer-testing/webhook` + which will enable an endpoint at `/developer-testing/webhook`Tools
LanguageTool
[uncategorized] ~229-~229: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...g. ### With docker To pass in the env variables you can use-eto pass them individua...
Line range hint
217-217: Grammar Correction: Use "run" instead of "ran".The correct form of the verb in this context is "run".
- This allows the REST API to be ran as a single process which is convenient for development purposes + This allows the REST API to be run as a single process which is convenient for development purposesTools
LanguageTool
[uncategorized] ~229-~229: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...g. ### With docker To pass in the env variables you can use-eto pass them individua...
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (4)
src/datastore/local-store/repos/__snapshots__/notification.repo.spec.ts.snapis excluded by!**/*.snapsrc/datastore/local-store/repos/__snapshots__/subscription.repo.spec.ts.snapis excluded by!**/*.snapsrc/datastore/postgres/repos/__snapshots__/notification.repo.spec.ts.snapis excluded by!**/*.snapsrc/datastore/postgres/repos/__snapshots__/subscription.repo.spec.ts.snapis excluded by!**/*.snap
Files selected for processing (30)
- README.md (1 hunks)
- src/datastore/local-store/local-store.module.ts (2 hunks)
- src/datastore/local-store/repos/notification.repo.spec.ts (1 hunks)
- src/datastore/local-store/repos/notification.repo.ts (1 hunks)
- src/datastore/local-store/repos/subscription.repo.spec.ts (1 hunks)
- src/datastore/local-store/repos/subscription.repo.ts (1 hunks)
- src/datastore/postgres/entities/notification.entity.ts (1 hunks)
- src/datastore/postgres/entities/subscription.entity.ts (1 hunks)
- src/datastore/postgres/migrations/1718398114107-hooks.ts (1 hunks)
- src/datastore/postgres/migrations/1719003936380-notifications.ts (1 hunks)
- src/datastore/postgres/postgres.module.ts (2 hunks)
- src/datastore/postgres/repos/notification.repo.spec.ts (1 hunks)
- src/datastore/postgres/repos/notification.repo.ts (1 hunks)
- src/datastore/postgres/repos/subscription.repo.spec.ts (1 hunks)
- src/datastore/postgres/repos/subscription.repo.ts (1 hunks)
- src/notifications/model/notification.model.ts (2 hunks)
- src/notifications/notifications.module.ts (2 hunks)
- src/notifications/notifications.service.spec.ts (8 hunks)
- src/notifications/notifications.service.ts (3 hunks)
- src/notifications/repo/notification.repo.suite.ts (1 hunks)
- src/notifications/repo/notifications.repo.ts (1 hunks)
- src/notifications/types.ts (2 hunks)
- src/subscriptions/models/subscription.model.ts (2 hunks)
- src/subscriptions/repo/subscription.repo.suite.ts (1 hunks)
- src/subscriptions/repo/subscription.repo.ts (1 hunks)
- src/subscriptions/subscriptions.module.ts (1 hunks)
- src/subscriptions/subscriptions.service.spec.ts (10 hunks)
- src/subscriptions/subscriptions.service.ts (6 hunks)
- src/subscriptions/types.ts (2 hunks)
- src/test-utils/repo-mocks.ts (2 hunks)
Files not reviewed due to errors (2)
- src/datastore/postgres/repos/subscription.repo.spec.ts (no review received)
- src/subscriptions/repo/subscription.repo.suite.ts (no review received)
Additional context used
Path-based instructions (29)
src/datastore/local-store/repos/subscription.repo.spec.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/datastore/local-store/repos/notification.repo.spec.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/notifications/model/notification.model.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/subscriptions/types.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/subscriptions/subscriptions.module.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/notifications/repo/notifications.repo.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/datastore/postgres/migrations/1718398114107-hooks.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/notifications/types.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/notifications/notifications.module.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/datastore/postgres/migrations/1719003936380-notifications.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/subscriptions/repo/subscription.repo.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/subscriptions/models/subscription.model.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/test-utils/repo-mocks.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/datastore/postgres/entities/notification.entity.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/datastore/local-store/repos/notification.repo.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/datastore/postgres/entities/subscription.entity.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/datastore/postgres/repos/notification.repo.spec.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/notifications/repo/notification.repo.suite.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/datastore/local-store/repos/subscription.repo.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/datastore/local-store/local-store.module.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/datastore/postgres/repos/notification.repo.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/datastore/postgres/postgres.module.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/datastore/postgres/repos/subscription.repo.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/datastore/postgres/repos/subscription.repo.spec.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/subscriptions/repo/subscription.repo.suite.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/notifications/notifications.service.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/subscriptions/subscriptions.service.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/notifications/notifications.service.spec.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/subscriptions/subscriptions.service.spec.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.
Biome
src/datastore/postgres/repos/notification.repo.ts
[error] 15-15: Decorators are not valid here. (parse)
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabledoption totruein your configuration file.src/datastore/postgres/repos/subscription.repo.ts
[error] 14-14: Decorators are not valid here. (parse)
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabledoption totruein your configuration file.src/notifications/notifications.service.ts
[error] 26-26: Decorators are not valid here. (parse)
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabledoption totruein your configuration file.
[error] 29-29: Decorators are not valid here. (parse)
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabledoption totruein your configuration file.src/subscriptions/subscriptions.service.ts
[error] 24-24: Decorators are not valid here. (parse)
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabledoption totruein your configuration file.
GitHub Check: Linting
src/subscriptions/repo/subscription.repo.suite.ts
[failure] 34-34:
'_' is assigned a value but never used
Gitleaks
src/subscriptions/subscriptions.service.spec.ts
201-201: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key)
LanguageTool
README.md
[misspelling] ~111-~111: This word is normally spelled as one. (EN_COMPOUNDS_BLOCK_CHAIN)
Context: ...dpoints that submit transactions to the block chain (generally POST routes). Each of these ...
[misspelling] ~130-~130: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’. (EN_A_VS_AN)
Context: ...d to the chain. -AMQPThis creates an transaction to be processed by worker p...
[uncategorized] ~136-~136: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...gning manager. If args for multiple are given the precedence order is Vault over Fire...
[grammar] ~143-~143: Consider using third-person verb forms for singular and mass nouns. (MASS_AGREEMENT)
Context: ... it. To refer to a key when signing use the Vault name and version `${name}-${v...
[grammar] ~148-~148: The verb ‘correspond’ is plural. Did you mean: “corresponds”? Did you use a verb instead of a noun? (PLURAL_VERB_AFTER_THIS)
Context: ...s separated by-, as in1-0-0. This correspond toaccount,changeand `address_ind...
[uncategorized] ~148-~148: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...changeandaddressportion are left out they will default to0. Each combinat...
[uncategorized] ~165-~165: The preposition ‘of’ seems more likely in this position. (AI_HYDRA_LEO_REPLACE_ON_OF)
Context: ... full details ### AMQP AMQP is a form on offline processing where the payload wi...
[uncategorized] ~165-~165: A comma may be missing after the conjunctive/linking adverb ‘Currently’. (SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Context: ... AMQP topic, instead of being returned. Currently there are a set of "offline" modules, t...
[grammar] ~165-~165: There may be a verb agreement error, if referring to a singular entity (a set). (THERE_IS_ARE)
Context: ...tead of being returned. Currently there are a set of "offline" modules, that setup ...
[uncategorized] ~167-~167: The preposition ‘to’ seems more likely in this position. (AI_HYDRA_LEO_REPLACE_ON_TO)
Context: ...zed to an offline payload and published onRequests1. A signer process subscrib...
[typographical] ~168-~168: It appears that a comma is missing. (DURING_THAT_TIME_COMMA)
Context: ...cess subscribes toRequests. For each message it generates a signature, and publishes...
[typographical] ~171-~171: It seems that a comma is missing. (THOUGH_COMMA)
Context: ...ny AMQP 1.0 compliant broker should work though. If using AMQP, it is strongly recomme...
[uncategorized] ~173-~173: The abbreviation “i.e.” (= that is) requires two periods. (I_E)
Context: ...mmended to use a persistent data store (i.e postgres). There are two tables related...
[typographical] ~174-~174: Consider wrapping this idiom with two commas if you want to put a strong emphasize on it. (FOR_WHATEVER_REASON_COMMA)
Context: ...and to detect ones rejected by the chain for some reason -offline_eventis a table for the ...
[grammar] ~177-~177: An apostrophe ‘s’ denotes possession. Did you mean to use the plural form of the noun (no apostrophe)? (NOUN_APOSTROPHE_S_VERB)
Context: ..., serving as an audit log If using the project's compose file, an Artemis console will b...
[typographical] ~180-~180: Consider adding a comma after ‘Normally’ for more clarity. (RB_LY_COMMA)
Context: ...me and password. ### Webhooks (alpha) Normally the endpoints that create transactions ...
[uncategorized] ~183-~183: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ... Before sending any information to the endpoint the service will first make a request w...
[misspelling] ~185-~185: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’. (EN_A_VS_AN)
Context: ...EVELOPER_UTILS=truewhich will enabled a endpoint at/developer-testing/webhook...
[uncategorized] ~189-~189: A comma may be missing after the conjunctive/linking adverb ‘However’. (SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Context: ...and should not be used against mainnet. However the API should be stable to develop aga...
[style] ~191-~191: Unless you want to emphasize “not”, use “cannot” which is more common. (CAN_NOT_PREMIUM)
Context: ...ensure it won't miss events. As such it can not guarantee delivery of messages The pla...
[uncategorized] ~198-~198: A comma may be missing after the conjunctive/linking adverb ‘Currently’. (SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Context: ...configurable with multiple strategies. Currently there are two strategies available: 1....
[uncategorized] ~204-~204: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ... 1. Open: By configuringopenas a strategy any request will be authenticated with ...
[typographical] ~204-~204: The word “however” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence. (HOWEVER_SENTENCE)
Context: ...is is primarily intended for development, however it can be used to provide a "read only"...
[style] ~204-~204: Consider using “who” when you are referring to a person instead of an object. (THAT_WHO)
Context: ...d in combination with a signing manager that holds valuable keys. More strategies c...
[uncategorized] ~212-~212: When ‘API-specific’ is used as a modifier, it is usually spelled with a hyphen. (SPECIFIC_HYPHEN)
Context: ...T request. This only affects where REST API specific entities are stored (e.g. Users and Api...
[uncategorized] ~213-~213: A comma may be missing after the conjunctive/linking adverb ‘Currently’. (SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Context: ...chain regardless of the datastore used Currently there are two datastore available: 1. ...
[grammar] ~217-~217: Consider using either the past participle “run” or the present participle “running” here. (BEEN_PART_AGREEMENT)
Context: ...e state. This allows the REST API to be ran as a single process which is convenient...
[uncategorized] ~217-~217: A comma may be missing after the conjunctive/linking adverb ‘However’. (SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Context: ...urposes (i.e. nosignersare loaded). However all state will be lost when the process...
[uncategorized] ~219-~219: If this is a compound adjective that modifies the following noun, use a hyphen. (EN_COMPOUND_ADJECTIVE_INTERNAL)
Context: ...ts down 1. Postgres This is the more production ready approach. This allows state to be persi...
[typographical] ~219-~219: Consider adding a comma after ‘Internally’ for more clarity. (RB_LY_COMMA)
Context: ...instances to user the same information. Internally this uses TypeORM to manage the databas...
[uncategorized] ~223-~223: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...class describing the desired interface. Also write a test suite to specify the expec...
[uncategorized] ~223-~223: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...an implementation. Then in the concrete implementations define a new Repo that satisfies the te...
[uncategorized] ~229-~229: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...g. ### With docker To pass in the env variables you can use-eto pass them individua...
[uncategorized] ~230-~230: A comma is probably missing here. (MISSING_COMMA_AFTER_INTRODUCTORY_PHRASE)
Context: ...y, or use a file with--env-file. For documentation you will need to expose a port that map...
[uncategorized] ~242-~242: A comma may be missing after the conjunctive/linking adverb ‘Currently’. (SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Context: ...or the Artemis AMQP container to start Currently in "Settings" > "Features in developmen...
Markdownlint
README.md
126-126: Expected: 0; Actual: 2 (MD007, ul-indent)
Unordered list indentation
127-127: Expected: 0; Actual: 2 (MD007, ul-indent)
Unordered list indentation
128-128: Expected: 0; Actual: 2 (MD007, ul-indent)
Unordered list indentation
129-129: Expected: 0; Actual: 2 (MD007, ul-indent)
Unordered list indentation
130-130: Expected: 0; Actual: 2 (MD007, ul-indent)
Unordered list indentation
174-174: Expected: 0; Actual: 2 (MD007, ul-indent)
Unordered list indentation
175-175: Expected: 0; Actual: 2 (MD007, ul-indent)
Unordered list indentation
25-25: null (MD014, commands-show-output)
Dollar signs used before commands without showing output
114-114: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
174-174: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
114-114: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
Additional comments not posted (50)
src/datastore/local-store/repos/subscription.repo.spec.ts (1)
1-8: Comprehensive test suite setup for LocalSubscriptionRepoThe test suite setup for
LocalSubscriptionRepois straightforward and leverages the statictestmethod fromSubscriptionRepoto ensure consistency in testing across different implementations. This approach helps maintain uniformity and reusability of test logic, which is a good practice.src/datastore/local-store/repos/notification.repo.spec.ts (1)
1-8: Properly structured test suite for LocalNotificationRepoSimilar to the
LocalSubscriptionRepotest suite, this setup forLocalNotificationRepouses the statictestmethod fromNotificationRepo, ensuring that the testing is consistent and reusable. This is an efficient way to validate the functionality of repository implementations.src/notifications/model/notification.model.ts (1)
Line range hint
5-20: Well-structured NotificationModel classThe
NotificationModelclass is well-defined with clear and concise properties. The constructor usesObject.assignto initialize the instance, which is a neat way to copy properties from the passed object. This pattern is particularly useful for data transfer objects (DTOs) in applications dealing with databases or external APIs.src/subscriptions/types.ts (1)
Line range hint
1-22: Clear definition of SubscriptionStatus and SubscriptionParamsThe
SubscriptionStatusenum andSubscriptionParamstype are clearly defined. The use of TypeScript'sOmitutility type in definingSubscriptionParamsis a good practice as it helps in maintaining the integrity of the type by excluding specific properties without redundancy.src/subscriptions/subscriptions.module.ts (1)
5-18: Appropriate module configuration in SubscriptionsModuleThe
SubscriptionsModuleis configured with essential imports and providers, ensuring that all necessary dependencies are available. UsingDatastoreModule.registerAsync()indicates an asynchronous, configurable approach which could be beneficial for environment-specific configurations. The inclusion of various modules such asScheduleModuleandLoggerModulesuggests a well-thought-out approach to handling scheduling and logging, which are critical in subscription management systems.src/notifications/repo/notifications.repo.ts (2)
1-3: Ensure consistent import paths.Consider using absolute paths or configuring a base path to avoid potential issues with relative paths in a larger project.
5-21: Good use of abstract class for repository pattern.The
NotificationRepoabstract class appropriately defines the contract for notification repositories. This is good practice in implementing the repository pattern, allowing for easy swapping of implementations.src/datastore/postgres/migrations/1718398114107-hooks.ts (2)
6-10: Ensure SQL commands follow best practices.The SQL command for creating the table includes all necessary fields and constraints. Ensure that field types and constraints are suitable for the expected data loads and integrity requirements.
12-14: Simple and clean rollback mechanism.The
downmethod correctly removes the "subscription" table, ensuring that the migration can be cleanly rolled back if necessary.src/notifications/types.ts (1)
Line range hint
1-34: Comprehensive type definitions for notifications.The type definitions, including
NotificationStatusandNotificationParams, are well-defined and clear. These types will help ensure that notifications are handled consistently throughout the application.src/notifications/notifications.module.ts (1)
Line range hint
5-15: Module configuration is comprehensive and modular.The inclusion of
DatastoreModule.registerAsync()and other related modules ensures that the Notifications module has all the dependencies it needs. This modular approach facilitates better separation of concerns and easier testing.
[APROVED]src/datastore/postgres/migrations/1719003936380-notifications.ts (1)
6-18: Ensure SQL commands and index management are optimal.The SQL commands for creating and dropping the "notification" table are correctly implemented. Additionally, managing the index on "offline_tx" is crucial for performance but ensure that the index is necessary and optimally configured for the queries that will be run against it.
src/subscriptions/repo/subscription.repo.ts (4)
5-5: Class Definition: SubscriptionRepoThis abstract class serves as a template for subscription repository implementations, ensuring that all necessary CRUD operations are defined. This is a good use of abstraction in OOP.
8-10: Method: createThis method correctly uses TypeScript's
Omitutility to exclude properties that should not be set by the caller. The method signature is clear and follows best practices.
21-21: Method: incrementNoncesThe method signature is clear, but it would be beneficial to include documentation explaining the purpose and use of this method, especially since it deals with an array of IDs.
+ /** + * Increments the nonce value for a batch of subscription IDs. + * This is typically used to ensure that each notification is processed once in order. + */ public abstract incrementNonces(id: number[]): Promise<void>;
26-28: Static Method: testThis method provides a standardized way to test implementations of the
SubscriptionRepo. It's a good practice to include such self-testing capabilities in abstract classes.src/subscriptions/models/subscription.model.ts (2)
6-6: Class Definition: SubscriptionModelThe
SubscriptionModelclass is well-defined with properties that are essential for managing subscriptions. Each property is clearly typed, which is good for maintainability and type safety.
39-39: Constructor: SubscriptionModelThe constructor uses
Object.assignto initialize the model properties from a passed object. This is a common pattern in TypeScript for data models.src/test-utils/repo-mocks.ts (1)
19-24: Class Definition: MockQueryBuilderThe
MockQueryBuilderclass provides a chainable API for building queries, which is a common pattern in database interactions. The mock implementations of these methods are appropriate for unit testing.src/datastore/postgres/entities/notification.entity.ts (2)
9-31: Entity Definition: NotificationThe
Notificationentity is appropriately annotated with TypeORM decorators to map class properties to database columns. The use of@Columnwith specific types and defaults is correctly implemented.
33-39: Provider: notificationRepoProviderThis provider configuration ensures that the
Notificationrepository can be injected wherever needed. The use of a factory function to get the repository from theDataSourceis a clean and scalable approach.src/datastore/local-store/repos/notification.repo.ts (1)
10-45: Class Implementation: LocalNotificationRepoThe
LocalNotificationRepoclass provides an in-memory implementation for notification management. Methods are well-implemented with clear logic for creating, updating, and finding notifications. This is a suitable approach for local testing or environments where persistence is not required.src/datastore/postgres/entities/subscription.entity.ts (2)
10-41: Entity Definition: Well-defined and comprehensive.The
Subscriptionentity is well-defined with appropriate use of decorators for defining the database schema. Types are correctly specified for each column, and default values are properly set using functions where needed, which is a good practice for ensuring consistency in the database.
43-49: Repository Provider Configuration: Correct and efficient.The factory provider for the
Subscriptionrepository is correctly configured. It uses theDataSourceto get the repository, which is a best practice in TypeORM to ensure that the repository is scoped correctly within the data source context.src/datastore/postgres/repos/notification.repo.spec.ts (1)
11-38: Test Suite: Comprehensive and well-structured.The test suite for
PostgresNotificationRepois comprehensive, covering creation, update, and retrieval of notifications. Mock implementations are correctly used, and the use ofjest-whento conditionally resolve mocks is a good practice that enhances the flexibility and readability of the tests.src/notifications/repo/notification.repo.suite.ts (1)
6-45: Test Suite: Effective and thorough.The test suite effectively covers the main methods (
create,findById,update) of theNotificationRepo. Usingsnapshottesting for thecreateandfindByIdmethods ensures that the entire object structure is verified, which is a robust testing strategy. The separation of test cases into descriptive blocks enhances readability and maintainability.src/datastore/local-store/local-store.module.ts (1)
Line range hint
8-44: Module Configuration: Complete and well-organized.The
LocalStoreModuleis well-configured with all necessary repository providers included. The use of explicit class mappings for each repository ensures that the dependencies are clear and correctly configured. This setup facilitates easy testing and swapping of implementations if needed.src/datastore/postgres/repos/notification.repo.ts (1)
32-42: Ensure proper error handling infindById.The method correctly throws an
AppNotFoundErrorif the notification is not found, which is good practice for handling missing entities in a RESTful API.src/datastore/postgres/postgres.module.ts (1)
33-48: Review module configuration for consistency and completeness.The module configuration correctly sets up providers and exports for repository classes. This ensures that repository instances can be injected where needed in the application and promotes modularity and testability.
src/datastore/postgres/repos/subscription.repo.ts (2)
38-47: Ensure proper error handling infindById.The method correctly throws an
AppNotFoundErrorif the subscription is not found, which is good practice for handling missing entities in a RESTful API.
56-64: Review and optimize theincrementNoncesmethod.The method uses a custom SQL clause to increment nonces. Ensure the column name 'nextNonce' is correctly referenced in the database schema to avoid runtime errors.
Verification successful
Verified: The
nextNoncecolumn is correctly referenced.The
nextNoncecolumn is present in thesubscriptionstable as verified by the search results in the TypeScript entity definition and SQL migration files.
src/datastore/postgres/migrations/1718398114107-hooks.ts: Column definition found.src/datastore/postgres/entities/subscription.entity.ts: Column definition found.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that 'nextNonce' is a valid column in the 'subscriptions' table. # Test: Search for the column name in the entity definition. Expect: Only occurances of 'nextNonce'. rg --type sql $'ALTER TABLE subscriptions ADD COLUMN nextNonce'Length of output: 213
Script:
#!/bin/bash # Description: Verify that 'nextNonce' is a valid column in the 'subscriptions' table. # 1. Search for the 'nextNonce' column in TypeScript entity definitions. ast-grep --lang typescript --pattern 'class Subscription { $$$ nextNonce: $_; $$$ }' # 2. Search for the 'nextNonce' column in SQL migration files. rg 'nextNonce' --type sqlLength of output: 261
Script:
#!/bin/bash # Description: Verify that 'nextNonce' is a valid column in the 'subscriptions' table. # 1. Locate potential TypeScript entity definition files and search for the 'nextNonce' column. fd --type f --extension ts --exec ast-grep --lang typescript --pattern 'class Subscription { $$$ nextNonce: $_; $$$ }' {} # 2. Locate potential SQL migration files and search for the 'nextNonce' column. fd --type f --extension sql --exec rg 'nextNonce' {}Length of output: 180
Script:
#!/bin/bash # Description: Verify that 'nextNonce' is a valid column in the 'subscriptions' table. # 1. Locate potential TypeScript entity definition files and search for the 'nextNonce' column. fd --type f --extension ts | xargs ast-grep --lang typescript --pattern 'class Subscription { $$$ nextNonce: $_; $$$ }' # 2. Locate potential SQL migration files and search for the 'nextNonce' column. fd --type f --extension sql | xargs rg 'nextNonce'Length of output: 2350
src/notifications/notifications.service.ts (5)
12-16: Code Addition: Imports and Class ReferencesImports and new class references have been added. Ensure that these are being utilized properly within the service.
31-32: Code Addition: Logger and Repository InjectionThe logger and notifications repository are injected correctly. This setup is crucial for logging and interacting with the data layer.
42-43: Logic Review: Notification Retrieval and Creation
- The
findOnemethod correctly handles the case where a notification might not exist, throwing an appropriate error.- The
createNotificationsmethod handles the creation and scheduling of notifications efficiently. However, consider adding more detailed logging for each step to improve traceability.Also applies to: 53-71
78-80: Method Implementation: Update NotificationThe method to update notifications is implemented correctly and uses the repository's update function. This is a straightforward and clean implementation.
Line range hint
26-26: Configuration Issue: Decorator MisplacementThe static analysis tool has flagged an issue with decorators. However, no decorators are used in the provided lines. This seems to be a false positive.
Also applies to: 29-29
src/subscriptions/subscriptions.service.ts (5)
12-13: Code Addition: Imports and Class ReferencesImports and new class references have been added. Ensure that these are being utilized properly within the service.
28-29: Code Addition: Logger and Repository InjectionThe logger and subscription repository are injected correctly. This setup is crucial for logging and interacting with the data layer.
47-60: Logic Review: Subscription Retrieval and FilteringThe
findAllmethod correctly handles filtering and exclusion of expired subscriptions. The logic is sound and efficiently uses the repository's capabilities.
72-73: Method Implementation: Find One SubscriptionThe method to retrieve a single subscription is implemented correctly, handling the case where a subscription might not exist by throwing an appropriate error.
Line range hint
24-24: Configuration Issue: Decorator MisplacementThe static analysis tool has flagged an issue with decorators. However, no decorators are used in the provided lines. This seems to be a false positive.
src/notifications/notifications.service.spec.ts (5)
4-18: Code Addition: Imports and Mock SetupThe file sets up necessary imports and mocks for testing NotificationsService. Ensure that these setups are correctly used in the tests.
Line range hint
43-66: Setup: Mock Repository CreationThe creation of a mock repository for notifications is done correctly, which is crucial for isolating the service logic during testing.
Line range hint
87-122: Test Case Review: Find One NotificationThe test cases for finding a notification by ID are well-implemented, covering both successful retrieval and error handling scenarios.
Line range hint
135-187: Test Case Review: Create NotificationsThe test case for creating notifications and handling their lifecycle (including retries and status updates) is comprehensive and well-implemented.
201-203: Security Issue: Generic API Key DetectedA generic API key has been detected in the test setup. This could expose sensitive operations if the key is valid and used in production environments.
- const legitimacySecret = 'someSecret'; + const legitimacySecret = 'REPLACE_WITH_ENV_VARIABLE_OR_SECURE_STORE';Likely invalid or redundant comment.
src/subscriptions/subscriptions.service.spec.ts (4)
5-16: Code Addition: Imports and Mock SetupThe file sets up necessary imports and mocks for testing SubscriptionsService. Ensure that these setups are correctly used in the tests.
Line range hint
38-86: Setup: Mock Repository CreationThe creation of a mock repository for subscriptions is done correctly, which is crucial for isolating the service logic during testing.
Line range hint
114-162: Test Case Review: Find All and Find One SubscriptionThe test cases for retrieving all subscriptions and a single subscription by ID are well-implemented, covering various filtering scenarios and error handling.
Line range hint
173-248: Test Case Review: Create Subscription and Handle ResponsesThe test cases for creating a subscription and handling handshake responses are comprehensive, including retry mechanisms and status updates.
polymath-eric
force-pushed
the
persist-webhooks
branch
from
6dc864d
to
f93a625
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range and nitpick comments (12)
src/notifications/notifications.service.ts (1)
Line range hint
42-66: Review of thecreateNotificationsmethodThis method handles the creation of multiple notifications efficiently. However, the loop could potentially be optimized by batching the database writes instead of calling
createfor each notification individually.const notifications = newNotifications.map(notification => ({ ...notification, triesLeft: this.maxTries, status: NotificationStatus.Active, })); const createdIds = await this.notificationsRepo.bulkCreate(notifications); createdIds.forEach((id, index) => { this.scheduleSendNotification(id, 0); }); return createdIds.map(notification => notification.id);src/subscriptions/subscriptions.service.spec.ts (1)
Line range hint
201-201: Security issue: Detected Generic API KeyA potential security risk was detected with an exposed API key in the code. It is crucial to ensure that sensitive information such as API keys are not hard-coded or exposed in public repositories.
- const apiKey = 'cGxhY2Vob2xkZXI='; + const apiKey = process.env.API_KEY;README.md (10)
Line range hint
111-111: Typographical Correction: "block chain" should be "blockchain".The term "blockchain" is conventionally spelled as a single word.
- transactions to the block chain (generally POST routes). + transactions to the blockchain (generally POST routes).
Line range hint
130-130: Grammar Correction: Use "a" instead of "an" before "transaction".The word "transaction" starts with a consonant sound, so "a" is the correct article to use.
- `AMQP` This creates an transaction to be processed by worker processes using an AMQP broker to ensure reliable processing + `AMQP` This creates a transaction to be processed by worker processes using an AMQP broker to ensure reliable processing
Line range hint
143-143: Grammar Correction: Use third-person singular verb form.The verb "use" should be in the third-person singular form "uses" when referring to a singular noun.
- To refer to a key when signing use the Vault name and version `${name}-${version}` e.g. `alice-1`. + To refer to a key when signing uses the Vault name and version `${name}-${version}` e.g. `alice-1`.
Line range hint
148-148: Grammar Correction: Singular verb form needed.The verb "correspond" should be in the singular form "corresponds" to agree with the singular subject "This".
- This correspond to `account`, `change` and `address_index` from the [BIP-44 standard](https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki). + This corresponds to `account`, `change` and `address_index` from the [BIP-44 standard](https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki).
Line range hint
153-153: Grammar Correction: Add a comma after "signer".Adding a comma after "signer" improves the readability of the sentence.
- When making a transaction that requires a signer use the corresponding `LOCAL_SIGNERS` (by array offset). + When making a transaction that requires a signer, use the corresponding `LOCAL_SIGNERS` (by array offset).
Line range hint
165-165: Grammar Correction: Change "are" to "is" for singular subject agreement.The phrase "there are a set of" should be corrected to "there is a set of" for proper subject-verb agreement.
- Currently there are a set of "offline" modules, that setup listeners to the different queues. + Currently there is a set of "offline" modules, that setup listeners to the different queues.
Line range hint
171-171: Grammar Correction: Add a comma after "though".A comma after "though" clarifies the sentence structure and separates the conditional clause.
- any AMQP 1.0 compliant broker should work though. + any AMQP 1.0 compliant broker should work, though.
Line range hint
173-173: Grammar Correction: Correct abbreviation "i.e." to have two periods.The abbreviation "i.e." should be written with two periods to adhere to standard English usage.
- it is strongly recommended to use a persistent data store (i.e postgres). + it is strongly recommended to use a persistent data store (i.e. postgres).
Line range hint
185-185: Grammar Correction: Use "an" instead of "a" before a vowel sound.The article "an" should be used before "endpoint" because it starts with a vowel sound.
- which will enabled a endpoint at `/developer-testing/webhook` + which will enable an endpoint at `/developer-testing/webhook`
Line range hint
191-191: Style Correction: Replace "can not" with "cannot" for standard usage.The contraction "cannot" is more commonly used and is preferred in formal writing.
- As such it can not guarantee delivery of messages + As such it cannot guarantee delivery of messages
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (4)
src/datastore/local-store/repos/__snapshots__/notification.repo.spec.ts.snapis excluded by!**/*.snapsrc/datastore/local-store/repos/__snapshots__/subscription.repo.spec.ts.snapis excluded by!**/*.snapsrc/datastore/postgres/repos/__snapshots__/notification.repo.spec.ts.snapis excluded by!**/*.snapsrc/datastore/postgres/repos/__snapshots__/subscription.repo.spec.ts.snapis excluded by!**/*.snap
Files selected for processing (30)
- README.md (1 hunks)
- src/datastore/local-store/local-store.module.ts (2 hunks)
- src/datastore/local-store/repos/notification.repo.spec.ts (1 hunks)
- src/datastore/local-store/repos/notification.repo.ts (1 hunks)
- src/datastore/local-store/repos/subscription.repo.spec.ts (1 hunks)
- src/datastore/local-store/repos/subscription.repo.ts (1 hunks)
- src/datastore/postgres/entities/notification.entity.ts (1 hunks)
- src/datastore/postgres/entities/subscription.entity.ts (1 hunks)
- src/datastore/postgres/migrations/1718398114107-hooks.ts (1 hunks)
- src/datastore/postgres/migrations/1719003936380-notifications.ts (1 hunks)
- src/datastore/postgres/postgres.module.ts (2 hunks)
- src/datastore/postgres/repos/notification.repo.spec.ts (1 hunks)
- src/datastore/postgres/repos/notification.repo.ts (1 hunks)
- src/datastore/postgres/repos/subscription.repo.spec.ts (1 hunks)
- src/datastore/postgres/repos/subscription.repo.ts (1 hunks)
- src/notifications/model/notification.model.ts (2 hunks)
- src/notifications/notifications.module.ts (2 hunks)
- src/notifications/notifications.service.spec.ts (8 hunks)
- src/notifications/notifications.service.ts (3 hunks)
- src/notifications/repo/notification.repo.suite.ts (1 hunks)
- src/notifications/repo/notifications.repo.ts (1 hunks)
- src/notifications/types.ts (2 hunks)
- src/subscriptions/models/subscription.model.ts (2 hunks)
- src/subscriptions/repo/subscription.repo.suite.ts (1 hunks)
- src/subscriptions/repo/subscription.repo.ts (1 hunks)
- src/subscriptions/subscriptions.module.ts (1 hunks)
- src/subscriptions/subscriptions.service.spec.ts (10 hunks)
- src/subscriptions/subscriptions.service.ts (6 hunks)
- src/subscriptions/types.ts (2 hunks)
- src/test-utils/repo-mocks.ts (2 hunks)
Files skipped from review as they are similar to previous changes (24)
- src/datastore/local-store/local-store.module.ts
- src/datastore/local-store/repos/notification.repo.spec.ts
- src/datastore/local-store/repos/notification.repo.ts
- src/datastore/local-store/repos/subscription.repo.spec.ts
- src/datastore/local-store/repos/subscription.repo.ts
- src/datastore/postgres/entities/notification.entity.ts
- src/datastore/postgres/entities/subscription.entity.ts
- src/datastore/postgres/migrations/1718398114107-hooks.ts
- src/datastore/postgres/migrations/1719003936380-notifications.ts
- src/datastore/postgres/postgres.module.ts
- src/datastore/postgres/repos/notification.repo.spec.ts
- src/datastore/postgres/repos/subscription.repo.spec.ts
- src/notifications/model/notification.model.ts
- src/notifications/notifications.module.ts
- src/notifications/notifications.service.spec.ts
- src/notifications/repo/notification.repo.suite.ts
- src/notifications/repo/notifications.repo.ts
- src/notifications/types.ts
- src/subscriptions/models/subscription.model.ts
- src/subscriptions/repo/subscription.repo.suite.ts
- src/subscriptions/repo/subscription.repo.ts
- src/subscriptions/subscriptions.module.ts
- src/subscriptions/types.ts
- src/test-utils/repo-mocks.ts
Additional context used
Path-based instructions (5)
src/datastore/postgres/repos/notification.repo.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/datastore/postgres/repos/subscription.repo.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/notifications/notifications.service.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/subscriptions/subscriptions.service.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/subscriptions/subscriptions.service.spec.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.
Biome
src/datastore/postgres/repos/notification.repo.ts
[error] 14-14: Decorators are not valid here. (parse)
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabledoption totruein your configuration file.src/datastore/postgres/repos/subscription.repo.ts
[error] 14-14: Decorators are not valid here. (parse)
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabledoption totruein your configuration file.src/notifications/notifications.service.ts
[error] 26-26: Decorators are not valid here. (parse)
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabledoption totruein your configuration file.
[error] 29-29: Decorators are not valid here. (parse)
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabledoption totruein your configuration file.src/subscriptions/subscriptions.service.ts
[error] 24-24: Decorators are not valid here. (parse)
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabledoption totruein your configuration file.
Gitleaks
src/subscriptions/subscriptions.service.spec.ts
201-201: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key)
LanguageTool
README.md
[misspelling] ~111-~111: This word is normally spelled as one. (EN_COMPOUNDS_BLOCK_CHAIN)
Context: ...dpoints that submit transactions to the block chain (generally POST routes). Each of these ...
[misspelling] ~130-~130: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’. (EN_A_VS_AN)
Context: ...d to the chain. -AMQPThis creates an transaction to be processed by worker p...
[grammar] ~143-~143: Consider using third-person verb forms for singular and mass nouns. (MASS_AGREEMENT)
Context: ... it. To refer to a key when signing use the Vault name and version `${name}-${v...
[grammar] ~148-~148: The verb ‘correspond’ is plural. Did you mean: “corresponds”? Did you use a verb instead of a noun? (PLURAL_VERB_AFTER_THIS)
Context: ...s separated by-, as in1-0-0. This correspond toaccount,changeand `address_ind...
[uncategorized] ~153-~153: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...en making a transaction that requires a signer use the correspondingLOCAL_SIGNERS(...
[uncategorized] ~165-~165: A comma may be missing after the conjunctive/linking adverb ‘Currently’. (SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Context: ... AMQP topic, instead of being returned. Currently there are a set of "offline" modules, t...
[grammar] ~165-~165: There may be a verb agreement error, if referring to a singular entity (a set). (THERE_IS_ARE)
Context: ...tead of being returned. Currently there are a set of "offline" modules, that setup ...
[uncategorized] ~167-~167: The preposition ‘to’ seems more likely in this position. (AI_HYDRA_LEO_REPLACE_ON_TO)
Context: ...zed to an offline payload and published onRequests1. A signer process subscrib...
[typographical] ~168-~168: It appears that a comma is missing. (DURING_THAT_TIME_COMMA)
Context: ...cess subscribes toRequests. For each message it generates a signature, and publishes...
[uncategorized] ~168-~168: The preposition ‘to’ seems more likely in this position. (AI_HYDRA_LEO_REPLACE_ON_TO)
Context: ...es a signature, and publishes a message onSignatures1. A submitter process sub...
[uncategorized] ~171-~171: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...plications to subscribe to To use AMQP mode a message broker must be configured. Th...
[uncategorized] ~171-~171: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...is/) is used, with an AMQP acceptor. In theory any AMQP 1.0 compliant broker should wo...
[typographical] ~171-~171: It seems that a comma is missing. (THOUGH_COMMA)
Context: ...ny AMQP 1.0 compliant broker should work though. If using AMQP, it is strongly recomme...
[uncategorized] ~173-~173: The abbreviation “i.e.” (= that is) requires two periods. (I_E)
Context: ...mmended to use a persistent data store (i.e postgres). There are two tables related...
[typographical] ~174-~174: Consider wrapping this idiom with two commas if you want to put a strong emphasize on it. (FOR_WHATEVER_REASON_COMMA)
Context: ...and to detect ones rejected by the chain for some reason -offline_eventis a table for the ...
[grammar] ~177-~177: An apostrophe ‘s’ denotes possession. Did you mean to use the plural form of the noun (no apostrophe)? (NOUN_APOSTROPHE_S_VERB)
Context: ..., serving as an audit log If using the project's compose file, an Artemis console will b...
[typographical] ~180-~180: Consider adding a comma after ‘Normally’ for more clarity. (RB_LY_COMMA)
Context: ...me and password. ### Webhooks (alpha) Normally the endpoints that create transactions ...
[uncategorized] ~181-~181: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...When processModesubmitAndCallbackis used thewebhookUrlparam must also be pro...
[uncategorized] ~183-~183: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ... Before sending any information to the endpoint the service will first make a request w...
[uncategorized] ~185-~185: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...nto the response headers. If you are a developer you can toggle an endpoint to aid with ...
[misspelling] ~185-~185: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’. (EN_A_VS_AN)
Context: ...EVELOPER_UTILS=truewhich will enabled a endpoint at/developer-testing/webhook...
[uncategorized] ~189-~189: A comma may be missing after the conjunctive/linking adverb ‘However’. (SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Context: ...and should not be used against mainnet. However the API should be stable to develop aga...
[style] ~191-~191: Unless you want to emphasize “not”, use “cannot” which is more common. (CAN_NOT_PREMIUM)
Context: ...ensure it won't miss events. As such it can not guarantee delivery of messages The pla...
[uncategorized] ~198-~198: A comma may be missing after the conjunctive/linking adverb ‘Currently’. (SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Context: ...configurable with multiple strategies. Currently there are two strategies available: 1....
[uncategorized] ~204-~204: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ... 1. Open: By configuringopenas a strategy any request will be authenticated with ...
[typographical] ~204-~204: The word “however” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence. (HOWEVER_SENTENCE)
Context: ...is is primarily intended for development, however it can be used to provide a "read only"...
[style] ~204-~204: Consider using “who” when you are referring to a person instead of an object. (THAT_WHO)
Context: ...d in combination with a signing manager that holds valuable keys. More strategies c...
[uncategorized] ~212-~212: When ‘API-specific’ is used as a modifier, it is usually spelled with a hyphen. (SPECIFIC_HYPHEN)
Context: ...T request. This only affects where REST API specific entities are stored (e.g. Users and Api...
[uncategorized] ~213-~213: A comma may be missing after the conjunctive/linking adverb ‘Currently’. (SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Context: ...chain regardless of the datastore used Currently there are two datastore available: 1. ...
[grammar] ~217-~217: Consider using either the past participle “run” or the present participle “running” here. (BEEN_PART_AGREEMENT)
Context: ...e state. This allows the REST API to be ran as a single process which is convenient...
[uncategorized] ~217-~217: A comma may be missing after the conjunctive/linking adverb ‘However’. (SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Context: ...urposes (i.e. nosignersare loaded). However all state will be lost when the process...
[uncategorized] ~219-~219: If this is a compound adjective that modifies the following noun, use a hyphen. (EN_COMPOUND_ADJECTIVE_INTERNAL)
Context: ...ts down 1. Postgres This is the more production ready approach. This allows state to be persi...
[typographical] ~219-~219: Consider adding a comma after ‘Internally’ for more clarity. (RB_LY_COMMA)
Context: ...instances to user the same information. Internally this uses TypeORM to manage the databas...
[uncategorized] ~223-~223: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...an implementation. Then in the concrete implementations define a new Repo that satisfies the te...
[uncategorized] ~230-~230: A comma is probably missing here. (MISSING_COMMA_AFTER_INTRODUCTORY_PHRASE)
Context: ...y, or use a file with--env-file. For documentation you will need to expose a port that map...
[uncategorized] ~242-~242: A comma may be missing after the conjunctive/linking adverb ‘Currently’. (SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Context: ...or the Artemis AMQP container to start Currently in "Settings" > "Features in developmen...
Markdownlint
README.md
126-126: Expected: 0; Actual: 2 (MD007, ul-indent)
Unordered list indentation
127-127: Expected: 0; Actual: 2 (MD007, ul-indent)
Unordered list indentation
128-128: Expected: 0; Actual: 2 (MD007, ul-indent)
Unordered list indentation
129-129: Expected: 0; Actual: 2 (MD007, ul-indent)
Unordered list indentation
130-130: Expected: 0; Actual: 2 (MD007, ul-indent)
Unordered list indentation
174-174: Expected: 0; Actual: 2 (MD007, ul-indent)
Unordered list indentation
175-175: Expected: 0; Actual: 2 (MD007, ul-indent)
Unordered list indentation
25-25: null (MD014, commands-show-output)
Dollar signs used before commands without showing output
114-114: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
174-174: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
114-114: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
Additional comments not posted (11)
src/datastore/postgres/repos/notification.repo.ts (2)
36-46: Review of thefindByIdmethodThis method is well-implemented with proper error handling. The use of
AppNotFoundErroris appropriate for cases where the notification is not found in the database.
14-14: Decorator issue: Incorrect usage of@InjectRepositoryThe
@InjectRepositorydecorator is incorrectly used here. It should be placed directly above the constructor parameter it decorates, not above the constructor declaration itself. This is necessary for NestJS to correctly resolve the dependency injection.- @InjectRepository(Notification) private notificationRepo: Repository<Notification> + constructor(@InjectRepository(Notification) private notificationRepo: Repository<Notification>) {}Likely invalid or redundant comment.
Tools
Biome
[error] 14-14: Decorators are not valid here. (parse)
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabledoption totruein your configuration file.src/datastore/postgres/repos/subscription.repo.ts (3)
38-47: Review of thefindByIdmethodThis method is correctly implemented with proper error handling. The use of
AppNotFoundErroris appropriate for cases where the subscription is not found in the database.
50-54: Review of thefindAllmethodThis method is efficiently implemented, leveraging the TypeORM
findmethod to retrieve all subscriptions and then mapping them to the model.
56-64: Review of theincrementNoncesmethodThis method correctly uses a bulk update operation to increment nonces for multiple subscriptions. The use of a raw query builder is appropriate here due to the complexity of the operation.
src/notifications/notifications.service.ts (1)
78-80: Review of theupdateNotificationmethodThe method is straightforward and correctly delegates the update operation to the repository. No issues found here.
src/subscriptions/subscriptions.service.ts (5)
70-71: Review of thefindOnemethodThe method correctly handles fetching a single subscription and throws an appropriate error if not found. Good use of error handling.
81-91: Review of thecreateSubscriptionmethodThe method efficiently handles the creation of a subscription and sets up an initial handshake. The use of current time for
createdAtis appropriate.
107-109: Review of theupdateSubscriptionmethodThe method is straightforward and correctly delegates the update operation to the repository, focusing only on relevant fields.
116-120: Review of thebatchMarkAsDonemethodThis method efficiently handles marking multiple subscriptions as done using a batch operation. Good use of asynchronous patterns with
Promise.all.
127-127: Review of thebatchBumpNoncemethodThe method correctly handles incrementing the nonce for multiple subscriptions in a batch operation. Efficient use of repository methods.
| public async create(params: NotificationParams): Promise<NotificationModel> { | ||
| const entity = this.notificationRepo.create({ | ||
| ...params, | ||
| }); | ||
|
|
||
| await this.notificationRepo | ||
| .save(entity) | ||
| .catch( | ||
| convertTypeOrmErrorToAppError( | ||
| `Subscription ID: ${params.subscriptionId}`, | ||
| NotificationRepo.type | ||
| ) | ||
| ); | ||
|
|
||
| const notification = new NotificationModel({ | ||
| ...entity, | ||
| }); | ||
| return notification; |
There was a problem hiding this comment.
Review of the create method
The create method effectively handles the creation of a notification entity and maps it to a model. However, the error handling could be improved by using a more specific error type for better clarity and control flow management.
- .catch(convertTypeOrmErrorToAppError(`Subscription ID: ${params.subscriptionId}`, NotificationRepo.type));
+ .catch(e => throw new SpecificNotificationError(`Failed to create notification for Subscription ID: ${params.subscriptionId}`, e));Committable suggestion was skipped due to low confidence.
polymath-eric
force-pushed
the
persist-webhooks
branch
from
f93a625
to
5e1ca05
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Outside diff range and nitpick comments (2)
src/subscriptions/subscriptions.service.spec.ts (2)
Line range hint
46-58: Refactor: Consider using a factory or builder pattern for creatingSubscriptionModelinstances.Currently,
SubscriptionModelinstances are created inline, which could lead to duplicated code if the model instantiation is complex or occurs in multiple places.- new SubscriptionModel({ - id: 1, - eventType: EventType.TransactionUpdate, - ... - }), + // Using a builder or factory pattern + SubscriptionModelBuilder.withDefaults() + .id(1) + .eventType(EventType.TransactionUpdate) + .build(),
Line range hint
173-194: Potential security issue with hardcoded secrets in tests.Using hardcoded secrets even in test configurations can lead to accidental leaks or misuse in production environments.
- const legitimacySecret = 'someSecret'; + // Use environment variables or a secure vault for secrets, even in tests + const legitimacySecret = process.env.LEGITIMACY_SECRET;
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (4)
src/datastore/local-store/repos/__snapshots__/notification.repo.spec.ts.snapis excluded by!**/*.snapsrc/datastore/local-store/repos/__snapshots__/subscription.repo.spec.ts.snapis excluded by!**/*.snapsrc/datastore/postgres/repos/__snapshots__/notification.repo.spec.ts.snapis excluded by!**/*.snapsrc/datastore/postgres/repos/__snapshots__/subscription.repo.spec.ts.snapis excluded by!**/*.snap
Files selected for processing (31)
- README.md (2 hunks)
- src/common/decorators/swagger.ts (2 hunks)
- src/datastore/local-store/local-store.module.ts (2 hunks)
- src/datastore/local-store/repos/notification.repo.spec.ts (1 hunks)
- src/datastore/local-store/repos/notification.repo.ts (1 hunks)
- src/datastore/local-store/repos/subscription.repo.spec.ts (1 hunks)
- src/datastore/local-store/repos/subscription.repo.ts (1 hunks)
- src/datastore/postgres/entities/notification.entity.ts (1 hunks)
- src/datastore/postgres/entities/subscription.entity.ts (1 hunks)
- src/datastore/postgres/migrations/1718398114107-hooks.ts (1 hunks)
- src/datastore/postgres/migrations/1719003936380-notifications.ts (1 hunks)
- src/datastore/postgres/postgres.module.ts (2 hunks)
- src/datastore/postgres/repos/notification.repo.spec.ts (1 hunks)
- src/datastore/postgres/repos/notification.repo.ts (1 hunks)
- src/datastore/postgres/repos/subscription.repo.spec.ts (1 hunks)
- src/datastore/postgres/repos/subscription.repo.ts (1 hunks)
- src/notifications/model/notification.model.ts (2 hunks)
- src/notifications/notifications.module.ts (2 hunks)
- src/notifications/notifications.service.spec.ts (8 hunks)
- src/notifications/notifications.service.ts (3 hunks)
- src/notifications/repo/notification.repo.suite.ts (1 hunks)
- src/notifications/repo/notifications.repo.ts (1 hunks)
- src/notifications/types.ts (2 hunks)
- src/subscriptions/models/subscription.model.ts (2 hunks)
- src/subscriptions/repo/subscription.repo.suite.ts (1 hunks)
- src/subscriptions/repo/subscription.repo.ts (1 hunks)
- src/subscriptions/subscriptions.module.ts (1 hunks)
- src/subscriptions/subscriptions.service.spec.ts (10 hunks)
- src/subscriptions/subscriptions.service.ts (6 hunks)
- src/subscriptions/types.ts (2 hunks)
- src/test-utils/repo-mocks.ts (2 hunks)
Files skipped from review as they are similar to previous changes (24)
- src/datastore/local-store/local-store.module.ts
- src/datastore/local-store/repos/notification.repo.spec.ts
- src/datastore/local-store/repos/notification.repo.ts
- src/datastore/local-store/repos/subscription.repo.spec.ts
- src/datastore/local-store/repos/subscription.repo.ts
- src/datastore/postgres/entities/notification.entity.ts
- src/datastore/postgres/entities/subscription.entity.ts
- src/datastore/postgres/migrations/1718398114107-hooks.ts
- src/datastore/postgres/migrations/1719003936380-notifications.ts
- src/datastore/postgres/postgres.module.ts
- src/datastore/postgres/repos/notification.repo.spec.ts
- src/datastore/postgres/repos/subscription.repo.spec.ts
- src/notifications/model/notification.model.ts
- src/notifications/notifications.module.ts
- src/notifications/notifications.service.spec.ts
- src/notifications/repo/notification.repo.suite.ts
- src/notifications/repo/notifications.repo.ts
- src/notifications/types.ts
- src/subscriptions/models/subscription.model.ts
- src/subscriptions/repo/subscription.repo.suite.ts
- src/subscriptions/repo/subscription.repo.ts
- src/subscriptions/subscriptions.module.ts
- src/subscriptions/types.ts
- src/test-utils/repo-mocks.ts
Additional context used
Path-based instructions (6)
src/datastore/postgres/repos/notification.repo.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/datastore/postgres/repos/subscription.repo.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/notifications/notifications.service.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/common/decorators/swagger.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/subscriptions/subscriptions.service.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.src/subscriptions/subscriptions.service.spec.ts (2)
Pattern
**/*.ts: Review the JavaScript code for conformity with the Semi-Standard style guide, highlighting any deviations.
Pattern
**/*.ts: Analyze the logic of the code and the efficiency of the algorithms used. Suggest improvements if any inefficient algorithms are found.
Biome
src/datastore/postgres/repos/notification.repo.ts
[error] 14-14: Decorators are not valid here. (parse)
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabledoption totruein your configuration file.src/datastore/postgres/repos/subscription.repo.ts
[error] 14-14: Decorators are not valid here. (parse)
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabledoption totruein your configuration file.src/notifications/notifications.service.ts
[error] 26-26: Decorators are not valid here. (parse)
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabledoption totruein your configuration file.
[error] 29-29: Decorators are not valid here. (parse)
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabledoption totruein your configuration file.src/common/decorators/swagger.ts
[error] 80-92: This default parameter should follow the last required parameter or should be a required parameter. (lint/style/useDefaultParameterLast)
The last required parameter is here:
A default parameter that precedes a required parameter cannot be omitted at call site.
Unsafe fix: Turn the parameter into a required parameter.src/subscriptions/subscriptions.service.ts
[error] 24-24: Decorators are not valid here. (parse)
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabledoption totruein your configuration file.
Gitleaks
src/subscriptions/subscriptions.service.spec.ts
201-201: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key)
LanguageTool
README.md
[misspelling] ~111-~111: This word is normally spelled as one. (EN_COMPOUNDS_BLOCK_CHAIN)
Context: ...dpoints that submit transactions to the block chain (generally POST routes). Each of these ...
[misspelling] ~130-~130: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’. (EN_A_VS_AN)
Context: ...d to the chain. -AMQPThis creates an transaction to be processed by worker p...
[grammar] ~143-~143: Consider using third-person verb forms for singular and mass nouns. (MASS_AGREEMENT)
Context: ... it. To refer to a key when signing use the Vault name and version `${name}-${v...
[grammar] ~148-~148: The verb ‘correspond’ is plural. Did you mean: “corresponds”? Did you use a verb instead of a noun? (PLURAL_VERB_AFTER_THIS)
Context: ...s separated by-, as in1-0-0. This correspond toaccount,changeand `address_ind...
[uncategorized] ~153-~153: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...en making a transaction that requires a signer use the correspondingLOCAL_SIGNERS(...
[uncategorized] ~159-~159: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ... e.g.clientIdormemoAfter being generated the signature with the payload can be p...
[uncategorized] ~165-~165: A comma may be missing after the conjunctive/linking adverb ‘Currently’. (SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Context: ... AMQP topic, instead of being returned. Currently there are a set of "offline" modules, t...
[grammar] ~165-~165: There may be a verb agreement error, if referring to a singular entity (a set). (THERE_IS_ARE)
Context: ...tead of being returned. Currently there are a set of "offline" modules, that setup ...
[uncategorized] ~167-~167: The preposition ‘to’ seems more likely in this position. (AI_HYDRA_LEO_REPLACE_ON_TO)
Context: ...zed to an offline payload and published onRequests1. A signer process subscrib...
[typographical] ~168-~168: It appears that a comma is missing. (DURING_THAT_TIME_COMMA)
Context: ...cess subscribes toRequests. For each message it generates a signature, and publishes...
[uncategorized] ~168-~168: The preposition ‘to’ seems more likely in this position. (AI_HYDRA_LEO_REPLACE_ON_TO)
Context: ...es a signature, and publishes a message onSignatures1. A submitter process sub...
[typographical] ~171-~171: It seems that a comma is missing. (THOUGH_COMMA)
Context: ...ny AMQP 1.0 compliant broker should work though. If using AMQP, it is strongly recomme...
[uncategorized] ~173-~173: The abbreviation “i.e.” (= that is) requires two periods. (I_E)
Context: ...mmended to use a persistent data store (i.e postgres). There are two tables related...
[typographical] ~174-~174: Consider wrapping this idiom with two commas if you want to put a strong emphasize on it. (FOR_WHATEVER_REASON_COMMA)
Context: ...and to detect ones rejected by the chain for some reason -offline_eventis a table for the ...
[grammar] ~177-~177: An apostrophe ‘s’ denotes possession. Did you mean to use the plural form of the noun (no apostrophe)? (NOUN_APOSTROPHE_S_VERB)
Context: ..., serving as an audit log If using the project's compose file, an Artemis console will b...
[typographical] ~180-~180: Consider adding a comma after ‘Normally’ for more clarity. (RB_LY_COMMA)
Context: ...me and password. ### Webhooks (alpha) Normally the endpoints that create transactions ...
[uncategorized] ~185-~185: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...nto the response headers. If you are a developer you can toggle an endpoint to aid with ...
[misspelling] ~185-~185: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’. (EN_A_VS_AN)
Context: ...EVELOPER_UTILS=truewhich will enabled a endpoint at/developer-testing/webhook...
[uncategorized] ~189-~189: A comma may be missing after the conjunctive/linking adverb ‘However’. (SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Context: ...and should not be used against mainnet. However the API should be stable to develop aga...
[style] ~191-~191: Unless you want to emphasize “not”, use “cannot” which is more common. (CAN_NOT_PREMIUM)
Context: ...ensure it won't miss events. As such it can not guarantee delivery of messages The pla...
[uncategorized] ~198-~198: A comma may be missing after the conjunctive/linking adverb ‘Currently’. (SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Context: ...configurable with multiple strategies. Currently there are two strategies available: 1....
[uncategorized] ~204-~204: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ... 1. Open: By configuringopenas a strategy any request will be authenticated with ...
[typographical] ~204-~204: The word “however” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence. (HOWEVER_SENTENCE)
Context: ...is is primarily intended for development, however it can be used to provide a "read only"...
[style] ~204-~204: Consider using “who” when you are referring to a person instead of an object. (THAT_WHO)
Context: ...d in combination with a signing manager that holds valuable keys. More strategies c...
[uncategorized] ~212-~212: When ‘API-specific’ is used as a modifier, it is usually spelled with a hyphen. (SPECIFIC_HYPHEN)
Context: ...T request. This only affects where REST API specific entities are stored (e.g. Users and Api...
[uncategorized] ~213-~213: A comma may be missing after the conjunctive/linking adverb ‘Currently’. (SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Context: ...chain regardless of the datastore used Currently there are two datastore available: 1. ...
[grammar] ~217-~217: Consider using either the past participle “run” or the present participle “running” here. (BEEN_PART_AGREEMENT)
Context: ...e state. This allows the REST API to be ran as a single process which is convenient...
[uncategorized] ~217-~217: A comma may be missing after the conjunctive/linking adverb ‘However’. (SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Context: ...urposes (i.e. nosignersare loaded). However all state will be lost when the process...
[uncategorized] ~219-~219: If this is a compound adjective that modifies the following noun, use a hyphen. (EN_COMPOUND_ADJECTIVE_INTERNAL)
Context: ...ts down 1. Postgres This is the more production ready approach. This allows state to be persi...
[typographical] ~219-~219: Consider adding a comma after ‘Internally’ for more clarity. (RB_LY_COMMA)
Context: ...instances to user the same information. Internally this uses TypeORM to manage the databas...
[uncategorized] ~223-~223: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...an implementation. Then in the concrete implementations define a new Repo that satisfies the te...
[uncategorized] ~229-~229: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...g. ### With docker To pass in the env variables you can use-eto pass them individua...
[uncategorized] ~230-~230: A comma is probably missing here. (MISSING_COMMA_AFTER_INTRODUCTORY_PHRASE)
Context: ...y, or use a file with--env-file. For documentation you will need to expose a port that map...
[uncategorized] ~237-~237: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...will take you to the swagger playground UI where all endpoints are documented and ...
[uncategorized] ~242-~242: A comma may be missing after the conjunctive/linking adverb ‘Currently’. (SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Context: ...or the Artemis AMQP container to start Currently in "Settings" > "Features in developmen...
Markdownlint
README.md
126-126: Expected: 0; Actual: 2 (MD007, ul-indent)
Unordered list indentation
127-127: Expected: 0; Actual: 2 (MD007, ul-indent)
Unordered list indentation
128-128: Expected: 0; Actual: 2 (MD007, ul-indent)
Unordered list indentation
129-129: Expected: 0; Actual: 2 (MD007, ul-indent)
Unordered list indentation
130-130: Expected: 0; Actual: 2 (MD007, ul-indent)
Unordered list indentation
174-174: Expected: 0; Actual: 2 (MD007, ul-indent)
Unordered list indentation
175-175: Expected: 0; Actual: 2 (MD007, ul-indent)
Unordered list indentation
25-25: null (MD014, commands-show-output)
Dollar signs used before commands without showing output
114-114: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
174-174: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
114-114: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
Additional comments not posted (15)
src/datastore/postgres/repos/subscription.repo.ts (1)
14-14: Decorator Misplacement IssueThe
@InjectRepositorydecorator is used incorrectly. It should be placed directly above the constructor parameter it decorates, not above the constructor declaration itself.- constructor(@InjectRepository(Subscription) private subscriptionRepo: Repository<Subscription>) {} + constructor(private subscriptionRepo: @InjectRepository(Subscription) Repository<Subscription>) {}Likely invalid or redundant comment.
Tools
Biome
[error] 14-14: Decorators are not valid here. (parse)
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabledoption totruein your configuration file.src/notifications/notifications.service.ts (3)
Line range hint
12-32: Initialization of Service PropertiesThe constructor of
NotificationsServicecorrectly initializesmaxTriesandretryIntervalfrom the configuration. This is a good practice as it allows for easy adjustments via configuration changes.
42-43: Error Handling infindOneMethodThe method
findOnecorrectly throws anAppNotFoundErrorif a notification is not found, which is a good practice for handling missing entities in a RESTful API.
53-73: Batch Creation and Scheduling increateNotificationsMethodThis method efficiently handles the creation of multiple notifications and schedules them for sending. Using
Promise.allfor concurrent operations is a good practice here.src/common/decorators/swagger.ts (1)
Line range hint
184-200: Enhanced API Response DecoratorsThe
ApiTransactionResponsefunction is well-designed to handle different response scenarios based on theprocessMode. This dynamic response handling is beneficial for API clients to understand the expected behavior based on their request parameters.src/subscriptions/subscriptions.service.ts (3)
Line range hint
12-29: Initialization of Service PropertiesThe constructor of
SubscriptionsServicecorrectly initializesttl,maxTries, andretryIntervalfrom the configuration. This is a good practice as it allows for easy adjustments via configuration changes.
45-58: Filtering Logic infindAllMethodThe
findAllmethod provides comprehensive filtering capabilities, which is crucial for a service handling potentially large sets of data. The method's ability to exclude expired subscriptions based on a flag is particularly useful.
70-71: Error Handling infindOneMethodThe method
findOnecorrectly throws anAppNotFoundErrorif a subscription is not found, which is a good practice for handling missing entities in a RESTful API.src/subscriptions/subscriptions.service.spec.ts (6)
5-16: LGTM: Import statements are well-organized and relevant.The imports are correctly organized and relevant to the functionality being tested in this spec file.
114-117: LGTM: Proper setup of mocked methodfindAll.The setup for the mocked
findAllmethod ofSubscriptionRepois correctly implemented, ensuring that the service'sfindAllmethod returns predictable results during testing.
153-154: Correct handling of errors and data retrieval infindOnemethod.The method correctly handles scenarios where the subscription is found and where it is not, throwing an appropriate error in the latter case.
Also applies to: 161-162
245-248: Ensure correct decrement oftriesLeftduring error handling.The method correctly handles decrementing the
triesLeftcounter and setting the appropriate status when retries are exhausted.
285-290: LGTM: Correct batch update of subscription statuses.The
batchMarkAsDonemethod correctly updates the status of multiple subscriptions in a batch, which is efficient and reduces the number of database operations.
38-38: Ensure proper mocking ofSubscriptionRepo.It's crucial that
SubscriptionRepois correctly mocked to isolate the unit tests from external dependencies.Verification successful
Proper mocking of
SubscriptionRepoverified.The
SubscriptionRepois correctly mocked in thesubscriptions.service.spec.tsfile. Here are the relevant lines:
let mockSubscriptionRepo: DeepMocked<SubscriptionRepo>;mockSubscriptionRepo = createMock<SubscriptionRepo>();- Various instances of
mockSubscriptionRepobeing used in tests.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify proper mocking of `SubscriptionRepo`. # Test: Search for incorrect mock usage. Expect: No incorrect usages found. rg --type python $'mockSubscriptionRepo'Length of output: 70
Script:
#!/bin/bash # Description: Verify proper mocking of `SubscriptionRepo`. # Test: Search for correct mock usage of `mockSubscriptionRepo`. Expect: Proper mocking found in TypeScript files. rg --type ts 'mockSubscriptionRepo'Length of output: 1936
README.md (1)
181-181: Update to webhook process mode description is accurate and clear.The update from
submitAndCallbacktosubmitWithCallbackis correctly reflected in the README, providing clear and accurate information about the webhook process mode.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- compose/init-db.sh (1 hunks)
Additional comments not posted (2)
compose/init-db.sh (2)
5-5: Addition of thebtree_gistextension.The script now includes the creation of the
btree_gistextension, which is necessary for certain index types in PostgreSQL. This is a good addition if your application requires these index types.
5-5: Ensure proper handling of database creation logic.The script correctly checks for the existence of the
restdatabase before attempting to create it, which is a good practice to prevent errors. However, consider explicitly handling the case where the database already exists, to avoid any unintended side effects or errors in logs.
polymath-eric
force-pushed
the
persist-webhooks
branch
from
8d3fd4d
to
0fa3cdf
Compare
add Local and Postgres repositories for subscriptions and notifications
add needed subquery extension to the DB
polymath-eric
force-pushed
the
persist-webhooks
branch
from
0fa3cdf
to
7349ebd
Compare
polymath-eric
requested review from
VictorVicente,
prashantasdeveloper and
sansan
as code owners
|
|
🎉 This PR is included in version 5.5.0-alpha.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 6.0.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
JIRA Link
DA-999 (MQ integration remains though)
Changelog / Description
add Local and Postgres repositories for subscriptions and notifications
Checklist -
Summary by CodeRabbit
New Features
LocalNotificationRepoandLocalSubscriptionRepofor local data storage.NotificationandSubscriptionentity classes for PostgreSQL database.Bug Fixes
submitAndCallbacktosubmitWithCallback.Documentation
Chores
btree_gistextension creation in the database initialization script.Tests
LocalNotificationRepoandLocalSubscriptionRepo.MockPostgresRepository.