Skip to content
7 changes: 6 additions & 1 deletion firestore_to_postgres/src/recipient/recipient.transformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,12 @@ export class RecipientTransformer extends BaseTransformer<FirestoreRecipientWith
},
},
}
: undefined;
: {
create: {
number: '',
hasWhatsApp: false,
},
};

transformed.push({
legacyFirestoreId: raw.id,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/*
Warnings:
- Made the column `phone_id` on table `payment_information` required. This step will fail if there are existing NULL values in that column.
*/
-- DropForeignKey
ALTER TABLE "payment_information" DROP CONSTRAINT "payment_information_phone_id_fkey";

-- AlterTable
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;
Comment on lines +11 to +14
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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).

4 changes: 2 additions & 2 deletions shared/src/database/schema.prisma
Original file line number Diff line number Diff line change
Expand Up @@ -427,8 +427,8 @@ model PaymentInformation {
id String @id @default(cuid()) @map("id")
provider PaymentProvider @map("provider")
code String @unique @map("code")
phoneId String? @map("phone_id")
phone Phone? @relation(fields: [phoneId], references: [id])
phoneId String @map("phone_id")
phone Phone @relation(fields: [phoneId], references: [id])

recipients Recipient[]

Expand Down
19 changes: 18 additions & 1 deletion shared/src/database/services/recipient/recipient.service.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ProgramPermission, Recipient, RecipientStatus } from '@prisma/client';
import { FirebaseService } from '@socialincome/shared/src/firebase/services/auth.service';
import { BaseService } from '../core/base.service';
import { ServiceResult } from '../core/base.types';
import { ProgramAccessService } from '../program-access/program-access.service';
Expand All @@ -12,6 +13,7 @@ import {

export class RecipientService extends BaseService {
private programAccessService = new ProgramAccessService();
private firebaseAuthService = new FirebaseService();

async create(userId: string, recipient: RecipientCreateInput): Promise<ServiceResult<Recipient>> {
const accessResult = await this.programAccessService.getAccessiblePrograms(userId);
Expand All @@ -27,6 +29,12 @@ export class RecipientService extends BaseService {
}

try {
const phoneNumber = recipient.paymentInformation?.create?.phone?.create?.number;
if (!phoneNumber) {
return this.resultFail('No phone number provided for recipient creation');
}

await this.firebaseAuthService.createByPhoneNumber(phoneNumber);
const newRecipient = await this.db.recipient.create({ data: recipient });
return this.resultOk(newRecipient);
} catch (error) {
Expand All @@ -44,7 +52,7 @@ export class RecipientService extends BaseService {

const existing = await this.db.recipient.findUnique({
where: { id: recipient.id?.toString() },
select: { programId: true },
select: { programId: true, paymentInformation: { select: { phone: { select: { number: true } } } } },
});

if (!existing) {
Expand All @@ -58,6 +66,15 @@ export class RecipientService extends BaseService {
}

try {
const phoneNumber =
recipient.paymentInformation?.upsert?.update?.phone?.upsert?.update.number?.toString() ||
recipient.paymentInformation?.upsert?.create?.phone?.create?.number?.toString();
Comment on lines +70 to +72
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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.

Suggested change
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().


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);
Comment on lines +70 to +78
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

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.

Suggested change
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.

const updatedRecipient = await this.db.recipient.update({
where: { id: recipient.id?.toString() },
data: recipient,
Expand Down
59 changes: 59 additions & 0 deletions shared/src/firebase/services/auth.service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { getOrInitializeFirebaseAdmin } from '@socialincome/shared/src/firebase/admin/app';
import { AuthAdmin } from '@socialincome/shared/src/firebase/admin/AuthAdmin';
import { credential } from 'firebase-admin';

const { FIREBASE_SERVICE_ACCOUNT_JSON, FIREBASE_DATABASE_URL } = process.env;

// Firebase service account JSON is Base64-encoded to avoid multiline GitHub Secrets issues in Docker builds
const credentials =
FIREBASE_SERVICE_ACCOUNT_JSON && FIREBASE_DATABASE_URL
? {
credential: credential.cert(JSON.parse(Buffer.from(FIREBASE_SERVICE_ACCOUNT_JSON, 'base64').toString('utf-8'))),
databaseURL: FIREBASE_DATABASE_URL,
}
: undefined;

export class FirebaseService {
private authAdmin = new AuthAdmin(getOrInitializeFirebaseAdmin(credentials));

async createByPhoneNumber(phoneNumber: string) {
try {
const existingUser = await this.getByPhoneNumber(phoneNumber);
if (existingUser) {
console.log('User already exists for phone number:', phoneNumber);
return existingUser;
}
return await this.authAdmin.auth.createUser({
phoneNumber,
});
} catch (error) {
console.error('Error creating user by phone number:', error);
throw new Error('Could not create auth user by phone number');
}
}

async updateByPhoneNumber(oldPhoneNumber: string, newPhoneNumber: string) {
try {
const existingUser = await this.getByPhoneNumber(oldPhoneNumber);
if (!existingUser) throw new Error('Auth user not found');
return await this.authAdmin.auth.updateUser(existingUser.uid, {
phoneNumber: newPhoneNumber,
});
} catch (error) {
console.error('Error fetching user by phone number:', error);
throw new Error('Could not update auth user by phone number');
}
}

async getByPhoneNumber(phoneNumber: string) {
try {
return await this.authAdmin.auth.getUserByPhoneNumber(phoneNumber);
} catch (error: any) {
if (error?.code === 'auth/user-not-found') {
return null;
}
console.error('Error getting user by phone number:', error);
throw new Error('Auth user not found by phone number');
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export function RecipientsTableClient({

const onError = (error: unknown) => {
setHasError(true);
console.error('Local Partner Form Error: ', error);
console.error('Recipient Form Error: ', error);
};

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export function buildUpdateRecipientInput(
upsert: {
create: {
...basePaymentInfo,
phone: paymentInfoFields.phone.value ? { create: { number: paymentInfoFields.phone.value } } : undefined,
phone: { create: { number: paymentInfoFields.phone.value } },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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.

Suggested change
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.

},
update: { ...basePaymentInfo, phone: paymentPhoneUpdate },
where: { id: recipient.paymentInformation?.id },
Expand Down Expand Up @@ -91,7 +91,7 @@ export function buildCreateRecipientInput(
create: {
provider: paymentInfoFields.provider.value,
code: paymentInfoFields.code.value,
phone: paymentInfoFields.phone.value ? { create: { number: paymentInfoFields.phone.value } } : undefined,
phone: { create: { number: paymentInfoFields.phone.value } },
},
},
contact: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,7 @@ const initialFormSchema: RecipientFormSchema = {
label: 'Phone Number',
zodSchema: z
.string()
// TODO: chek regex and optional
.regex(/^\s*(?:\+?(\d{1,3}))?[-. (]*(\d{3})[-. )]*(\d{3})[-. ]*(\d{4})(?: *x(\d+))?\s*$/gm)
.or(z.literal(''))
.optional(),
.regex(/^\+[1-9]\d{1,14}$/, 'Phone number must be in valid E.164 format (e.g., +12345678901)'),
},
},
},
Expand Down
Loading