feat(condo): DOMA-13192 do not send push message based on appId + messageType + messageData validator#7786
Conversation
…sageType + messageData validator
📝 WalkthroughWalkthroughAdds per-app-id Zod-based push data validation to the push transport's ChangesPer-app Zod push data validation for VoIP
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0dc54267df
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| const pushTokensWhichDidNotPassValidation = !message.type | ||
| ? [] | ||
| : pushTokens.filter(pushToken => PUSH_DATA_VALIDATORS_BY_APP_ID[pushToken.appId]?.[message.type]?.safeParse(data)?.success === false) |
There was a problem hiding this comment.
When a validator is configured with a strict JSON Schema (for example one produced from a plain z.object(...).toJSONSchema(), which rejects unknown keys), this parses the push transport data object rather than the original message payload. prepareMessageToSend adds transport-only fields such as notificationId, type, and messageCreatedAt to that object, so otherwise valid app payloads fail validation and the token is removed before sending. Validate message.meta.data or strip those transport fields so app validators do not have to be loose.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/condo/domains/miniapp/schema/SendVoIPCallStartMessageService.spec.js`:
- Around line 163-165: Remove the leftover debug logging from the waitFor
polling in SendVoIPCallStartMessageService.spec.js: the console.error call after
Message.getOne is spamming CI on every retry. Delete that debug statement and
keep the polling logic intact, using the existing Message.getOne/admin waitFor
flow to locate the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7bc86bb5-ab32-476a-a552-01aed6bd4e19
📒 Files selected for processing (3)
apps/condo/domains/miniapp/schema/SendVoIPCallStartMessageService.spec.jsapps/condo/domains/miniapp/utils/testSchema/index.jsapps/condo/domains/notification/transports/push.js
| const sentOldNativeMessage = await Message.getOne(admin, { id: messageId }) | ||
| console.error('Message', JSON.stringify(sentOldNativeMessage, null, 2)) | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove leftover debug logging. The console.error('Message', ...) inside the waitFor poll runs on every retry and spams CI output; it looks like a debugging artifact.
🧹 Proposed fix
const sentOldNativeMessage = await Message.getOne(admin, { id: messageId })
- console.error('Message', JSON.stringify(sentOldNativeMessage, null, 2))
-
const responses = sentOldNativeMessage.processingMeta.transportsMeta?.[0]?.deliveryMetadata?.responses ?? []📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const sentOldNativeMessage = await Message.getOne(admin, { id: messageId }) | |
| console.error('Message', JSON.stringify(sentOldNativeMessage, null, 2)) | |
| const sentOldNativeMessage = await Message.getOne(admin, { id: messageId }) | |
| const responses = sentOldNativeMessage.processingMeta.transportsMeta?.[0]?.deliveryMetadata?.responses ?? [] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/condo/domains/miniapp/schema/SendVoIPCallStartMessageService.spec.js`
around lines 163 - 165, Remove the leftover debug logging from the waitFor
polling in SendVoIPCallStartMessageService.spec.js: the console.error call after
Message.getOne is spamming CI on every retry. Delete that debug statement and
keep the polling logic intact, using the existing Message.getOne/admin waitFor
flow to locate the change.
|
3 participants
Summary by CodeRabbit
New Features
Bug Fixes
Tests