Conversation
…l verified residents on address + unit
📝 WalkthroughWalkthroughAdds a VoIP "start call" feature: GraphQL mutation, access checker, service to resolve verified contacts and send VOIP_INCOMING_CALL messages with rate limiting and Redis/KV caching, VoIP utilities, translations, constants, tests, and test helpers. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant GraphQL as GraphQL<br/>Endpoint
participant Access as Access<br/>Checker
participant Service as SendVoIP<br/>Service
participant DB as Database
participant Redis as Redis<br/>(RateLimit)
participant KV as KVStore
participant Msg as Message<br/>Dispatch
Client->>GraphQL: sendVoIPStartMessage(data)
GraphQL->>Access: canSendVoIPStartMessage(user)
Access-->>GraphQL: allow/deny
alt denied
GraphQL-->>Client: auth error
else allowed
GraphQL->>Service: execute send flow
Service->>DB: resolve B2CAppProperty by addressKey
Service->>DB: query verified Contacts & Residents
Service->>Redis: check/enforce rate limit
alt rate limited
Service-->>GraphQL: rate limit error
else proceed
Service->>Msg: create & dispatch VOIP_INCOMING_CALL messages
Msg->>DB: persist messages
Msg-->>Service: per-message results
Service->>KV: setCallStatus(b2cAppId, callId, status, startingMsgIds)
Service-->>GraphQL: {verifiedContactsCount, createdMessagesCount, erroredMessagesCount}
GraphQL-->>Client: response
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: Ping-pong health check failed 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: b6ca3b8681
ℹ️ 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 requiredMetaData = get(MESSAGE_META[VOIP_INCOMING_CALL_MESSAGE_TYPE], 'data', {}) | ||
| const metaData = Object.fromEntries( | ||
| Object.keys(requiredMetaData).map((key) => [key, preparedDataArgs[key]]) | ||
| ) |
There was a problem hiding this comment.
This resolver builds meta.data strictly from keys present in MESSAGE_META[VOIP_INCOMING_CALL_MESSAGE_TYPE].data, so any input field not listed there is silently dropped. Even though SendVoIPStartMessageData accepts voipPanels and preparedDataArgs includes it, this mapping strips it before sendMessage, which means clients cannot receive multi-panel commands from this new mutation despite the API contract advertising that field.
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: 7
🤖 Fix all issues with AI agents
In `@apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.js`:
- Around line 49-57: The ERRORS entry PROPERTY_NOT_FOUND uses the wrong key
`query: SERVICE_NAME`; change `query` to `mutation` so it matches the mutation
error shape used by other entries (e.g., APP_NOT_FOUND) — update the
PROPERTY_NOT_FOUND object in the ERRORS map to use `mutation: SERVICE_NAME`
instead of `query: SERVICE_NAME` to keep the schema consistent for the
SendVoIPStartMessageService.
- Around line 387-415: The map over sendMessageResults already updates
logInfoStats (createdMessagesCount and erroredMessagesCount) and produces
sendMessageStats, so remove the subsequent for (const messageStat of
sendMessageStats) loop entirely to avoid double-counting; ensure any later code
still reads sendMessageStats (or its filtered results) and that
MESSAGE_SENDING_STATUS logic remains in the existing map block (no other changes
to logInfoStats are needed).
In `@apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.test.js`:
- Line 243: The test's uniqueness assertion uses new Set on msg.user (an
object), which compares by reference and won't deduplicate by value; update the
assertion to deduplicate by the user identifier instead by mapping
createdMessages to the user id (e.g., use createdMessages.map(msg =>
msg.user.id) or msg.userId) before creating the Set and asserting its length so
duplicates are correctly collapsed; ensure you reference createdMessages and
msg.user (or msg.userId) in the change.
In `@apps/condo/lang/en/en.json`:
- Around line 464-469: The translation key
"api.miniapp.sendVoIPStartMessage.WRONG_SENDER_FORMAT" in
apps/condo/lang/en/en.json is missing the {details} placeholder; update its
value to include the "{details}" placeholder (matching the pattern used by
sendB2CAppPushMessage) so that the backend messageInterpolation from
dvAndSender.js can be rendered correctly.
In `@apps/condo/lang/es/es.json`:
- Line 464: Update the Spanish message for the locale key
"api.miniapp.sendVoIPStartMessage.PROPERTY_NOT_FOUND" to use the consistent term
"comunidad" instead of "edificio"; modify the JSON value so it reads something
like "No se encontró la comunidad para el \"addressKey\" proporcionado" while
preserving the key name and proper escaping of the quotes.
In `@apps/condo/schema.graphql`:
- Around line 92024-92108: The GraphQL description contains a typo ("Dtfm
command for panel") originating from the source constant in
SendVoIPStartMessageService.js; open that file, locate the input/field
description string 'Dtfm command for panel' (used to generate the
VoIPPanel.dtfmCommand description) and change it to 'DTMF command for panel',
then re-run whatever schema generation/build step produces
apps/condo/schema.graphql so the corrected description is propagated to the
autogenerated schema.
In `@apps/condo/schema.ts`:
- Around line 115379-115384: The generated GraphQL type VoIpPanel contains a
misspelled field dtfmCommand that must be renamed to dtmfCommand; because
schema.ts is autogenerated, update the source schema/SDL or model that defines
VoIpPanel to use dtmfCommand, then update all downstream references to the old
name (e.g. SendVoIPStartMessageService.js — the call/site around the dtfmCommand
usage, SendB2CAppPushMessageService.js, notification constants, and related
tests) to use dtmfCommand so the GraphQL API and runtime code stay consistent;
run codegen/build and update any serializers/deserializers or tests that
reference dtfmCommand to the corrected symbol.
🧹 Nitpick comments (8)
apps/condo/domains/miniapp/access/SendVoIPStartMessageService.js (2)
5-5: Unuseddatadestructuring.
datais destructured fromargsbut never referenced in the function body. Remove it to avoid confusion.Proposed fix
-async function canSendVoIPStartMessage ({ args: { data }, authentication: { item: user } }) { +async function canSendVoIPStartMessage ({ authentication: { item: user } }) {
10-12: Emptyifblock for SERVICE users silently falls through toreturn false.The TODO is noted, but the empty block is easy to miss during future maintenance. Consider adding an explicit
return falseinside or converting to a comment-only guard so intent is clearer.Would you like me to open an issue to track the SERVICE-type user access implementation (the TODO on line 11)?
apps/condo/domains/miniapp/utils/voip.js (2)
20-24: Biome control-character warning is a false positive here — consider adding a suppression comment.The regex intentionally strips control characters (
\u0000-\u001F) fromcallIdfor sanitization. The existingeslint-disablecomment covers ESLint, but Biome still flags it. If the project runs Biome in CI, you may want to add a Biome-specific suppression or a brief rationale comment so future readers understand the intent.
32-40: Inconsistent return type:falseon validation failure vs. Redis response string on success.
setCallStatusreturnsfalsewhencallIdis invalid (line 33) but returns the raw result ofkv.set(...)on success (typically the string'OK'). Callers checking=== falsewon't break, but a consistent boolean return would be clearer.Proposed fix
async function setCallStatus ({ b2cAppId, callId, status, startingMessagesIdsByUserIds }) { if (!validateCallId(callId)) return false - return kv.set( + await kv.set( buildKey(b2cAppId, normalizeCallId(callId)), JSON.stringify({ status, startingMessagesIdsByUserIds }), 'EX', CALL_STATUS_TTL_IN_SECONDS, ) + return true }apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.test.js (1)
85-97: Test name says "no app with provided id" but the expected error isPROPERTY_NOT_FOUND.The test description mentions a missing app, yet it asserts
ERRORS.PROPERTY_NOT_FOUND. If the service resolves the app viaB2CAppPropertylookup first, this is technically correct behavior, but the test name is misleading. Consider renaming to clarify the actual validation path (e.g., "should throw error if no B2CAppProperty found for app and addressKey").apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.js (3)
297-297:forEachcallback should not return a value (flagged by static analysis).The arrow function
user => userIdToLocale[user.id] = user.localeimplicitly returns the assignment result. Use a block body to avoid this.Proposed fix
- usersOfContacts.forEach(user => userIdToLocale[user.id] = user.locale) + usersOfContacts.forEach(user => { userIdToLocale[user.id] = user.locale })
76-78: Logging:entityNameshould beentity, anderris misleading when there are no errors.Per the coding guidelines, the standard structured logging field is
entity, notentityName. Additionally,err: errorsis passed even on success paths whereerrorsisundefined—consider only including it when errors are present, to avoid noisy log entries.Proposed fix
-const logInfo = ({ b2cAppId, callId, stats, errors }) => { - logger.info({ msg: `${SERVICE_NAME} stats`, entityName: 'B2CApp', entityId: b2cAppId, data: { callId, stats }, err: errors }) +const logInfo = ({ b2cAppId, callId, stats, errors }) => { + logger.info({ msg: `${SERVICE_NAME} stats`, entity: 'B2CApp', entityId: b2cAppId, data: { callId, stats }, ...(errors ? { err: errors } : {}) }) }As per coding guidelines, "Use structured logging with standard fields (msg, data, entityId, entity, count, status, err)".
339-341: Remove commented-out code.Line 340 contains a commented-out
.filter(...)call referencingrateLimitsErrorsByUserIdswhich doesn't exist in this file. This is leftover debug/development code that adds noise.Proposed fix
const sendMessagePromises = verifiedResidentsWithUniqueUsers - // .filter(resident => !rateLimitsErrorsByUserIds[resident.user]) .map(async (resident) => {
| const ERRORS = { | ||
| PROPERTY_NOT_FOUND: { | ||
| query: SERVICE_NAME, | ||
| variable: ['data', 'addressKey'], | ||
| code: BAD_USER_INPUT, | ||
| type: PROPERTY_NOT_FOUND_ERROR, | ||
| message: 'Unable to find Property or B2CAppProperty by provided addressKey', | ||
| messageForUser: `api.miniapp.${SERVICE_NAME}.PROPERTY_NOT_FOUND`, | ||
| }, |
There was a problem hiding this comment.
Inconsistent field: query should be mutation.
PROPERTY_NOT_FOUND on line 51 uses query: SERVICE_NAME, while APP_NOT_FOUND on line 59 and the other errors use mutation: SERVICE_NAME. Since this is a mutation, the field should be mutation for consistency and correctness.
Proposed fix
PROPERTY_NOT_FOUND: {
- query: SERVICE_NAME,
+ mutation: SERVICE_NAME,
variable: ['data', 'addressKey'],📝 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 ERRORS = { | |
| PROPERTY_NOT_FOUND: { | |
| query: SERVICE_NAME, | |
| variable: ['data', 'addressKey'], | |
| code: BAD_USER_INPUT, | |
| type: PROPERTY_NOT_FOUND_ERROR, | |
| message: 'Unable to find Property or B2CAppProperty by provided addressKey', | |
| messageForUser: `api.miniapp.${SERVICE_NAME}.PROPERTY_NOT_FOUND`, | |
| }, | |
| const ERRORS = { | |
| PROPERTY_NOT_FOUND: { | |
| mutation: SERVICE_NAME, | |
| variable: ['data', 'addressKey'], | |
| code: BAD_USER_INPUT, | |
| type: PROPERTY_NOT_FOUND_ERROR, | |
| message: 'Unable to find Property or B2CAppProperty by provided addressKey', | |
| messageForUser: `api.miniapp.${SERVICE_NAME}.PROPERTY_NOT_FOUND`, | |
| }, |
🤖 Prompt for AI Agents
In `@apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.js` around
lines 49 - 57, The ERRORS entry PROPERTY_NOT_FOUND uses the wrong key `query:
SERVICE_NAME`; change `query` to `mutation` so it matches the mutation error
shape used by other entries (e.g., APP_NOT_FOUND) — update the
PROPERTY_NOT_FOUND object in the ERRORS map to use `mutation: SERVICE_NAME`
instead of `query: SERVICE_NAME` to keep the schema consistent for the
SendVoIPStartMessageService.
| const sendMessageStats = sendMessageResults.map(promiseResult => { | ||
| if (promiseResult.status === 'rejected') { | ||
| logInfoStats.erroredMessagesCount++ | ||
| logInfoStats.createMessageErrors.push(promiseResult.reason) | ||
| return { error: promiseResult.reason } | ||
| } | ||
| const { resident, result } = promiseResult.value | ||
| if (result.isDuplicateMessage) { | ||
| logInfoStats.erroredMessagesCount++ | ||
| logInfoStats.createMessageErrors.push(`${resident.id} duplicate message`) | ||
| return { error: `${resident.id} duplicate message` } | ||
| } | ||
| if (result.status !== MESSAGE_SENDING_STATUS) { | ||
| logInfoStats.erroredMessagesCount++ | ||
| logInfoStats.createMessageErrors.push(`${resident.id} invalid status for some reason`) | ||
| return { error: `${resident.id} invalid status for some reason` } | ||
| } | ||
| logInfoStats.createdMessagesCount++ | ||
| return result | ||
| }) | ||
|
|
||
| for (const messageStat of sendMessageStats) { | ||
| if (messageStat.error) { | ||
| logInfoStats.erroredMessagesCount++ | ||
| logInfoStats.createMessageErrors.push(messageStat.error) | ||
| continue | ||
| } | ||
| logInfoStats.createdMessagesCount++ | ||
| } |
There was a problem hiding this comment.
Bug: logInfoStats counters are incremented twice — once in the .map() (lines 387–406) and again in the for loop (lines 408–415).
The .map() on lines 387–406 already increments logInfoStats.erroredMessagesCount and logInfoStats.createdMessagesCount. The for loop on lines 408–415 iterates over the same sendMessageStats array and increments them again. This causes logged stats to report 2× the real counts.
The returned values (lines 432–436) are computed independently from sendMessageStats.filter(...) and are correct, so this only corrupts the logged metrics—but that makes debugging and monitoring unreliable.
Pick one loop or the other. The simplest fix is to remove the second for loop entirely, since the .map() already accumulates the stats:
Proposed fix — remove the redundant second loop
- const sendMessageStats = sendMessageResults.map(promiseResult => {
+ const sendMessageStats = sendMessageResults.map((promiseResult) => {
if (promiseResult.status === 'rejected') {
logInfoStats.erroredMessagesCount++
logInfoStats.createMessageErrors.push(promiseResult.reason)
return { error: promiseResult.reason }
}
const { resident, result } = promiseResult.value
if (result.isDuplicateMessage) {
logInfoStats.erroredMessagesCount++
logInfoStats.createMessageErrors.push(`${resident.id} duplicate message`)
return { error: `${resident.id} duplicate message` }
}
if (result.status !== MESSAGE_SENDING_STATUS) {
logInfoStats.erroredMessagesCount++
logInfoStats.createMessageErrors.push(`${resident.id} invalid status for some reason`)
return { error: `${resident.id} invalid status for some reason` }
}
logInfoStats.createdMessagesCount++
return result
})
-
- for (const messageStat of sendMessageStats) {
- if (messageStat.error) {
- logInfoStats.erroredMessagesCount++
- logInfoStats.createMessageErrors.push(messageStat.error)
- continue
- }
- logInfoStats.createdMessagesCount++
- }🤖 Prompt for AI Agents
In `@apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.js` around
lines 387 - 415, The map over sendMessageResults already updates logInfoStats
(createdMessagesCount and erroredMessagesCount) and produces sendMessageStats,
so remove the subsequent for (const messageStat of sendMessageStats) loop
entirely to avoid double-counting; ensure any later code still reads
sendMessageStats (or its filtered results) and that MESSAGE_SENDING_STATUS logic
remains in the existing map block (no other changes to logInfoStats are needed).
| }, { first: residentsCount }) | ||
|
|
||
| expect(createdMessages).toHaveLength(residentsCount) | ||
| expect([...new Set(createdMessages.map(msg => msg.user))]).toHaveLength(residentsCount) |
There was a problem hiding this comment.
Set on user objects does not deduplicate by value — assertion is weaker than intended.
msg.user is likely an object (e.g., { id: '...' }), so new Set(createdMessages.map(msg => msg.user)) will never collapse duplicates (objects are reference-compared). Map to the ID first:
Proposed fix
- expect([...new Set(createdMessages.map(msg => msg.user))]).toHaveLength(residentsCount)
+ expect([...new Set(createdMessages.map(msg => msg.user.id))]).toHaveLength(residentsCount)📝 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([...new Set(createdMessages.map(msg => msg.user))]).toHaveLength(residentsCount) | |
| expect([...new Set(createdMessages.map(msg => msg.user.id))]).toHaveLength(residentsCount) |
🤖 Prompt for AI Agents
In `@apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.test.js` at
line 243, The test's uniqueness assertion uses new Set on msg.user (an object),
which compares by reference and won't deduplicate by value; update the assertion
to deduplicate by the user identifier instead by mapping createdMessages to the
user id (e.g., use createdMessages.map(msg => msg.user.id) or msg.userId) before
creating the Set and asserting its length so duplicates are correctly collapsed;
ensure you reference createdMessages and msg.user (or msg.userId) in the change.
| "api.miniapp.sendVoIPStartMessage.PROPERTY_NOT_FOUND": "Building not found for the provided \"addressKey\"", | ||
| "api.miniapp.sendVoIPStartMessage.APP_NOT_FOUND": "Application not found for the provided \"id\"", | ||
| "api.miniapp.sendVoIPStartMessage.DV_VERSION_MISMATCH": "Invalid data version value", | ||
| "api.miniapp.sendVoIPStartMessage.WRONG_SENDER_FORMAT": "Invalid format for the \"sender\" field", | ||
| "api.miniapp.sendVoIPStartMessage.pushData.title": "Intercom call", | ||
| "api.miniapp.sendVoIPStartMessage.pushData.body": "Intercom call", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "WRONG_SENDER_FORMAT" -g '!**/node_modules/**' --max-count=50Repository: open-condo-software/condo
Length of output: 12383
🏁 Script executed:
rg -n "sendVoIPStartMessage" -g '!**/node_modules/**' -A 5 -B 5 | head -100Repository: open-condo-software/condo
Length of output: 10455
🏁 Script executed:
rg -n "sendB2CAppPushMessage" -g '!**/node_modules/**' | grep -i "wrong_sender_format"Repository: open-condo-software/condo
Length of output: 470
🏁 Script executed:
rg -n "api.miniapp.sendB2CAppPushMessage.WRONG_SENDER_FORMAT|api.miniapp.sendVoIPStartMessage.WRONG_SENDER_FORMAT" apps/condo/lang/ --multiline -B 0 -A 0Repository: open-condo-software/condo
Length of output: 861
🏁 Script executed:
git diff HEAD -- apps/condo/lang/ | grep -A 2 -B 2 "WRONG_SENDER_FORMAT" | head -60Repository: open-condo-software/condo
Length of output: 51
Add {details} placeholder to sendVoIPStartMessage.WRONG_SENDER_FORMAT translation.
The backend passes a details field via messageInterpolation for this error (as seen in dvAndSender.js). The English translation for sendB2CAppPushMessage correctly includes the {details} placeholder; sendVoIPStartMessage should match this pattern.
Fix
- "api.miniapp.sendVoIPStartMessage.WRONG_SENDER_FORMAT": "Invalid format for the \"sender\" field",
+ "api.miniapp.sendVoIPStartMessage.WRONG_SENDER_FORMAT": "Invalid format for the \"sender\" field. {details}",📝 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.
| "api.miniapp.sendVoIPStartMessage.PROPERTY_NOT_FOUND": "Building not found for the provided \"addressKey\"", | |
| "api.miniapp.sendVoIPStartMessage.APP_NOT_FOUND": "Application not found for the provided \"id\"", | |
| "api.miniapp.sendVoIPStartMessage.DV_VERSION_MISMATCH": "Invalid data version value", | |
| "api.miniapp.sendVoIPStartMessage.WRONG_SENDER_FORMAT": "Invalid format for the \"sender\" field", | |
| "api.miniapp.sendVoIPStartMessage.pushData.title": "Intercom call", | |
| "api.miniapp.sendVoIPStartMessage.pushData.body": "Intercom call", | |
| "api.miniapp.sendVoIPStartMessage.PROPERTY_NOT_FOUND": "Building not found for the provided \"addressKey\"", | |
| "api.miniapp.sendVoIPStartMessage.APP_NOT_FOUND": "Application not found for the provided \"id\"", | |
| "api.miniapp.sendVoIPStartMessage.DV_VERSION_MISMATCH": "Invalid data version value", | |
| "api.miniapp.sendVoIPStartMessage.WRONG_SENDER_FORMAT": "Invalid format for the \"sender\" field. {details}", | |
| "api.miniapp.sendVoIPStartMessage.pushData.title": "Intercom call", | |
| "api.miniapp.sendVoIPStartMessage.pushData.body": "Intercom call", |
🤖 Prompt for AI Agents
In `@apps/condo/lang/en/en.json` around lines 464 - 469, The translation key
"api.miniapp.sendVoIPStartMessage.WRONG_SENDER_FORMAT" in
apps/condo/lang/en/en.json is missing the {details} placeholder; update its
value to include the "{details}" placeholder (matching the pattern used by
sendB2CAppPushMessage) so that the backend messageInterpolation from
dvAndSender.js can be rendered correctly.
| "api.miniapp.sendB2CAppPushMessage.RESIDENT_NOT_FOUND": "No se ha encontrado el vecino con el id introducido", | ||
| "api.miniapp.sendB2CAppPushMessage.USER_NOT_FOUND": "No se ha encontrado el usuario con el id introducido", | ||
| "api.miniapp.sendB2CAppPushMessage.WRONG_SENDER_FORMAT": "Formato del campo \"sender\" no válido", | ||
| "api.miniapp.sendVoIPStartMessage.PROPERTY_NOT_FOUND": "No se encontró el edificio para el \"addressKey\" proporcionado", |
There was a problem hiding this comment.
Use consistent “comunidad” terminology
Line 464 uses “edificio,” but the rest of the Spanish locale consistently refers to properties as “comunidad.” Aligning terms avoids user confusion.
✏️ Suggested wording tweak
- "api.miniapp.sendVoIPStartMessage.PROPERTY_NOT_FOUND": "No se encontró el edificio para el \"addressKey\" proporcionado",
+ "api.miniapp.sendVoIPStartMessage.PROPERTY_NOT_FOUND": "No se encontró la comunidad para el \"addressKey\" proporcionado",📝 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.
| "api.miniapp.sendVoIPStartMessage.PROPERTY_NOT_FOUND": "No se encontró el edificio para el \"addressKey\" proporcionado", | |
| "api.miniapp.sendVoIPStartMessage.PROPERTY_NOT_FOUND": "No se encontró la comunidad para el \"addressKey\" proporcionado", |
🤖 Prompt for AI Agents
In `@apps/condo/lang/es/es.json` at line 464, Update the Spanish message for the
locale key "api.miniapp.sendVoIPStartMessage.PROPERTY_NOT_FOUND" to use the
consistent term "comunidad" instead of "edificio"; modify the JSON value so it
reads something like "No se encontró la comunidad para el \"addressKey\"
proporcionado" while preserving the key name and proper escaping of the quotes.
| enum VoIPType { | ||
| """Makes mobile app use it's call app instead of B2CApp's""" | ||
| sip | ||
| } | ||
|
|
||
| input VoIPPanel { | ||
| """Dtfm command for panel""" | ||
| dtfmCommand: String! | ||
|
|
||
| """Name of a panel to be displayed""" | ||
| name: String! | ||
| } | ||
|
|
||
| input SendVoIPStartMessageData { | ||
| """ | ||
| If you want your B2CApp to handle incoming VoIP call, provide this argument. Otherwise provide all others | ||
| """ | ||
| B2CAppContext: String | ||
|
|
||
| """ | ||
| Unique value for each call session between panel and resident (means same for different devices also). Must be provided for correct work with multiple devices that use same voip call.F.e. to cancel calls with CANCELED_CALL_MESSAGE_PUSH messages | ||
| """ | ||
| callId: String! | ||
|
|
||
| """ | ||
| If "sip" was passed, mobile device will try to start native call. Info about other values will be added later | ||
| """ | ||
| voipType: VoIPType | ||
|
|
||
| """Address of sip server, which device should connect to""" | ||
| voipAddress: String | ||
|
|
||
| """Login for connection to sip server""" | ||
| voipLogin: String | ||
|
|
||
| """Password for connection to sip server""" | ||
| voipPassword: String | ||
|
|
||
| """ | ||
| Panels and their commands to open. First one must be the main one. Multiple panels are in testing stage right now and may change | ||
| """ | ||
| voipPanels: [VoIPPanel] | ||
|
|
||
| """Stun server url""" | ||
| stun: String | ||
|
|
||
| """Preferred codec (usually vp8)""" | ||
| codec: String | ||
| } | ||
|
|
||
| input SendVoIPStartMessageInput { | ||
| dv: Int! | ||
| sender: SenderFieldInput! | ||
| app: B2CAppWhereUniqueInput! | ||
|
|
||
| """ | ||
| Should be "addressKey" of B2CAppProperty / Property for which you want to send message | ||
| """ | ||
| addressKey: String! | ||
|
|
||
| """Name of unit, same as in Property map""" | ||
| unitName: String! | ||
|
|
||
| """Type of unit, same as in Property map""" | ||
| unitType: AllowedVoIPMessageUnitType! | ||
| data: SendVoIPStartMessageData! | ||
| } | ||
|
|
||
| type SendVoIPStartMessageOutput { | ||
| """ | ||
| Count of all Organization Contacts, which we possibly could've sent messages to | ||
| """ | ||
| verifiedContactsCount: Int | ||
|
|
||
| """Count of Messages that will be sent, one for each verified Resident""" | ||
| createdMessagesCount: Int | ||
|
|
||
| """Count of Messages which was not created due to some internal error""" | ||
| erroredMessagesCount: Int | ||
| } | ||
|
|
||
| enum AllowedVoIPMessageUnitType { | ||
| flat | ||
| apartment | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n -i 'dtfm' --type=js --type=ts -g '!schema.graphql'Repository: open-condo-software/condo
Length of output: 1447
Fix the DTMF typo in the source file.
In apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.js line 95, change 'Dtfm command for panel' to 'DTMF command for panel'. This typo is propagated to the autogenerated schema.graphql.
🤖 Prompt for AI Agents
In `@apps/condo/schema.graphql` around lines 92024 - 92108, The GraphQL
description contains a typo ("Dtfm command for panel") originating from the
source constant in SendVoIPStartMessageService.js; open that file, locate the
input/field description string 'Dtfm command for panel' (used to generate the
VoIPPanel.dtfmCommand description) and change it to 'DTMF command for panel',
then re-run whatever schema generation/build step produces
apps/condo/schema.graphql so the corrected description is propagated to the
autogenerated schema.
| export type VoIpPanel = { | ||
| /** Dtfm command for panel */ | ||
| dtfmCommand: Scalars['String']['input']; | ||
| /** Name of a panel to be displayed */ | ||
| name: Scalars['String']['input']; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n 'dtfmCommand' --type js --type ts -C 2Repository: open-condo-software/condo
Length of output: 3120
🏁 Script executed:
rg -n 'dtmfCommand' --type js --type ts -C 2Repository: open-condo-software/condo
Length of output: 51
🏁 Script executed:
rg -n 'SendVoIPStartMessageService' --type js --type ts -C 2Repository: open-condo-software/condo
Length of output: 3054
🏁 Script executed:
head -30 apps/condo/schema.tsRepository: open-condo-software/condo
Length of output: 1690
🏁 Script executed:
rg -n 'voipDtfm' --type js --type tsRepository: open-condo-software/condo
Length of output: 742
The typo dtfmCommand should be dtmfCommand — fix requires coordinated changes across multiple files.
"DTMF" (Dual-Tone Multi-Frequency) is the standard telecom acronym; this field uses the transposed spelling. While schema.ts is autogenerated and should not be edited directly, the typo originates in SendVoIPStartMessageService.js (line 97) and is consistently applied throughout the codebase—including production code, tests, and notification constants. Fixing this requires updating the source schema definition and all downstream usages (SendVoIPStartMessageService.js, SendB2CAppPushMessageService.js, test files, and notification constants) since this is part of the public GraphQL API.
🤖 Prompt for AI Agents
In `@apps/condo/schema.ts` around lines 115379 - 115384, The generated GraphQL
type VoIpPanel contains a misspelled field dtfmCommand that must be renamed to
dtmfCommand; because schema.ts is autogenerated, update the source schema/SDL or
model that defines VoIpPanel to use dtmfCommand, then update all downstream
references to the old name (e.g. SendVoIPStartMessageService.js — the call/site
around the dtfmCommand usage, SendB2CAppPushMessageService.js, notification
constants, and related tests) to use dtmfCommand so the GraphQL API and runtime
code stay consistent; run codegen/build and update any serializers/deserializers
or tests that reference dtfmCommand to the corrected symbol.
| types: [ | ||
| { | ||
| access: true, | ||
| type: 'enum VoIPType {' + |
There was a problem hiding this comment.
not readable format for enum type
There was a problem hiding this comment.
Don't understand what do you mean tbh
There was a problem hiding this comment.
`
enum VoIPType {
"""Makes mobile app use it's call app instead of B2CApp's"""
sip
}
`
| }, | ||
| { | ||
| access: true, | ||
| type: 'input SendVoIPStartMessageData { ' + |
There was a problem hiding this comment.
you can use to write multi line code
There was a problem hiding this comment.
Will try.
Last time I checked generated docs had invalid spacings because of that
| function normalizeCallId (callId) { | ||
| return callId | ||
| // eslint-disable-next-line no-control-regex | ||
| .replace(/[\u0000-\u001F\s]/g, '') |
There was a problem hiding this comment.
better to move to constant and name it
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.test.js (1)
156-184: Redundant IIFE wrappingadmin.The
(async (admin) => { ... })(admin)pattern is used throughout, butadminis aconstthat never changes — a plainasync () => { ... }closure would suffice. This pattern is repeated in multiple test blocks (lines 217, 427, 475).Example simplification
for (let i = 0; i < verifiedContactsCount; i++) { - prepareDataPromises.push((async (admin) => { + prepareDataPromises.push((async () => { const phone = createTestPhone() const userClient = await makeClientWithResidentUser({}, { phone }) ... - })(admin)) + })()) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.test.js` around lines 156 - 184, The tests create prepareDataPromises using unnecessary immediately-invoked async functions that accept admin ((async (admin) => { ... })(admin)) even though admin is a surrounding const; replace those IIFE patterns with plain async arrow functions (e.g., prepareDataPromises.push(async () => { ... })) in the blocks that call createTestContact, createTestResident and makeClientWithResidentUser so the code is simpler and functionally identical; update all occurrences (including the ones mentioned near createTestContact, createTestResident, and makeClientWithResidentUser) to remove the admin parameter from the inner function and reference the outer admin directly.apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.js (2)
47-47: Pass a service identifier togetLogger().Without a name, log entries from this module won't be easily filterable. Consider passing the service name for better observability.
Proposed fix
-const logger = getLogger() +const logger = getLogger(SERVICE_NAME)Note:
SERVICE_NAMEis defined at line 49, so you'd need to either reorder the declarations or inline the string.-const logger = getLogger() +const logger = getLogger('sendVoIPStartMessage')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.js` at line 47, The logger is created without a service name (const logger = getLogger()), so update it to pass a service identifier (e.g., getLogger(SERVICE_NAME) or getLogger('SendVoIPStartMessageService')); either move the SERVICE_NAME declaration above the logger or inline the string literal to ensure the logger receives the service name; change the const logger assignment accordingly so all log entries from this module are identifiable.
322-323: Static analysis:forEachcallback implicitly returns a value.The arrow function
user => userIdToLocale[user.id] = user.localereturns the assigned value. Use braces to avoid the implicit return, or use afor...ofloop.Proposed fix
- usersOfContacts.forEach(user => userIdToLocale[user.id] = user.locale) + usersOfContacts.forEach(user => { userIdToLocale[user.id] = user.locale })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.js` around lines 322 - 323, The forEach callback currently uses an arrow expression that implicitly returns the assignment (usersOfContacts.forEach(user => userIdToLocale[user.id] = user.locale)), which static analysis flags; update the loop to avoid implicit return by either using a block body (usersOfContacts.forEach(user => { userIdToLocale[user.id] = user.locale }) ) or replace the forEach with a for...of loop that assigns into userIdToLocale; locate the occurrences of userIdToLocale and usersOfContacts in SendVoIPStartMessageService.js and apply the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.js`:
- Line 90: Rename all occurrences of the "Dtfm" typo to the standard "DTMF"
across schema and code: update the GraphQL type field in VoIPPanel (the "Dtfm
command" comment/field), rename the data key voipDtfmCommand to voipDTMFCommand
(and any corresponding resolver/serializer/deserializer logic), and adjust any
variable names/comments that reference Dtfm to DTMF to keep identifiers
consistent; ensure you also add a temporary compatibility layer or mapping for
incoming/outgoing payloads that still use the old voipDtfmCommand key to avoid
breaking clients while you roll out the rename.
- Around line 67-73: The CALL_DATA_NOT_PROVIDED error definition is using a
single comma-separated string in the variable array; update the variable field
in the CALL_DATA_NOT_PROVIDED object (where mutation is SERVICE_NAME) to be two
separate strings ['data', 'callData'] instead of ['data, callData'] so it
matches the pattern used by the other error definitions and correctly lists both
variables.
- Around line 362-420: The code is embedding plaintext VoIP credentials into
messageAttrs.meta.data; update the preparedDataArgs construction (used before
building messageAttrs and sent via sendMessage) to never store raw
voipLogin/voipPassword/voipAddress: instead call a secure helper (e.g.,
encryptVoipCredentials or createSecretInSecureStore) to either (a) encrypt the
credential blob and store the ciphertext in meta.data (and mark with a
version/iv) or (b) store credentials in a secure key-value store and put only a
secretId reference in meta.data; ensure
get(MESSAGE_META[VOIP_INCOMING_CALL_MESSAGE_TYPE], 'data') mapping uses the
encrypted/token field names; add validation in the webhook/push transport layer
to require TLS/encryption (config flag like requireWebhookEncryption) and fail
fast if not enabled; and ensure any logging paths that might print messageAttrs
or meta (search for sendMessage usage and any loggers around sendMessage,
message creation, or admin APIs) redact voipLogin/voipPassword/voipAddress
before logging or return only the secretId/ciphertext marker.
In `@apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.test.js`:
- Around line 86-99: Rename the test to accurately reflect the failure source:
update the test title that currently reads "should throw error if no app with
provided id" to something like "should throw error if no B2CAppProperty found
for provided app id" (or similar) so it aligns with the asserted error
ERRORS.PROPERTY_NOT_FOUND; locate the test using
sendVoIPStartMessageByTestClient and the assertion against
ERRORS.PROPERTY_NOT_FOUND and change only the description string, leaving the
test body and assertions (including references to B2CAppProperty lookup
behavior) unchanged.
---
Duplicate comments:
In `@apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.js`:
- Around line 50-58: The ERRORS entry for PROPERTY_NOT_FOUND incorrectly uses
the key "query" instead of "mutation"; in the ERRORS object (look for
PROPERTY_NOT_FOUND and SERVICE_NAME in SendVoIPStartMessageService.js) replace
the "query: SERVICE_NAME" field with "mutation: SERVICE_NAME" so it matches the
other error entries for this mutation service.
- Around line 426-454: The log counters in SendVoIPStartMessageService are being
incremented twice: once inside the sendMessageResults.map (building
sendMessageStats) and again in the subsequent for loop that iterates
sendMessageStats; remove the redundant for loop that re-checks messageStat.error
and increments logInfoStats (so keep the existing counting logic inside the map
that references MESSAGE_SENDING_STATUS, resident and result) and ensure nothing
else depends on that removed loop for side effects; verify sendMessageStats and
the returned values remain unchanged.
In `@apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.test.js`:
- Line 271: The test uses new Set on msg.user (an object) so deduplication is by
reference; change the Set to use a scalar user identifier instead — e.g. replace
expect([...new Set(createdMessages.map(msg =>
msg.user))]).toHaveLength(residentsCount) with something like expect([...new
Set(createdMessages.map(msg => msg.userId ||
msg.user.id))]).toHaveLength(residentsCount) so the uniqueness check uses the
user ID rather than the user object reference.
In `@apps/condo/schema.ts`:
- Around line 115402-115407: The autogenerated type VoIpPanel has a typo: rename
the field dtfmCommand to dtmfCommand in the source GraphQL/SDL (or the
SendVoIPStartMessageService.js schema definition) and then re-run the code
generator to propagate changes; also update all usages and mappings of the
VoIpPanel type (e.g., any resolver, input builders, or client code that
references dtfmCommand) to use VoIpPanel.dtmfCommand so runtime code and types
stay in sync.
---
Nitpick comments:
In `@apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.js`:
- Line 47: The logger is created without a service name (const logger =
getLogger()), so update it to pass a service identifier (e.g.,
getLogger(SERVICE_NAME) or getLogger('SendVoIPStartMessageService')); either
move the SERVICE_NAME declaration above the logger or inline the string literal
to ensure the logger receives the service name; change the const logger
assignment accordingly so all log entries from this module are identifiable.
- Around line 322-323: The forEach callback currently uses an arrow expression
that implicitly returns the assignment (usersOfContacts.forEach(user =>
userIdToLocale[user.id] = user.locale)), which static analysis flags; update the
loop to avoid implicit return by either using a block body
(usersOfContacts.forEach(user => { userIdToLocale[user.id] = user.locale }) ) or
replace the forEach with a for...of loop that assigns into userIdToLocale;
locate the occurrences of userIdToLocale and usersOfContacts in
SendVoIPStartMessageService.js and apply the change.
In `@apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.test.js`:
- Around line 156-184: The tests create prepareDataPromises using unnecessary
immediately-invoked async functions that accept admin ((async (admin) => { ...
})(admin)) even though admin is a surrounding const; replace those IIFE patterns
with plain async arrow functions (e.g., prepareDataPromises.push(async () => {
... })) in the blocks that call createTestContact, createTestResident and
makeClientWithResidentUser so the code is simpler and functionally identical;
update all occurrences (including the ones mentioned near createTestContact,
createTestResident, and makeClientWithResidentUser) to remove the admin
parameter from the inner function and reference the outer admin directly.
| CALL_DATA_NOT_PROVIDED: { | ||
| mutation: SERVICE_NAME, | ||
| variable: ['data, callData'], | ||
| type: CALL_DATA_NOT_PROVIDED_ERROR, | ||
| code: BAD_USER_INPUT, | ||
| message: '"b2cAppCallData" or "nativeCallData" or both should be provided', | ||
| }, |
There was a problem hiding this comment.
Bug: variable array contains a single comma-separated string instead of two elements.
Line 69: ['data, callData'] is a single-element array with the string "data, callData". This should be two separate strings to match the pattern used by the other error definitions.
Proposed fix
CALL_DATA_NOT_PROVIDED: {
mutation: SERVICE_NAME,
- variable: ['data, callData'],
+ variable: ['data', 'callData'],
type: CALL_DATA_NOT_PROVIDED_ERROR,
code: BAD_USER_INPUT,
message: '"b2cAppCallData" or "nativeCallData" or both should be provided',
},📝 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.
| CALL_DATA_NOT_PROVIDED: { | |
| mutation: SERVICE_NAME, | |
| variable: ['data, callData'], | |
| type: CALL_DATA_NOT_PROVIDED_ERROR, | |
| code: BAD_USER_INPUT, | |
| message: '"b2cAppCallData" or "nativeCallData" or both should be provided', | |
| }, | |
| CALL_DATA_NOT_PROVIDED: { | |
| mutation: SERVICE_NAME, | |
| variable: ['data', 'callData'], | |
| type: CALL_DATA_NOT_PROVIDED_ERROR, | |
| code: BAD_USER_INPUT, | |
| message: '"b2cAppCallData" or "nativeCallData" or both should be provided', | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.js` around
lines 67 - 73, The CALL_DATA_NOT_PROVIDED error definition is using a single
comma-separated string in the variable array; update the variable field in the
CALL_DATA_NOT_PROVIDED object (where mutation is SERVICE_NAME) to be two
separate strings ['data', 'callData'] instead of ['data, callData'] so it
matches the pattern used by the other error definitions and correctly lists both
variables.
| """ | ||
| voipType: VoIPType, | ||
| */ | ||
| const MAGIC_VOIP_TYPE_CONSTANT_FOR_OLD_VERSIONS_COMPATIBILITY = 'sip' // without this constant mobile app will not try to make native call |
There was a problem hiding this comment.
Typo in comment and field name: "Dtfm" should be "DTMF".
Line 90 has MAGIC_VOIP_TYPE_CONSTANT_FOR_OLD_VERSIONS_COMPATIBILITY which is fine, but line 103 has Dtfm command and line 390 has voipDtfmCommand — the standard abbreviation is DTMF (Dual-Tone Multi-Frequency). This typo also appears in the GraphQL schema type VoIPPanel (line 103) and the data key (line 390), so it would be a breaking change to fix later.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.js` at line 90,
Rename all occurrences of the "Dtfm" typo to the standard "DTMF" across schema
and code: update the GraphQL type field in VoIPPanel (the "Dtfm command"
comment/field), rename the data key voipDtfmCommand to voipDTMFCommand (and any
corresponding resolver/serializer/deserializer logic), and adjust any variable
names/comments that reference Dtfm to DTMF to keep identifiers consistent;
ensure you also add a temporary compatibility layer or mapping for
incoming/outgoing payloads that still use the old voipDtfmCommand key to avoid
breaking clients while you roll out the rename.
| const startingMessagesIdsByUserIds = {} | ||
|
|
||
| /** @type {Array<Promise<{status, id, isDuplicateMessage}>>} */ | ||
| const sendMessagePromises = verifiedResidentsWithUniqueUsers | ||
| // .filter(resident => !rateLimitsErrorsByUserIds[resident.user]) | ||
| .map(async (resident) => { | ||
| // NOTE(YEgorLu): as in domains/notification/constants/config for VOIP_INCOMING_CALL_MESSAGE_TYPE | ||
| let preparedDataArgs = { | ||
| B2CAppId: b2cAppId, | ||
| B2CAppName: b2cAppName, | ||
| residentId: resident.id, | ||
| callId: callData.callId, | ||
| } | ||
|
|
||
| if (callData.b2cAppCallData) { | ||
| preparedDataArgs = { | ||
| ...preparedDataArgs, | ||
| B2CAppContext: callData.b2cAppCallData.B2CAppContext, | ||
| } | ||
| } | ||
|
|
||
| if (callData.nativeCallData) { | ||
| preparedDataArgs = { | ||
| ...preparedDataArgs, | ||
| voipType: MAGIC_VOIP_TYPE_CONSTANT_FOR_OLD_VERSIONS_COMPATIBILITY, | ||
| voipAddress: callData.nativeCallData.voipAddress, | ||
| voipLogin: callData.nativeCallData.voipLogin, | ||
| voipPassword: callData.nativeCallData.voipPassword, | ||
| voipDtfmCommand: callData.nativeCallData.voipPanels?.[0]?.dtfmCommand, | ||
| voipPanels: callData.nativeCallData.voipPanels, | ||
| stunServers: callData.nativeCallData.stunServers, | ||
| stun: callData.nativeCallData.stunServers?.[0], | ||
| codec: callData.nativeCallData.codec, | ||
| } | ||
| } | ||
|
|
||
| const requiredMetaData = get(MESSAGE_META[VOIP_INCOMING_CALL_MESSAGE_TYPE], 'data', {}) | ||
| const metaData = Object.fromEntries( | ||
| Object.keys(requiredMetaData).map((key) => [key, preparedDataArgs[key]]) | ||
| ) | ||
|
|
||
| const messageAttrs = { | ||
| sender, | ||
| type: VOIP_INCOMING_CALL_MESSAGE_TYPE, | ||
| to: { user: { id: resident.user } }, | ||
| meta: { | ||
| dv, | ||
| title: i18n('api.miniapp.sendVoIPStartMessage.pushData.title', { locale: userIdToLocale[resident.user] }), | ||
| body: i18n('api.miniapp.sendVoIPStartMessage.pushData.body', { locale: userIdToLocale[resident.user] }), | ||
| data: metaData, | ||
| }, | ||
| } | ||
|
|
||
| const res = await sendMessage(context, messageAttrs) | ||
| if (res?.id) { | ||
| startingMessagesIdsByUserIds[resident.user] = res.id | ||
| } | ||
| return { resident, result: res } | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine the sendMessage function implementation
fd -t f "\.js$|\.ts$" | xargs rg -l "export.*sendMessage|function sendMessage|const sendMessage" | grep -v node_modules | head -20Repository: open-condo-software/condo
Length of output: 889
🏁 Script executed:
# Get more context about where sendMessage is defined in this file
rg -n "sendMessage" apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.js -B 5 -A 5Repository: open-condo-software/condo
Length of output: 4080
🏁 Script executed:
# Check imports in the SendVoIPStartMessageService.js file
head -50 apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.jsRepository: open-condo-software/condo
Length of output: 2389
🏁 Script executed:
# Search for sendMessage definition in the codebase
rg -n "const sendMessage\s*=|function sendMessage|export.*sendMessage" --type js -g '!node_modules/**' | head -30Repository: open-condo-software/condo
Length of output: 981
🏁 Script executed:
# Read the sendMessage function implementation
sed -n '32,120p' apps/condo/domains/notification/utils/serverSchema/index.jsRepository: open-condo-software/condo
Length of output: 3626
🏁 Script executed:
# Check MESSAGE_META structure and what fields are allowed/sanitized
rg -n "MESSAGE_META" apps/condo/domains/notification/constants/constants.js -A 30 | head -80Repository: open-condo-software/condo
Length of output: 3753
🏁 Script executed:
# Check the Message model/schema to see how meta is stored
rg -n "meta.*Field|meta.*property" apps/condo/domains/notification/schema/Message.js -B 2 -A 2 | head -50Repository: open-condo-software/condo
Length of output: 51
🏁 Script executed:
# Search for any logging of sensitive data in notification domain
rg -n "logger|getLogger\|logInfo\|log\.info" apps/condo/domains/notification/utils/serverSchema/index.js | head -30Repository: open-condo-software/condo
Length of output: 158
🏁 Script executed:
# Search for VOIP_INCOMING_CALL_MESSAGE_TYPE in MESSAGE_META
rg -n "VOIP_INCOMING_CALL_MESSAGE_TYPE" apps/condo/domains/notification/constants/constants.js -A 20 | head -50Repository: open-condo-software/condo
Length of output: 3354
🏁 Script executed:
# Find the SEND_MESSAGE mutation
rg -n "SEND_MESSAGE.*=|query SEND_MESSAGE" apps/condo/domains/notification/utils/serverSchema/index.js -A 30 | head -80Repository: open-condo-software/condo
Length of output: 51
🏁 Script executed:
# Check the Message.js schema file
rg -n "class Message|meta.*Field" apps/condo/domains/notification/schema/Message.js -B 2 -A 5 | head -100Repository: open-condo-software/condo
Length of output: 51
🏁 Script executed:
# Search for any logging in the Message creation/mutation handler
rg -n "logger\." apps/condo/domains/notification/schema/Message.js | head -20Repository: open-condo-software/condo
Length of output: 51
🏁 Script executed:
# Check if there's any encryption or sanitization of meta in the notification domain
rg -n "encrypt|sanitize|redact" apps/condo/domains/notification/ -t js --type-add "js:!*.test.*" | grep -v node_modules | head -20Repository: open-condo-software/condo
Length of output: 629
🏁 Script executed:
# Get the full VOIP_INCOMING_CALL_MESSAGE_TYPE MESSAGE_META definition
sed -n '614,650p' apps/condo/domains/notification/constants/constants.jsRepository: open-condo-software/condo
Length of output: 1374
🏁 Script executed:
# Search for SEND_MESSAGE mutation definition more broadly
rg -n "SEND_MESSAGE" apps/condo/domains/notification/utils/serverSchema/index.jsRepository: open-condo-software/condo
Length of output: 178
🏁 Script executed:
# Find the GraphQL mutation implementation for sending messages
rg -n "SendMessage.*mutation|resolve.*message.*send" apps/condo/domains/notification/schema/Message.js -i | head -20Repository: open-condo-software/condo
Length of output: 51
🏁 Script executed:
# Check the Message model schema file completely
wc -l apps/condo/domains/notification/schema/Message.jsRepository: open-condo-software/condo
Length of output: 122
🏁 Script executed:
# Read the Message schema to see how meta is handled
head -100 apps/condo/domains/notification/schema/Message.jsRepository: open-condo-software/condo
Length of output: 3529
🏁 Script executed:
# Look for sendMessageTask to see how messages are processed
rg -n "voipLogin\|voipPassword\|voipAddress" apps/condo/domains/notification/ -t js --type-add "js:!*.test.*" | head -20Repository: open-condo-software/condo
Length of output: 51
🏁 Script executed:
# Find where SEND_MESSAGE is imported from or defined
rg -n "SEND_MESSAGE\s*=" apps/condo/domains/notification/ -B 2 -A 2 | head -30Repository: open-condo-software/condo
Length of output: 854
🏁 Script executed:
# Check the graphql queries/mutations file
fd -name "*.js" -path "*/notification/*" | xargs rg -l "gql\`" | head -10Repository: open-condo-software/condo
Length of output: 723
🏁 Script executed:
# Look for the actual mutation handler/resolver
rg -n "SendMessage|sendMessage.*resolver|input SendMessage" apps/condo/domains/notification/schema/Message.js -A 10 | head -60Repository: open-condo-software/condo
Length of output: 51
🏁 Script executed:
# Check task handler for sendMessageBatch to see how messages are processed
head -200 apps/condo/domains/notification/tasks/sendMessageBatch.jsRepository: open-condo-software/condo
Length of output: 2598
🏁 Script executed:
# Search for any sanitization or credential stripping in notification processing
rg -n "voipPassword\|voipLogin\|omit.*voip\|pick.*voip" apps/condo/domains/ -t js --type-add "js:!*.test.*" | head -20Repository: open-condo-software/condo
Length of output: 51
🏁 Script executed:
# Check if there's database logging or audit trail of message meta
rg -n "audit\|history\|log.*meta" apps/condo/domains/notification/ -t js --type-add "js:!*.test.*" -i | head -20Repository: open-condo-software/condo
Length of output: 51
🏁 Script executed:
# Check sendMessageBatch.helpers to see how prepareAndSendMessage works
cat apps/condo/domains/notification/tasks/sendMessageBatch.helpers.js | head -150Repository: open-condo-software/condo
Length of output: 5824
🏁 Script executed:
# Look for how RemoteClient or push notifications are delivered
rg -n "pushToken\|RemoteClient" apps/condo/domains/notification/adapters/ -t js | head -30Repository: open-condo-software/condo
Length of output: 51
🏁 Script executed:
# Check if meta is filtered/sanitized before sending to clients
rg -n "meta.*filter\|meta.*omit\|meta.*pick\|sanitize" apps/condo/domains/notification/ -t js --type-add "js:!*.test.*" | head -20Repository: open-condo-software/condo
Length of output: 51
🏁 Script executed:
# Look for webhook/push adapter implementation to see how meta is transmitted
head -150 apps/condo/domains/notification/adapters/webhookAdapter.jsRepository: open-condo-software/condo
Length of output: 5931
🏁 Script executed:
# Search for how the notification service actually sends data to devices/clients
rg -n "sendPush\|firebase\|fcm\|apns" apps/condo/domains/notification/ -t js --type-add "js:!*.test.*" -i | head -20Repository: open-condo-software/condo
Length of output: 51
🏁 Script executed:
# Check if there's any documentation about security/encryption in notification domain
find apps/condo/domains/notification -name "*.md" -o -name "README*" | xargs cat 2>/dev/null | head -100Repository: open-condo-software/condo
Length of output: 2330
VoIP credentials in meta.data require protection against unauthorized access and logging.
VoIP credentials (voipLogin, voipPassword, voipAddress) are persisted in plaintext in the Message table's meta JSON field. The notification system provides optional webhook encryption but does not enforce it by default—production deployments using webhooks will transmit credentials unencrypted. Implement:
- Encrypt credentials at rest in the Message.meta field
- Enforce encryption in transit for all transport layers (webhooks, push services)
- Exclude credentials from debug/logging output if messages are exposed via APIs or admin interfaces
- Consider storing credentials separately (e.g., in a secure key-value store referenced by ID) rather than embedding them in every message record
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.js` around
lines 362 - 420, The code is embedding plaintext VoIP credentials into
messageAttrs.meta.data; update the preparedDataArgs construction (used before
building messageAttrs and sent via sendMessage) to never store raw
voipLogin/voipPassword/voipAddress: instead call a secure helper (e.g.,
encryptVoipCredentials or createSecretInSecureStore) to either (a) encrypt the
credential blob and store the ciphertext in meta.data (and mark with a
version/iv) or (b) store credentials in a secure key-value store and put only a
secretId reference in meta.data; ensure
get(MESSAGE_META[VOIP_INCOMING_CALL_MESSAGE_TYPE], 'data') mapping uses the
encrypted/token field names; add validation in the webhook/push transport layer
to require TLS/encryption (config flag like requireWebhookEncryption) and fail
fast if not enabled; and ensure any logging paths that might print messageAttrs
or meta (search for sendMessage usage and any loggers around sendMessage,
message creation, or admin APIs) redact voipLogin/voipPassword/voipAddress
before logging or return only the secretId/ciphertext marker.
| test('should throw error if no app with provided id', async () => { | ||
| await expectToThrowGQLErrorToResult(async () => { | ||
| await sendVoIPStartMessageByTestClient(admin, { | ||
| app: { id: faker.datatype.uuid() }, | ||
| addressKey: faker.datatype.uuid(), | ||
| unitName: faker.random.alphaNumeric(3), | ||
| unitType: FLAT_UNIT_TYPE, | ||
| callData: { | ||
| callId: faker.datatype.uuid(), | ||
| b2cAppCallData: { B2CAppContext: '' }, | ||
| }, | ||
| }) | ||
| }, ERRORS.PROPERTY_NOT_FOUND) | ||
| }) |
There was a problem hiding this comment.
Misleading test name: error is PROPERTY_NOT_FOUND, not "no app".
The test sends a nonexistent app.id but asserts ERRORS.PROPERTY_NOT_FOUND. The name "should throw error if no app with provided id" suggests app-existence validation, but the service actually fails on the B2CAppProperty lookup. Consider renaming to something like 'should throw error if no b2cappproperty found for provided app id' to match the actual behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.test.js` around
lines 86 - 99, Rename the test to accurately reflect the failure source: update
the test title that currently reads "should throw error if no app with provided
id" to something like "should throw error if no B2CAppProperty found for
provided app id" (or similar) so it aligns with the asserted error
ERRORS.PROPERTY_NOT_FOUND; locate the test using
sendVoIPStartMessageByTestClient and the assertion against
ERRORS.PROPERTY_NOT_FOUND and change only the description string, leaving the
test body and assertions (including references to B2CAppProperty lookup
behavior) unchanged.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.js (1)
92-94: Use the standardentityfield in this log record.
entityNameis outside the repo logging schema, so consumers keyed onentitywill miss these entries.Suggested fix
const logInfo = ({ b2cAppId, callId, stats, errors }) => { - logger.info({ msg: `${SERVICE_NAME} stats`, entityName: 'B2CApp', entityId: b2cAppId, data: { callId, stats }, err: errors }) + logger.info({ msg: `${SERVICE_NAME} stats`, entity: 'B2CApp', entityId: b2cAppId, data: { callId, stats }, err: errors }) }As per coding guidelines,
Use standard logging fields: msg, data, entityId, entity, count, status, and err instead of custom field names.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.js` around lines 92 - 94, The log record in the logInfo function uses a non-standard field entityName; update logger.info call in logInfo to use the standard fields: msg, data, entityId, entity, and err (e.g., set entity to 'B2CApp'), keeping msg as `${SERVICE_NAME} stats` and preserving data: { callId, stats } and err: errors so consumers keyed on entity will surface these entries correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.js`:
- Around line 382-390: When building preparedDataArgs in the
SendVoIPStartMessageService code path that handles callData.nativeCallData, add
an explicit check for callData.nativeCallData.voipPanels?.length > 0 and only
read voipPanels[0].dtfmCommand (and include voipPanels in preparedDataArgs) when
that length check passes; do not blindly access voipPanels?.[0] or include an
empty voipPanels array — alternatively reject/throw a validation error when
voipPanels is an empty array. Update the assignment that sets voipDtfmCommand
and voipPanels (the block that also sets voipType using
MAGIC_VOIP_TYPE_CONSTANT_FOR_OLD_VERSIONS_COMPATIBILITY) to guard on the
non-empty condition.
- Around line 291-296: Remove the manual "deletedAt: null" filters so the
soft-delete plugin can apply automatic filtering; specifically, in the find call
that sets residentsOnUnit (the find('Resident', { unitName_i, unitType,
deletedAt: null, property: { id_in: [...] } }) created from
verifiedContactsOnUnit) remove the root deletedAt: null and do the same for the
other similar find/query at the indicated block around lines 335-337—do not add
deletedAt to nested property or b2cApp filters either, let the schema plugin
handle all soft-delete filtering.
- Around line 412-415: The code records any sendMessage(...) result id into
startingMessagesIdsByUserIds[resident.user] but only calls setCallStatus(...)
when a message was treated as a fresh success, which means duplicate-only
retries won't persist the call status; change the logic so that whenever a
message id is available from sendMessage (including duplicates) you
persist/update the call status (call setCallStatus with the appropriate
resident/user and message id) instead of gating it only on "fresh" success —
update both the block around sendMessage -> startingMessagesIdsByUserIds and the
similar block at lines 453-463 to always persist status when res?.id is present.
---
Nitpick comments:
In `@apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.js`:
- Around line 92-94: The log record in the logInfo function uses a non-standard
field entityName; update logger.info call in logInfo to use the standard fields:
msg, data, entityId, entity, and err (e.g., set entity to 'B2CApp'), keeping msg
as `${SERVICE_NAME} stats` and preserving data: { callId, stats } and err:
errors so consumers keyed on entity will surface these entries correctly.
🪄 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: ac973bb0-fcba-4c6c-9134-6394b83a36e6
📒 Files selected for processing (4)
apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.jsapps/condo/lang/en/en.jsonapps/condo/lang/es/es.jsonapps/condo/lang/ru/ru.json
✅ Files skipped from review due to trivial changes (2)
- apps/condo/lang/ru/ru.json
- apps/condo/lang/en/en.json
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/condo/lang/es/es.json
| if (callData.nativeCallData) { | ||
| preparedDataArgs = { | ||
| ...preparedDataArgs, | ||
| voipType: MAGIC_VOIP_TYPE_CONSTANT_FOR_OLD_VERSIONS_COMPATIBILITY, | ||
| voipAddress: callData.nativeCallData.voipAddress, | ||
| voipLogin: callData.nativeCallData.voipLogin, | ||
| voipPassword: callData.nativeCallData.voipPassword, | ||
| voipDtfmCommand: callData.nativeCallData.voipPanels?.[0]?.dtfmCommand, | ||
| voipPanels: callData.nativeCallData.voipPanels, |
There was a problem hiding this comment.
Reject empty voipPanels before reading the first panel.
This path always uses the first panel's dtfmCommand. If a client sends voipPanels: [], the mutation still builds nativeCallData, but the payload is incomplete because the main panel command becomes undefined. Add an explicit length check before building preparedDataArgs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.js` around
lines 382 - 390, When building preparedDataArgs in the
SendVoIPStartMessageService code path that handles callData.nativeCallData, add
an explicit check for callData.nativeCallData.voipPanels?.length > 0 and only
read voipPanels[0].dtfmCommand (and include voipPanels in preparedDataArgs) when
that length check passes; do not blindly access voipPanels?.[0] or include an
empty voipPanels array — alternatively reject/throw a validation error when
voipPanels is an empty array. Update the assignment that sets voipDtfmCommand
and voipPanels (the block that also sets voipType using
MAGIC_VOIP_TYPE_CONSTANT_FOR_OLD_VERSIONS_COMPATIBILITY) to guard on the
non-empty condition.
| const res = await sendMessage(context, messageAttrs) | ||
| if (res?.id) { | ||
| startingMessagesIdsByUserIds[resident.user] = res.id | ||
| } |
There was a problem hiding this comment.
A duplicate-only retry can never restore missing call status.
Any sendMessage() result with an id is recorded here, including duplicates, but setCallStatus() only runs when at least one message is treated as success. If a previous attempt created messages and then failed before caching, the retry will likely hit only duplicates and skip the cache write again, so cancel handling stays broken. Persist the status whenever message IDs are available, not only when a fresh message was created.
Also applies to: 453-463
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.js` around
lines 412 - 415, The code records any sendMessage(...) result id into
startingMessagesIdsByUserIds[resident.user] but only calls setCallStatus(...)
when a message was treated as a fresh success, which means duplicate-only
retries won't persist the call status; change the logic so that whenever a
message id is available from sendMessage (including duplicates) you
persist/update the call status (call setCallStatus with the appropriate
resident/user and message id) instead of gating it only on "fresh" success —
update both the block around sendMessage -> startingMessagesIdsByUserIds and the
similar block at lines 453-463 to always persist status when res?.id is present.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.js (1)
408-410: Redundant filter: actors already guaranteed to have resident, contact, and user.The actors array was filtered on line 346 to only include items with
actor.resident. Earlier filtering at line 333 ensuredactor.userexists. The filter on line 409actor => !!actor.resident && !!actor.contact && !!actor.useris defensive but redundant.Suggested simplification
- const sendMessagePromises = actors - .filter(actor => !!actor.resident && !!actor.contact && !!actor.user) - .map(async ({ contact, resident, user }) => { + const sendMessagePromises = actors.map(async ({ contact, resident, user }) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.js` around lines 408 - 410, The filter in the sendMessagePromises construction is redundant because the actors array has already been filtered to guarantee resident, contact, and user; remove the .filter(actor => !!actor.resident && !!actor.contact && !!actor.user) and directly map over actors (actors.map(async ({ contact, resident, user }) => {...})), keeping the same async mapping logic so sendMessagePromises, the mapping callback, and any uses of contact/resident/user remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/condo/domains/miniapp/schema/sendVoIPStartMessageService.spec.js`:
- Around line 273-280: The test duplicates the same assertion on
b2cAppVoIPTypeMessages (expect(b2cAppVoIPTypeMessages).toHaveLength(1)) twice;
remove the redundant first assertion (the standalone expect) and keep the single
assertion inside the try-catch (or conversely remove the try-catch and keep one
assert), so only one expect(b2cAppVoIPTypeMessages).toHaveLength(1) remains;
update any surrounding comments/logging accordingly to avoid duplicate checks in
the test for b2cAppVoIPTypeMessages.
---
Nitpick comments:
In `@apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.js`:
- Around line 408-410: The filter in the sendMessagePromises construction is
redundant because the actors array has already been filtered to guarantee
resident, contact, and user; remove the .filter(actor => !!actor.resident &&
!!actor.contact && !!actor.user) and directly map over actors (actors.map(async
({ contact, resident, user }) => {...})), keeping the same async mapping logic
so sendMessagePromises, the mapping callback, and any uses of
contact/resident/user remain unchanged.
🪄 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: 06def00b-86ea-42c4-acfb-d10dc7311700
📒 Files selected for processing (6)
apps/condo/domains/miniapp/constants.jsapps/condo/domains/miniapp/schema/SendVoIPStartMessageService.jsapps/condo/domains/miniapp/schema/SendVoIPStartMessageService.test.jsapps/condo/domains/miniapp/schema/sendVoIPStartMessageService.spec.jsapps/condo/domains/notification/constants/constants.jsapps/condo/schema.graphql
✅ Files skipped from review due to trivial changes (2)
- apps/condo/domains/notification/constants/constants.js
- apps/condo/domains/miniapp/schema/SendVoIPStartMessageService.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/condo/domains/miniapp/constants.js
| const b2cAppVoIPTypeMessages = createdMessages.filter(msg => !!msg.meta.data.B2CAppContext) | ||
| expect(b2cAppVoIPTypeMessages).toHaveLength(1) | ||
| try { | ||
| expect(b2cAppVoIPTypeMessages).toHaveLength(1) | ||
| } catch (err) { | ||
| console.error('b2cAppVoIPTypeMessages fail') | ||
| throw err | ||
| } |
There was a problem hiding this comment.
Duplicate assertion: toHaveLength(1) is checked twice.
Line 274 and line 276 both assert expect(b2cAppVoIPTypeMessages).toHaveLength(1). The first assertion (line 274) is outside the try-catch, making the one inside (line 276) redundant.
Suggested fix
const b2cAppVoIPTypeMessages = createdMessages.filter(msg => !!msg.meta.data.B2CAppContext)
- expect(b2cAppVoIPTypeMessages).toHaveLength(1)
try {
expect(b2cAppVoIPTypeMessages).toHaveLength(1)
} catch (err) {
console.error('b2cAppVoIPTypeMessages fail')
throw err
}📝 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 b2cAppVoIPTypeMessages = createdMessages.filter(msg => !!msg.meta.data.B2CAppContext) | |
| expect(b2cAppVoIPTypeMessages).toHaveLength(1) | |
| try { | |
| expect(b2cAppVoIPTypeMessages).toHaveLength(1) | |
| } catch (err) { | |
| console.error('b2cAppVoIPTypeMessages fail') | |
| throw err | |
| } | |
| const b2cAppVoIPTypeMessages = createdMessages.filter(msg => !!msg.meta.data.B2CAppContext) | |
| try { | |
| expect(b2cAppVoIPTypeMessages).toHaveLength(1) | |
| } catch (err) { | |
| console.error('b2cAppVoIPTypeMessages fail') | |
| throw err | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/condo/domains/miniapp/schema/sendVoIPStartMessageService.spec.js` around
lines 273 - 280, The test duplicates the same assertion on
b2cAppVoIPTypeMessages (expect(b2cAppVoIPTypeMessages).toHaveLength(1)) twice;
remove the redundant first assertion (the standalone expect) and keep the single
assertion inside the try-catch (or conversely remove the try-catch and keep one
assert), so only one expect(b2cAppVoIPTypeMessages).toHaveLength(1) remains;
update any surrounding comments/logging accordingly to avoid duplicate checks in
the test for b2cAppVoIPTypeMessages.
|
| if (user.deletedAt) return false | ||
| if (user.isAdmin) return true | ||
|
|
||
| if (user.type === SERVICE) { |
There was a problem hiding this comment.
I guess its time to rebase?)
| * If debug app is set and debug app settings are configured, then user can send push messages without creating B2CApp first. | ||
| * This is useful for testing and development, but it should be turned off on production | ||
| */ | ||
| const DEBUG_APP_ID = conf.MINIAPP_PUSH_MESSAGE_DEBUG_APP_ID |
There was a problem hiding this comment.
should it be here? I thought it was some legacy or smth
There was a problem hiding this comment.
I don't exactly know, looks like it is being used for local development of miniapps
| const DEBUG_APP_ID = conf.MINIAPP_PUSH_MESSAGE_DEBUG_APP_ID | ||
| const DEBUG_APP_ENABLED = !!DEBUG_APP_ID | ||
| const DEBUG_APP_SETTINGS = DEBUG_APP_ENABLED ? Object.freeze(JSON.parse(conf.MINIAPP_PUSH_MESSAGE_DEBUG_APP_SETTINGS)) : {} | ||
| const ALLOWED_UNIT_TYPES = [FLAT_UNIT_TYPE, APARTMENT_UNIT_TYPE] |
There was a problem hiding this comment.
why COMMERCIAL_UNIT_TYPE is not allowed?
There was a problem hiding this comment.
why do you even want to restrict it?
There was a problem hiding this comment.
I remember we had problems with other systems as they would send voip push for (f.e.) parking lot, which is wrong. So we made a restriction for these unit types in them
There was a problem hiding this comment.
Ok now I know that we don't have that restriction product-wise, just external errors related to it. So I will allow all unitTypes
| const DEBUG_APP_ENABLED = !!DEBUG_APP_ID | ||
| const DEBUG_APP_SETTINGS = DEBUG_APP_ENABLED ? Object.freeze(JSON.parse(conf.MINIAPP_PUSH_MESSAGE_DEBUG_APP_SETTINGS)) : {} | ||
| const ALLOWED_UNIT_TYPES = [FLAT_UNIT_TYPE, APARTMENT_UNIT_TYPE] | ||
| const VOIP_TYPE_CUSTOM_FIELD_ID = conf.VOIP_TYPE_CUSTOM_FIELD_ID |
There was a problem hiding this comment.
I thought we'll do something like this:
const customValues = await CustomValue.getAll(where: { field: { name_starts: "voip" } })
const overrides = { ... } // converts customValues to { voipType: "...", [key]: value }
There was a problem hiding this comment.
There is no uniqueness in the name, so it looks even more strange?
Like, voipType depends on it's value, which can be anything with zero arrangements
| const SERVICE_NAME = 'sendVoIPStartMessage' | ||
| const ERRORS = { | ||
| PROPERTY_NOT_FOUND: { | ||
| query: SERVICE_NAME, |
There was a problem hiding this comment.
| query: SERVICE_NAME, | |
| mutation: SERVICE_NAME, |
| const B2CAPP_VOIP_TYPE = 'b2cApp' | ||
|
|
||
| function logInfo ({ b2cAppId, callId, stats, errors }) { | ||
| logger.info({ msg: `${SERVICE_NAME} stats`, entityName: 'B2CApp', entityId: b2cAppId, data: { callId, stats }, err: errors }) |
There was a problem hiding this comment.
I thought err is a single field 🤔
| access: true, | ||
| type: `input VoIPPanel { | ||
| """ | ||
| Dtfm command for panel |
There was a problem hiding this comment.
| Dtfm command for panel | |
| Dtfm command used to open the panel |
| """ | ||
| Name of a panel to be displayed | ||
| """ | ||
| name: String! |
There was a problem hiding this comment.
Should it be optional, I don't think we have it in all cases
| types: [ | ||
| { | ||
| access: true, | ||
| type: `input VoIPPanel { |
There was a problem hiding this comment.
It's better to keep naming a bit more detailed, in case we'll decided to add VoipPanel as model later.
Maybe something like:
| type: `input VoIPPanel { | |
| type: `input VoIPPanelParamters { |
There was a problem hiding this comment.
why its not prefixed with service name like other inputs?
| b2cAppCallData: SendVoIPStartMessageDataForCallHandlingByB2CApp, | ||
| """ | ||
| If you want mobile app to handle call (without your B2CApp), provide this argument. If "b2cAppCallData" and "nativeCallData" are provided together, native call is prioritized. | ||
| """ | ||
| nativeCallData: SendVoIPStartMessageDataForCallHandlingByNative |
There was a problem hiding this comment.
Hm, I'm not sure about this separation. Does it mean I cannot fill both or?
There was a problem hiding this comment.
In this part that you specified comment says that you can fill both.
We have different validation (required fields etc) for b2c data and for native call data. And there is 2 ways:
- Lets allow everything as in sendB2CAppPushMessage, and throw a bunch of errors
- Lets make fields required in schema, so caller can't miss
I guess 2) is better, thats why I separate them
| // 1) Check B2CApp and B2CAppProperty | ||
| const b2cAppId = app.id | ||
|
|
||
| const [b2cAppProperty] = await B2CAppProperty.getAll(context, { |
| } | ||
|
|
||
| // NOTE(YEgorLu): doing same as Resident.isVerifiedByManagingCompany virtual field | ||
| const usersOfContacts = await find('User', { |
There was a problem hiding this comment.
I don't get it, why not just fetching residents on address?
Btw, fetching users by phone seems strange, you have ids from residents, why phones
There was a problem hiding this comment.
I don't need ALL residents. I need only VERIFIED residents.
Verified means that I must find relations Resident <-> Contact
They relate only with user: Resident.user <-> User.id, User.phone <-> Contact.phone, Resident.address <-> Contact.address.
It can be started from Contacts, it can be started from Residents, but anyway I must in the end relate one to other using phone number and User.
| if (!DEBUG_APP_ENABLED || b2cAppId !== DEBUG_APP_ID) { | ||
| appSettings = await getByCondition('AppMessageSetting', { | ||
| b2cApp: { id: b2cAppId }, type: VOIP_INCOMING_CALL_MESSAGE_TYPE, deletedAt: null, | ||
| }) | ||
| } | ||
|
|
||
| else { | ||
| appSettings = { ...DEBUG_APP_SETTINGS } | ||
| } |
There was a problem hiding this comment.
- why else is so separated
- seems like duplication of legacy code, do we need it? maybe move it to util?
There was a problem hiding this comment.
Yeah it Ctrl C + Ctrl V.
It looks as it is needed for mobile app local development, tough I will recheck this
|
|
||
| const ACCESS_TOKEN_UPDATE_MANY_CHUNK_SIZE = 500 | ||
|
|
||
| const MAGIC_VOIP_TYPE_CONSTANT_FOR_OLD_VERSIONS_COMPATIBILITY = 'sip' // without this constant mobile app will not try to make native call |
There was a problem hiding this comment.
Yo, we dont name our constants like this...There's no magic in IT :)
There was a problem hiding this comment.
It's literally magic bs which does something unrelated to it's value
| const ACCESS_TOKEN_UPDATE_MANY_CHUNK_SIZE = 500 | ||
|
|
||
| const MAGIC_VOIP_TYPE_CONSTANT_FOR_OLD_VERSIONS_COMPATIBILITY = 'sip' // without this constant mobile app will not try to make native call | ||
| const DEFAULT_VOIP_TYPE = 0 |
There was a problem hiding this comment.
what's this magic number?
There was a problem hiding this comment.
So we have voipType.
Right now supported values are: 'sip' | anything other than 'sip', which means 2 clients
Now we are going to add N clients. Each app can treat client ('voipType') differently as it chooses. So giving voipType a meaningful name looks useless, same value will be meaningful for one app and random string for the other.
So I decided that voipType must just be a number. As all data in pushes it will be stringified anyway, so it is string with a number in it. So the default is 0, and we treat it like an old one (being replaced to 'sip')
3 participants
Summary by CodeRabbit
New Features
Documentation
Tests
Chores