feat(condo): DOMA-13192 divide voip push types by data#7706
Conversation
YEgorLu
changed the title
📝 WalkthroughWalkthroughThis PR introduces distinct VoIP incoming call message types for native and B2C app calls, with per-app push type filtering and multi-message-per-call sending. The service now generates both legacy and type-specific messages per resident, while push delivery is configurable per appId. Localization and tests are added across three languages and comprehensive coverage scenarios. ChangesVoIP Incoming Call Message Type Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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: d2d1fd904b
ℹ️ 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".
| } | ||
| results.push({ result, error }) | ||
| } | ||
| return { resident, contact, user, results } |
There was a problem hiding this comment.
When start pushes are now returned as results, parseStartingMessagesIdsByUserIdsByMessageResults still reads promiseResult.value.result?.id, so this new return shape leaves startingMessagesIdsByUserIds empty for every successful call. SendVoIPCallCancelMessageService later uses that cache to fill voipIncomingCallId for cancel pushes, so older mobile clients that rely on the start message id will receive cancel messages without it.
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/condo/domains/miniapp/schema/SendVoIPCallStartMessageService.js (1)
79-93:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
parseStartingMessagesIdsByUserIdsByMessageResultsdoes not handle the newresultsarray structure.The function destructures
result(singular) on line 86, but the promise now resolves to{ resident, contact, user, results }with a pluralresultsarray (line 306). This meansresult?.idon line 87 will always beundefined, and no starting message IDs will be cached for call status tracking.🐛 Proposed fix
function parseStartingMessagesIdsByUserIdsByMessageResults ({ sendMessagePromisesResults }) { const startingMessagesIdsByUserIds = {} for (const promiseResult of sendMessagePromisesResults) { if (promiseResult.status !== 'fulfilled') { continue } - const { user, result } = promiseResult.value - if (result?.id) { - startingMessagesIdsByUserIds[user.id] = result.id + const { user, results } = promiseResult.value + // Use the first successfully created message ID for call status tracking + const firstSuccessfulResult = results?.find(r => r.result?.id) + if (firstSuccessfulResult?.result?.id) { + startingMessagesIdsByUserIds[user.id] = firstSuccessfulResult.result.id } } return startingMessagesIdsByUserIds }🤖 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.js` around lines 79 - 93, parseStartingMessagesIdsByUserIdsByMessageResults currently expects a singular result but the promise value now contains a results array; update the function to iterate or search promiseResult.value.results (not promiseResult.value.result) and extract the appropriate message id (e.g. the first element with an id) and assign it to startingMessagesIdsByUserIds[user.id]; reference sendMessagePromisesResults, parseStartingMessagesIdsByUserIdsByMessageResults, promiseResult.value.results and user.id when making the change.
🧹 Nitpick comments (1)
apps/condo/domains/miniapp/schema/SendVoIPCallStartMessageService.spec.js (1)
11-17: ⚡ Quick winMake appId behavior explicit in mocked push rules
Current setup configures only one appId, while later assertions assume deterministic behavior for both appIds. Please add explicit rules for
APP_ID_DISABLED_VOIP_INCOMING_CALL_MESSAGEto avoid default-dependent/flaky assertions.Suggested deterministic setup
return { pushTypeSwitchRulesByAppId: { [APP_ID_ENABLED_VOIP_INCOMING_CALL_MESSAGE]: { [VOIP_INCOMING_CALL_MESSAGE_TYPE]: 'enabled', [VOIP_INCOMING_NATIVE_CALL_MESSAGE_TYPE]: 'disabled', [VOIP_INCOMING_B2C_APP_CALL_MESSAGE_TYPE]: 'disabled', }, + [APP_ID_DISABLED_VOIP_INCOMING_CALL_MESSAGE]: { + [VOIP_INCOMING_CALL_MESSAGE_TYPE]: 'disabled', + [VOIP_INCOMING_NATIVE_CALL_MESSAGE_TYPE]: 'enabled', + [VOIP_INCOMING_B2C_APP_CALL_MESSAGE_TYPE]: 'enabled', + }, }, }🤖 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 11 - 17, The mocked push rules only define pushTypeSwitchRulesByAppId for APP_ID_ENABLED_VOIP_INCOMING_CALL_MESSAGE but tests later assume deterministic behavior for both app IDs; add an explicit entry for APP_ID_DISABLED_VOIP_INCOMING_CALL_MESSAGE inside pushTypeSwitchRulesByAppId mirroring the enabled app's structure with the desired 'enabled'/'disabled' values for VOIP_INCOMING_CALL_MESSAGE_TYPE, VOIP_INCOMING_NATIVE_CALL_MESSAGE_TYPE, and VOIP_INCOMING_B2C_APP_CALL_MESSAGE_TYPE so assertions no longer rely on defaults and become deterministic; update the mock in the spec where pushTypeSwitchRulesByAppId is defined and ensure constants APP_ID_DISABLED_VOIP_INCOMING_CALL_MESSAGE, VOIP_INCOMING_CALL_MESSAGE_TYPE, VOIP_INCOMING_NATIVE_CALL_MESSAGE_TYPE, and VOIP_INCOMING_B2C_APP_CALL_MESSAGE_TYPE are used.
🤖 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.test.js`:
- Around line 873-880: The test assertion for createdNativeMessages incorrectly
expects VOIP_INCOMING_NATIVE_CALL_MESSAGE_TYPE twice; update the expectation to
assert that createdNativeMessages contains one object with type
VOIP_INCOMING_CALL_MESSAGE_TYPE (legacy) and one object with type
VOIP_INCOMING_NATIVE_CALL_MESSAGE_TYPE (native-specific), i.e., change one of
the expect.objectContaining entries to use VOIP_INCOMING_CALL_MESSAGE_TYPE so
the array assertion checks for both message types.
---
Outside diff comments:
In `@apps/condo/domains/miniapp/schema/SendVoIPCallStartMessageService.js`:
- Around line 79-93: parseStartingMessagesIdsByUserIdsByMessageResults currently
expects a singular result but the promise value now contains a results array;
update the function to iterate or search promiseResult.value.results (not
promiseResult.value.result) and extract the appropriate message id (e.g. the
first element with an id) and assign it to
startingMessagesIdsByUserIds[user.id]; reference sendMessagePromisesResults,
parseStartingMessagesIdsByUserIdsByMessageResults, promiseResult.value.results
and user.id when making the change.
---
Nitpick comments:
In `@apps/condo/domains/miniapp/schema/SendVoIPCallStartMessageService.spec.js`:
- Around line 11-17: The mocked push rules only define
pushTypeSwitchRulesByAppId for APP_ID_ENABLED_VOIP_INCOMING_CALL_MESSAGE but
tests later assume deterministic behavior for both app IDs; add an explicit
entry for APP_ID_DISABLED_VOIP_INCOMING_CALL_MESSAGE inside
pushTypeSwitchRulesByAppId mirroring the enabled app's structure with the
desired 'enabled'/'disabled' values for VOIP_INCOMING_CALL_MESSAGE_TYPE,
VOIP_INCOMING_NATIVE_CALL_MESSAGE_TYPE, and
VOIP_INCOMING_B2C_APP_CALL_MESSAGE_TYPE so assertions no longer rely on defaults
and become deterministic; update the mock in the spec where
pushTypeSwitchRulesByAppId is defined and ensure constants
APP_ID_DISABLED_VOIP_INCOMING_CALL_MESSAGE, VOIP_INCOMING_CALL_MESSAGE_TYPE,
VOIP_INCOMING_NATIVE_CALL_MESSAGE_TYPE, and
VOIP_INCOMING_B2C_APP_CALL_MESSAGE_TYPE are used.
🪄 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: 99356129-1530-4a01-9894-ac66e17ce8d2
📒 Files selected for processing (22)
apps/condo/domains/miniapp/schema/SendVoIPCallStartMessageService.jsapps/condo/domains/miniapp/schema/SendVoIPCallStartMessageService.spec.jsapps/condo/domains/miniapp/schema/SendVoIPCallStartMessageService.test.jsapps/condo/domains/miniapp/utils/sendVoIPCallMessage.jsapps/condo/domains/miniapp/utils/testSchema/index.jsapps/condo/domains/notification/constants/constants.jsapps/condo/domains/notification/transports/push.jsapps/condo/lang/en/en.jsonapps/condo/lang/en/messages/VOIP_INCOMING_B2C_APP_CALL_MESSAGE/default.njkapps/condo/lang/en/messages/VOIP_INCOMING_B2C_APP_CALL_MESSAGE/push.njkapps/condo/lang/en/messages/VOIP_INCOMING_NATIVE_CALL_MESSAGE/default.njkapps/condo/lang/en/messages/VOIP_INCOMING_NATIVE_CALL_MESSAGE/push.njkapps/condo/lang/es/es.jsonapps/condo/lang/es/messages/VOIP_INCOMING_B2C_APP_CALL_MESSAGE/default.njkapps/condo/lang/es/messages/VOIP_INCOMING_B2C_APP_CALL_MESSAGE/push.njkapps/condo/lang/es/messages/VOIP_INCOMING_NATIVE_CALL_MESSAGE/default.njkapps/condo/lang/es/messages/VOIP_INCOMING_NATIVE_CALL_MESSAGE/push.njkapps/condo/lang/ru/messages/VOIP_INCOMING_B2C_APP_CALL_MESSAGE/default.njkapps/condo/lang/ru/messages/VOIP_INCOMING_B2C_APP_CALL_MESSAGE/push.njkapps/condo/lang/ru/messages/VOIP_INCOMING_NATIVE_CALL_MESSAGE/default.njkapps/condo/lang/ru/messages/VOIP_INCOMING_NATIVE_CALL_MESSAGE/push.njkapps/condo/lang/ru/ru.json
| expect(createdNativeMessages).toEqual(expect.arrayContaining([ | ||
| expect.objectContaining({ | ||
| type: VOIP_INCOMING_NATIVE_CALL_MESSAGE_TYPE, | ||
| }), | ||
| expect.objectContaining({ | ||
| type: VOIP_INCOMING_NATIVE_CALL_MESSAGE_TYPE, | ||
| }), | ||
| ])) |
There was a problem hiding this comment.
Fix native-call message type assertion
Line 878 expects VOIP_INCOMING_NATIVE_CALL_MESSAGE_TYPE twice, but native flow should create one legacy VOIP_INCOMING_CALL_MESSAGE_TYPE plus one native-specific message.
Proposed fix
expect(createdNativeMessages).toEqual(expect.arrayContaining([
expect.objectContaining({
- type: VOIP_INCOMING_NATIVE_CALL_MESSAGE_TYPE,
+ type: VOIP_INCOMING_CALL_MESSAGE_TYPE,
}),
expect.objectContaining({
type: VOIP_INCOMING_NATIVE_CALL_MESSAGE_TYPE,
}),
]))📝 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.
| expect(createdNativeMessages).toEqual(expect.arrayContaining([ | |
| expect.objectContaining({ | |
| type: VOIP_INCOMING_NATIVE_CALL_MESSAGE_TYPE, | |
| }), | |
| expect.objectContaining({ | |
| type: VOIP_INCOMING_NATIVE_CALL_MESSAGE_TYPE, | |
| }), | |
| ])) | |
| expect(createdNativeMessages).toEqual(expect.arrayContaining([ | |
| expect.objectContaining({ | |
| type: VOIP_INCOMING_CALL_MESSAGE_TYPE, | |
| }), | |
| expect.objectContaining({ | |
| type: VOIP_INCOMING_NATIVE_CALL_MESSAGE_TYPE, | |
| }), | |
| ])) |
🤖 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.test.js`
around lines 873 - 880, The test assertion for createdNativeMessages incorrectly
expects VOIP_INCOMING_NATIVE_CALL_MESSAGE_TYPE twice; update the expectation to
assert that createdNativeMessages contains one object with type
VOIP_INCOMING_CALL_MESSAGE_TYPE (legacy) and one object with type
VOIP_INCOMING_NATIVE_CALL_MESSAGE_TYPE (native-specific), i.e., change one of
the expect.objectContaining entries to use VOIP_INCOMING_CALL_MESSAGE_TYPE so
the array assertion checks for both message types.
|
1 participant
Summary by CodeRabbit
New Features
Tests