-
-
Notifications
You must be signed in to change notification settings - Fork 38
feat: create and update recipients in firebase auth store #1321
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughMake PaymentInformation.phone mandatory in Prisma and DB; add FirebaseService to manage Auth users by phone and call it from recipient create/update flows; always emit phone.create in transformers/helpers and enforce strict E.164 validation on the frontend; add SQL migration to set payment_information.phone_id NOT NULL. Changes
Sequence Diagram(s)sequenceDiagram
participant FE as Frontend
participant RS as RecipientService
participant FS as FirebaseService
participant FA as Firebase Auth
participant DB as Database
rect rgb(240,248,255)
Note over FE,DB: Create Recipient (payment phone always emitted)
FE->>RS: createRecipient(payload with paymentInformation.phone.create.number)
RS->>FS: createByPhoneNumber(phoneNumber)
FS->>FA: getUserByPhone(phoneNumber)
alt user exists
FA-->>FS: user
else not found
FS->>FA: createUser(phoneNumber)
FA-->>FS: created user
end
FS-->>RS: user
RS->>DB: create recipient + paymentInformation + phone
DB-->>RS: created
RS-->>FE: created recipient
end
rect rgb(240,248,255)
Note over FE,DB: Update Recipient (phone change path)
FE->>RS: updateRecipient(payload with phone)
RS->>DB: fetch existing (incl. paymentInformation.phone.number)
DB-->>RS: existing record
RS->>FS: updateByPhoneNumber(oldPhone, newPhone)
FS->>FA: getUserByPhone(oldPhone)
FA-->>FS: user
FS->>FA: updateUserPhone(userId, newPhone)
FA-->>FS: updated user
FS-->>RS: user
RS->>DB: upsert paymentInformation + phone, update recipient
DB-->>RS: updated
RS-->>FE: updated recipient
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
firestore_to_postgres/src/recipient/recipient.transformer.ts (1)
31-46: Empty-string phone create will break importWhen
raw.mobile_money_phone?.phoneis missing you now create aPhonewithnumber: ''. Thephone.numbercolumn is unique and downstream code (Firebase Auth + E.164 validation) requires a real number, so repeated legacy rows without a phone will crash the import or violate constraints. Instead, skip these recipients (or surface an error) and keep the relation optional at load time. Example fix:- const paymentPhone = raw.mobile_money_phone?.phone - ? { /* existing connectOrCreate */ } - : { - create: { - number: '', - hasWhatsApp: false, - }, - }; + if (!raw.mobile_money_phone?.phone) { + skipped++; + console.warn(`⚠️ Skipping recipient ${raw.id} because payment phone is missing`); + continue; + } + + const paymentPhone = { + connectOrCreate: { + where: { number: raw.mobile_money_phone.phone.toString() }, + create: { + number: raw.mobile_money_phone.phone.toString(), + hasWhatsApp: raw.mobile_money_phone.has_whatsapp, + }, + }, + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
firestore_to_postgres/src/recipient/recipient.transformer.ts(1 hunks)shared/src/database/migrations/20251031122446_make_payment_information_phone_mandatory/migration.sql(1 hunks)shared/src/database/schema.prisma(1 hunks)shared/src/database/services/recipient/recipient.service.ts(5 hunks)shared/src/firebase/services/auth.service.ts(1 hunks)website/src/app/portal/components/data-table/clients/recipients-table-client.tsx(1 hunks)website/src/app/portal/components/forms/recipient-form.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: MDikkii
Repo: socialincome-san/public PR: 960
File: recipients_app/lib/data/datasource/remote/user_remote_data_source.dart:87-91
Timestamp: 2025-01-06T18:59:26.103Z
Learning: The `Recipient` model enforces a non-nullable `userId` property, so no null checks are necessary there.
📚 Learning: 2024-11-29T06:09:31.569Z
Learnt from: activus-d
Repo: socialincome-san/public PR: 926
File: website/src/app/[lang]/[region]/(website)/open-source/(sections)/contributors.tsx:10-25
Timestamp: 2024-11-29T06:09:31.569Z
Learning: In `contributors.tsx`, additional error handling and loading states in the `OpenSourceContributors` component are not required.
Applied to files:
website/src/app/portal/components/data-table/clients/recipients-table-client.tsx
📚 Learning: 2024-12-16T04:00:28.526Z
Learnt from: activus-d
Repo: socialincome-san/public PR: 985
File: website/src/app/[lang]/[region]/(website)/open-source/(components)/fetch-data.ts:14-26
Timestamp: 2024-12-16T04:00:28.526Z
Learning: In 'website/src/app/[lang]/[region]/(website)/open-source/(components)/fetch-data.ts', avoid suggesting inclusion of rate limit information in the error messages within the 'fetchData' function.
Applied to files:
website/src/app/portal/components/data-table/clients/recipients-table-client.tsx
📚 Learning: 2025-01-06T18:59:26.103Z
Learnt from: MDikkii
Repo: socialincome-san/public PR: 960
File: recipients_app/lib/data/datasource/remote/user_remote_data_source.dart:87-91
Timestamp: 2025-01-06T18:59:26.103Z
Learning: The `Recipient` model enforces a non-nullable `userId` property, so no null checks are necessary there.
Applied to files:
shared/src/database/services/recipient/recipient.service.ts
🧬 Code graph analysis (2)
shared/src/database/services/recipient/recipient.service.ts (1)
shared/src/firebase/services/auth.service.ts (1)
AuthService(16-56)
shared/src/firebase/services/auth.service.ts (2)
shared/src/firebase/admin/AuthAdmin.ts (1)
AuthAdmin(5-15)shared/src/firebase/admin/app.ts (1)
getOrInitializeFirebaseAdmin(7-10)
| ALTER TABLE "payment_information" ALTER COLUMN "phone_id" SET NOT NULL; | ||
|
|
||
| -- AddForeignKey | ||
| ALTER TABLE "payment_information" ADD CONSTRAINT "payment_information_phone_id_fkey" FOREIGN KEY ("phone_id") REFERENCES "phone"("id") ON DELETE RESTRICT ON UPDATE CASCADE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Backfill payment_information.phone_id before enforcing NOT NULL
If any existing payment_information row still has phone_id = NULL, this ALTER COLUMN ... SET NOT NULL will abort the migration. Please add a data backfill (or a guard that blocks the deployment earlier) so the constraint can be applied safely.
🤖 Prompt for AI Agents
In
shared/src/database/migrations/20251031122446_make_payment_information_phone_mandatory/migration.sql
around lines 11 to 14, the migration sets payment_information.phone_id NOT NULL
but doesn't backfill existing NULL values; add a safe data backfill before the
ALTER COLUMN so the migration won't fail: within the migration create or select
an appropriate fallback phone (or require a pre-check) and run an UPDATE
payment_information SET phone_id = <fallback_phone_id> WHERE phone_id IS NULL
(or insert phones and use their ids), then apply ALTER COLUMN ... SET NOT NULL
and add the foreign key; also include a reversible down-step (or guard) so
rollback restores the previous state (e.g., NULLs or drops the constraint).
|
Visit the preview URL for this PR (updated for commit 145273a): https://si-admin-staging--pr1321-feat-recipients-auth-rprpadja.web.app (expires Mon, 10 Nov 2025 13:24:03 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: b7b0969384059dce6ea8fad1ee1d1737e54e6676 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
shared/src/database/services/recipient/recipient.service.ts(5 hunks)website/src/app/portal/components/data-table/clients/recipients-table-client.tsx(1 hunks)website/src/app/portal/components/forms/recipient/recipient-form-helpers.ts(2 hunks)website/src/app/portal/components/forms/recipient/recipient-form.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-01-06T18:59:26.103Z
Learnt from: MDikkii
Repo: socialincome-san/public PR: 960
File: recipients_app/lib/data/datasource/remote/user_remote_data_source.dart:87-91
Timestamp: 2025-01-06T18:59:26.103Z
Learning: The `Recipient` model enforces a non-nullable `userId` property, so no null checks are necessary there.
Applied to files:
shared/src/database/services/recipient/recipient.service.ts
📚 Learning: 2024-11-29T06:09:31.569Z
Learnt from: activus-d
Repo: socialincome-san/public PR: 926
File: website/src/app/[lang]/[region]/(website)/open-source/(sections)/contributors.tsx:10-25
Timestamp: 2024-11-29T06:09:31.569Z
Learning: In `contributors.tsx`, additional error handling and loading states in the `OpenSourceContributors` component are not required.
Applied to files:
website/src/app/portal/components/data-table/clients/recipients-table-client.tsx
🧬 Code graph analysis (1)
shared/src/database/services/recipient/recipient.service.ts (1)
shared/src/firebase/services/auth.service.ts (1)
AuthService(16-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Deploy admin on Firebase (social-income-staging) / Deploy admin on Firebase
- GitHub Check: Test website
🔇 Additional comments (2)
website/src/app/portal/components/data-table/clients/recipients-table-client.tsx (1)
41-44: Log label alignment looks good.Renaming the console message keeps diagnostics consistent with the recipient form flow.
website/src/app/portal/components/forms/recipient/recipient-form.tsx (1)
101-104: E.164 validation update is on point.The stricter regex ensures we only pass Firebase-compatible numbers while giving users a precise hint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, 1 little nit, rest looks awesome. 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
website/src/app/portal/components/forms/recipient/recipient-form-helpers.ts (1)
94-94: Same issue: unconditional create with potentially undefined value.Identical to Line 53—if
paymentInfoFields.phone.valueis undefined, the backend will reject the request. Apply the same conditional check here.- phone: { create: { number: paymentInfoFields.phone.value } }, + phone: paymentInfoFields.phone.value + ? { create: { number: paymentInfoFields.phone.value } } + : undefined,
🧹 Nitpick comments (2)
shared/src/database/services/recipient/recipient.service.ts (2)
32-37: Firebase user creation return value discarded.
createByPhoneNumberreturns aUserRecordcontaining the Firebase UID. If you need to store or reference the Firebase UID in your database later, capture the return value now. Otherwise, you'll have to query Firebase again.
73-77: Consider capturing Firebase UID and optimizing no-change case.Similar to the create flow, the
updateByPhoneNumberreturn value (containing the Firebase UID) is discarded. Additionally, if the phone number hasn't changed (oldPhoneNumber === phoneNumber), you could skip the Firebase call entirely.+ if (existing.paymentInformation.phone.number === phoneNumber) { + // Phone unchanged, skip Firebase update + } else { await this.firebaseAuthService.updateByPhoneNumber(existing.paymentInformation.phone.number, phoneNumber); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
shared/src/database/services/recipient/recipient.service.ts(5 hunks)shared/src/firebase/services/auth.service.ts(1 hunks)website/src/app/portal/components/forms/recipient/recipient-form-helpers.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- shared/src/firebase/services/auth.service.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-01-06T18:59:26.103Z
Learnt from: MDikkii
Repo: socialincome-san/public PR: 960
File: recipients_app/lib/data/datasource/remote/user_remote_data_source.dart:87-91
Timestamp: 2025-01-06T18:59:26.103Z
Learning: The `Recipient` model enforces a non-nullable `userId` property, so no null checks are necessary there.
Applied to files:
shared/src/database/services/recipient/recipient.service.ts
🧬 Code graph analysis (1)
shared/src/database/services/recipient/recipient.service.ts (1)
shared/src/firebase/services/auth.service.ts (1)
FirebaseService(16-59)
🔇 Additional comments (1)
shared/src/database/services/recipient/recipient.service.ts (1)
55-55: LGTM: Select properly includes existing phone number.This change correctly fetches the existing phone number needed for the Firebase update call on Line 77.
| const phoneNumber = | ||
| recipient.paymentInformation?.upsert?.update?.phone?.upsert?.update.number?.toString() || | ||
| recipient.paymentInformation?.upsert?.create?.phone?.create?.number?.toString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unnecessary .toString() calls.
Phone numbers are already strings. Calling .toString() on null or undefined produces the literal strings "null" or "undefined", which could bypass validation and cause subtle bugs downstream.
const phoneNumber =
- recipient.paymentInformation?.upsert?.update?.phone?.upsert?.update.number?.toString() ||
- recipient.paymentInformation?.upsert?.create?.phone?.create?.number?.toString();
+ recipient.paymentInformation?.upsert?.update?.phone?.upsert?.update.number ||
+ recipient.paymentInformation?.upsert?.create?.phone?.create?.number;📝 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 phoneNumber = | |
| recipient.paymentInformation?.upsert?.update?.phone?.upsert?.update.number?.toString() || | |
| recipient.paymentInformation?.upsert?.create?.phone?.create?.number?.toString(); | |
| const phoneNumber = | |
| recipient.paymentInformation?.upsert?.update?.phone?.upsert?.update.number || | |
| recipient.paymentInformation?.upsert?.create?.phone?.create?.number; |
🤖 Prompt for AI Agents
In shared/src/database/services/recipient/recipient.service.ts around lines 69
to 71, remove the unnecessary .toString() calls on the phone number accessors;
instead read the number directly via the optional-chained properties so you get
either the original string or undefined (avoiding literal "null"/"undefined").
Update the expression to return the direct property (no .toString()), and if
downstream code requires a guaranteed string, perform an explicit, safe
validation or normalization step (e.g., check for a non-empty string) rather
than calling .toString().
| create: { | ||
| ...basePaymentInfo, | ||
| phone: paymentInfoFields.phone.value ? { create: { number: paymentInfoFields.phone.value } } : undefined, | ||
| phone: { create: { number: paymentInfoFields.phone.value } }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unconditional create may pass undefined phone number.
If paymentInfoFields.phone.value is undefined, Prisma receives { create: { number: undefined } }. Since phone is now mandatory, this will fail backend validation. Consider adding a conditional check or ensuring frontend validation prevents submission without a phone number.
- phone: { create: { number: paymentInfoFields.phone.value } },
+ phone: paymentInfoFields.phone.value
+ ? { create: { number: paymentInfoFields.phone.value } }
+ : undefined,📝 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.
| phone: { create: { number: paymentInfoFields.phone.value } }, | |
| phone: paymentInfoFields.phone.value | |
| ? { create: { number: paymentInfoFields.phone.value } } | |
| : undefined, |
🤖 Prompt for AI Agents
In website/src/app/portal/components/forms/recipient/recipient-form-helpers.ts
around line 53, the code unconditionally sets phone: { create: { number:
paymentInfoFields.phone.value } } which can pass number: undefined to Prisma and
break backend validation; change this to only include the phone create object
when paymentInfoFields.phone.value is non-empty (e.g., conditionally add the
phone property or use a conditional spread so phone is omitted when undefined),
or alternatively enforce frontend validation to prevent submission without a
phone before building this object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
shared/src/database/services/recipient/recipient.service.ts(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-01-06T18:59:26.103Z
Learnt from: MDikkii
Repo: socialincome-san/public PR: 960
File: recipients_app/lib/data/datasource/remote/user_remote_data_source.dart:87-91
Timestamp: 2025-01-06T18:59:26.103Z
Learning: The `Recipient` model enforces a non-nullable `userId` property, so no null checks are necessary there.
Applied to files:
shared/src/database/services/recipient/recipient.service.ts
🧬 Code graph analysis (1)
shared/src/database/services/recipient/recipient.service.ts (1)
shared/src/firebase/services/auth.service.ts (1)
FirebaseService(16-59)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test website
- GitHub Check: Test functions
- GitHub Check: Test admin
- GitHub Check: Test shared code
- GitHub Check: Prettify
| const phoneNumber = | ||
| recipient.paymentInformation?.upsert?.update?.phone?.upsert?.update.number?.toString() || | ||
| recipient.paymentInformation?.upsert?.create?.phone?.create?.number?.toString(); | ||
|
|
||
| if (!phoneNumber || !existing.paymentInformation?.phone?.number) { | ||
| return this.resultFail('No phone number available for recipient update'); | ||
| } | ||
|
|
||
| await this.firebaseAuthService.updateByPhoneNumber(existing.paymentInformation?.phone.number, phoneNumber); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blocker: allow recipient updates when phone stays untouched.
phoneNumber is undefined whenever the update payload doesn’t modify paymentInformation.phone, so the new guard immediately returns resultFail(...). That breaks every update (status changes, notes, etc.) that leaves the phone alone. Please only hit Firebase when a new number is supplied, and otherwise continue with the DB update.
- const phoneNumber =
- recipient.paymentInformation?.upsert?.update?.phone?.upsert?.update.number?.toString() ||
- recipient.paymentInformation?.upsert?.create?.phone?.create?.number?.toString();
-
- if (!phoneNumber || !existing.paymentInformation?.phone?.number) {
- return this.resultFail('No phone number available for recipient update');
- }
-
- await this.firebaseAuthService.updateByPhoneNumber(existing.paymentInformation?.phone.number, phoneNumber);
+ const newPhoneNumber =
+ recipient.paymentInformation?.upsert?.update?.phone?.upsert?.update.number ||
+ recipient.paymentInformation?.upsert?.create?.phone?.create?.number;
+ const existingPhoneNumber = existing.paymentInformation?.phone?.number;
+
+ if (newPhoneNumber) {
+ if (!existingPhoneNumber) {
+ return this.resultFail('No phone number available for recipient update');
+ }
+ await this.firebaseAuthService.updateByPhoneNumber(existingPhoneNumber, newPhoneNumber);
+ }📝 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 phoneNumber = | |
| recipient.paymentInformation?.upsert?.update?.phone?.upsert?.update.number?.toString() || | |
| recipient.paymentInformation?.upsert?.create?.phone?.create?.number?.toString(); | |
| if (!phoneNumber || !existing.paymentInformation?.phone?.number) { | |
| return this.resultFail('No phone number available for recipient update'); | |
| } | |
| await this.firebaseAuthService.updateByPhoneNumber(existing.paymentInformation?.phone.number, phoneNumber); | |
| const newPhoneNumber = | |
| recipient.paymentInformation?.upsert?.update?.phone?.upsert?.update.number || | |
| recipient.paymentInformation?.upsert?.create?.phone?.create?.number; | |
| const existingPhoneNumber = existing.paymentInformation?.phone?.number; | |
| if (newPhoneNumber) { | |
| if (!existingPhoneNumber) { | |
| return this.resultFail('No phone number available for recipient update'); | |
| } | |
| await this.firebaseAuthService.updateByPhoneNumber(existingPhoneNumber, newPhoneNumber); | |
| } |
🤖 Prompt for AI Agents
In shared/src/database/services/recipient/recipient.service.ts around lines
70-78, the current guard returns resultFail when phoneNumber is undefined which
blocks updates that don't change the phone; change the logic to NOT return when
phoneNumber is undefined and only call firebaseAuthService.updateByPhoneNumber
if a new phoneNumber is supplied and it is different from
existing.paymentInformation?.phone?.number (i.e., if phoneNumber && phoneNumber
!== existing.paymentInformation?.phone?.number). If phoneNumber is undefined,
proceed with the database update path; if phoneNumber is provided and existing
number is missing or different, call firebaseAuthService.updateByPhoneNumber
with the existing number (if any) and the new phoneNumber, otherwise skip the
Firebase call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
firestore_to_postgres/src/recipient/recipient.transformer.ts (1)
127-130: Helper is correct but duplicates logic in payout transformer.The E.164 formatting logic is sound. However, this duplicates the same formatting in
payout.transformer.tsline 24. Consider extracting to a shared utility as noted in the payout transformer review.firestore_to_postgres/src/payout/payout.transformer.ts (1)
24-24: Extract duplicate E.164 phone formatting to a shared utility function.The phone formatting logic (
+${phoneNumber}) is duplicated: payout.transformer.ts applies it inline (line 24), while recipient.transformer.ts encapsulates it in amapPhoneNumbermethod (lines 127–130). The template literal safely coerces both string and number types, so removing.toString()poses no type issues. However, consolidating this logic into a shared utility would eliminate duplication and improve maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
firestore_to_postgres/src/payout/payout.transformer.ts(1 hunks)firestore_to_postgres/src/recipient/recipient.transformer.ts(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Security checks (typescript)
- GitHub Check: Prettify
🔇 Additional comments (1)
firestore_to_postgres/src/recipient/recipient.transformer.ts (1)
19-29: LGTM - Proper E.164 formatting for optional communication phone.The helper correctly formats phone numbers to E.164 and appropriately returns
undefinedwhen no phone exists.
| const paymentPhone = raw.mobile_money_phone?.phone | ||
| ? { | ||
| connectOrCreate: { | ||
| where: { number: raw.mobile_money_phone.phone.toString() }, | ||
| where: { number: this.mapPhoneNumber(raw.mobile_money_phone.phone) }, | ||
| create: { | ||
| number: raw.mobile_money_phone.phone.toString(), | ||
| number: this.mapPhoneNumber(raw.mobile_money_phone.phone), | ||
| hasWhatsApp: raw.mobile_money_phone.has_whatsapp, | ||
| }, | ||
| }, | ||
| } | ||
| : undefined; | ||
| : { | ||
| create: { | ||
| number: '', | ||
| hasWhatsApp: false, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Empty string violates E.164 format and Firebase Auth requirements.
Creating a phone record with number: '' when no phone exists violates the E.164 format requirement stated in the PR objectives. Firebase Auth will reject empty phone numbers, causing the auth sync to fail.
If phone is now mandatory for PaymentInformation, the transformer should either:
- Skip recipients without valid payment phone numbers
- Fail with a clear error message
Apply this diff to skip recipients without payment phones:
const paymentPhone = raw.mobile_money_phone?.phone
? {
connectOrCreate: {
where: { number: this.mapPhoneNumber(raw.mobile_money_phone.phone) },
create: {
number: this.mapPhoneNumber(raw.mobile_money_phone.phone),
hasWhatsApp: raw.mobile_money_phone.has_whatsapp,
},
},
}
- : {
- create: {
- number: '',
- hasWhatsApp: false,
- },
- };
+ : null;
+
+ if (!paymentPhone) {
+ console.log(`⚠️ Skipping recipient ${raw.id} - missing payment phone`);
+ continue;
+ }Committable suggestion skipped: line range outside the PR's diff.
Summary by CodeRabbit
New Features
Bug Fixes