Open
Conversation
fix(engine): resolve lint-staged and json path typing
Contributor
Reviewer's GuideAdjusts label persistence to use primary keys, fixes JSON path usage for Prisma queries, adds MySQL-specific handling for message read/unread updates, simplifies onWhatsapp cache lid handling, ensures consistent JSON path filters across services, tweaks lint scripts to disable ESLint flat config, and introduces an Nx project.json for the engine app. Class diagram for updated WhatsApp services and cache utilityclassDiagram
class ChannelStartupService {
+fetchMessages(query: Query_Message_) Promise_any_
}
class BaileysStartupService {
+fetchMessages(query: Query_Message_) Promise_any_
-updateMessagesReadedByTimestamp(remoteJid: string, timestamp: number) Promise_number_
-updateChatUnreadMessages(remoteJid: string) Promise_number_
}
class BusinessStartupService {
+handleIncomingMessage(key: any, instanceId: string) void
}
class ChatwootService {
+syncLastIncomingMessage(instance: any) Promise_void_
+linkQuotedMessage(quotedId: string, instanceId: string) Promise_void_
+deleteMessagesByKey(body: any, instance: any) Promise_void_
}
class OnWhatsappCacheUtil {
<<utility>>
+saveOnWhatsappCache(data: ISaveOnWhatsappCacheParams_array_) Promise_void_
+getOnWhatsappCache(remoteJids: string_array_) Promise_any_array_
}
ChannelStartupService <|-- BaileysStartupService
ChannelStartupService <|-- BusinessStartupService
BaileysStartupService ..> OnWhatsappCacheUtil : uses
ChatwootService ..> BaileysStartupService : shares Message querying
BusinessStartupService ..> BaileysStartupService : shares Message querying
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The JSON
pathfilters you changed from['id']/['fromMe']to'$.id'/'$.fromMe'will behave differently across providers (e.g., Postgres vs MySQL); consider branching these filters byDATABASE.PROVIDERor using provider-agnostic queries as you did with the raw SQL paths to avoid breaking non-MySQL setups. - Changing
fetchMessagesto returnPromise<any>loses useful typing information; it would be better to return a concrete type (e.g.,Promise<Message[]>or a specific DTO) to preserve type safety for callers. - In
onWhatsappCache, the use of(item as any).lidsuggests the TypeScript types for theisOnWhatsappmodel are now out of sync with the actual data; updating the model/interface to reflect the new behavior (or explicitly documenting the computedlid) would avoid the need foranyand potential runtime confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The JSON `path` filters you changed from `['id']`/`['fromMe']` to `'$.id'`/`'$.fromMe'` will behave differently across providers (e.g., Postgres vs MySQL); consider branching these filters by `DATABASE.PROVIDER` or using provider-agnostic queries as you did with the raw SQL paths to avoid breaking non-MySQL setups.
- Changing `fetchMessages` to return `Promise<any>` loses useful typing information; it would be better to return a concrete type (e.g., `Promise<Message[]>` or a specific DTO) to preserve type safety for callers.
- In `onWhatsappCache`, the use of `(item as any).lid` suggests the TypeScript types for the `isOnWhatsapp` model are now out of sync with the actual data; updating the model/interface to reflect the new behavior (or explicitly documenting the computed `lid`) would avoid the need for `any` and potential runtime confusion.
## Individual Comments
### Comment 1
<location path="src/api/integrations/channel/whatsapp/whatsapp.baileys.service.ts" line_range="3784" />
<code_context>
const isLogicalDeleted = configService.get<Database>('DATABASE').DELETE_DATA.LOGICAL_MESSAGE_DELETE;
let message = await this.prismaRepository.message.findFirst({
- where: { key: { path: ['id'], equals: messageId } },
+ where: { key: { path: '$.id', equals: messageId } },
});
if (isLogicalDeleted) {
</code_context>
<issue_to_address>
**issue (bug_risk):** Using string JSON paths (`'$.id'`) in Prisma filters is likely incorrect and may not be supported.
Prisma’s JSON filter `path` option expects a `string[]` (e.g. `path: ['id']`), not a JSONPath-like string (`'$.id'`). Using the string form is likely to cause type errors or failing queries.
This change appears in multiple places (e.g. `fetchMessages`, `ChatwootService`, and other `findFirst` calls). Either keep the array form (`['id']`, `['fromMe']`, etc., which is portable across databases) or, if you need MySQL-specific JSON semantics, use `$queryRaw`/`$executeRaw` with explicit SQL as in `updateMessagesReadedByTimestamp`. Otherwise these filters may not match any messages at runtime.
</issue_to_address>
### Comment 2
<location path="src/api/integrations/channel/whatsapp/whatsapp.baileys.service.ts" line_range="5016-5019" />
<code_context>
}
}
- public async fetchMessages(query: Query<Message>) {
+ public async fetchMessages(query: Query<Message>): Promise<any> {
const keyFilters = query?.where?.key as ExtendedIMessageKey;
</code_context>
<issue_to_address>
**suggestion:** Loosening `fetchMessages` to return `Promise<any>` weakens type safety and downstream usage.
This change hides the actual result shape from callers and removes compile-time checks on returned fields. If you need to support multiple result shapes (e.g., DB-specific raw results), prefer a typed union or a dedicated interface/DTO over `any` to keep flexibility without losing type safety.
Suggested implementation:
```typescript
public async fetchMessages(query: Query<Message>): Promise<Message[]> {
```
If `fetchMessages` can actually return shapes other than `Message[]` (e.g., different DB-specific raw formats), you should:
1. Introduce a dedicated return type, for example:
`type FetchMessagesResult = Message[] | MysqlRawMessageRow[] | PostgresRawMessageRow[];`
2. Replace the method signature with:
`public async fetchMessages(query: Query<Message>): Promise<FetchMessagesResult> { ... }`
3. Ensure the implementation of `fetchMessages` constructs and returns instances matching the chosen union/interface so that call sites can rely on proper type narrowing instead of `any`.
</issue_to_address>
### Comment 3
<location path="project.json" line_range="3" />
<code_context>
+{
+ "name": "engine",
+ "$schema": "../../node_modules/nx/schemas/project-schema.json",
+ "sourceRoot": "apps/engine/src",
+ "projectType": "application",
</code_context>
<issue_to_address>
**issue (bug_risk):** The `$schema` path may be incorrect relative to the location of `project.json`.
Since this `project.json` is at the repo root and `sourceRoot` is `apps/engine/src`, the schema file should likely be referenced as `node_modules/nx/schemas/project-schema.json` (no `../` segments). The current `../../node_modules/...` path would resolve above the repo root and likely break Nx/IDE schema validation. Please verify the actual schema location and update the path accordingly so tooling can validate this config.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| const isLogicalDeleted = configService.get<Database>('DATABASE').DELETE_DATA.LOGICAL_MESSAGE_DELETE; | ||
| let message = await this.prismaRepository.message.findFirst({ | ||
| where: { key: { path: ['id'], equals: messageId } }, | ||
| where: { key: { path: '$.id', equals: messageId } }, |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Using string JSON paths ('$.id') in Prisma filters is likely incorrect and may not be supported.
Prisma’s JSON filter path option expects a string[] (e.g. path: ['id']), not a JSONPath-like string ('$.id'). Using the string form is likely to cause type errors or failing queries.
This change appears in multiple places (e.g. fetchMessages, ChatwootService, and other findFirst calls). Either keep the array form (['id'], ['fromMe'], etc., which is portable across databases) or, if you need MySQL-specific JSON semantics, use $queryRaw/$executeRaw with explicit SQL as in updateMessagesReadedByTimestamp. Otherwise these filters may not match any messages at runtime.
Comment on lines
-5016
to
5019
| public async fetchMessages(query: Query<Message>) { | ||
| public async fetchMessages(query: Query<Message>): Promise<any> { | ||
| const keyFilters = query?.where?.key as ExtendedIMessageKey; | ||
|
|
||
| const timestampFilter = {}; |
Contributor
There was a problem hiding this comment.
suggestion: Loosening fetchMessages to return Promise<any> weakens type safety and downstream usage.
This change hides the actual result shape from callers and removes compile-time checks on returned fields. If you need to support multiple result shapes (e.g., DB-specific raw results), prefer a typed union or a dedicated interface/DTO over any to keep flexibility without losing type safety.
Suggested implementation:
public async fetchMessages(query: Query<Message>): Promise<Message[]> {If fetchMessages can actually return shapes other than Message[] (e.g., different DB-specific raw formats), you should:
- Introduce a dedicated return type, for example:
type FetchMessagesResult = Message[] | MysqlRawMessageRow[] | PostgresRawMessageRow[]; - Replace the method signature with:
public async fetchMessages(query: Query<Message>): Promise<FetchMessagesResult> { ... } - Ensure the implementation of
fetchMessagesconstructs and returns instances matching the chosen union/interface so that call sites can rely on proper type narrowing instead ofany.
| @@ -0,0 +1,38 @@ | |||
| { | |||
| "name": "engine", | |||
| "$schema": "../../node_modules/nx/schemas/project-schema.json", | |||
Contributor
There was a problem hiding this comment.
issue (bug_risk): The $schema path may be incorrect relative to the location of project.json.
Since this project.json is at the repo root and sourceRoot is apps/engine/src, the schema file should likely be referenced as node_modules/nx/schemas/project-schema.json (no ../ segments). The current ../../node_modules/... path would resolve above the repo root and likely break Nx/IDE schema validation. Please verify the actual schema location and update the path accordingly so tooling can validate this config.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📋 Description
🔗 Related Issue
Closes #(issue_number)
🧪 Type of Change
🧪 Testing
📸 Screenshots (if applicable)
✅ Checklist
📝 Additional Notes
Summary by Sourcery
Align WhatsApp message and label persistence with database-specific JSON handling and improve tooling configuration for the engine app.
Bug Fixes:
Enhancements:
Build: