From 19afa50b9c3ea50dc27ff9be40fb840ecf704dea Mon Sep 17 00:00:00 2001 From: efrilm Date: Fri, 9 Oct 2026 22:58:24 +0700 Subject: [PATCH] =?UTF-8?q?feat(customers):=20store=20customer=20phone=20n?= =?UTF-8?q?umbers=20as=2062=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A customer's phone number (customers.phone_number, the one they log in with) has one stored form, 62 followed by the number without its 0: 6281234561234. util.NormalizePhoneNumber rewrites 0812…, +62 812…, 62812…, 812… and +62 0812…, with spaces, dashes, dots or parentheses, to it, and refuses anything that is not an Indonesian mobile number (628 and 7 to 11 more digits). It is applied wherever a customer types their number: check-phone, register, login and resend-OTP (the validator rewrites the request), and the transfer recipient. Before, the number was matched as typed and a leading 0 was refused, so the same customer written another way was not found. The masked recipient stays 08**-****-1234 as customers write numbers. Migration 000116 rewrites the numbers already stored in customers and otp_sessions. When several become the same number, the customer that already has it keeps it, else a registered one, else the oldest; the others, and numbers that are not Indonesian mobile numbers, are left as they are and cannot log in until fixed by hand (the query to find them is in the migration). Down does nothing. The migration was run, twice, against Postgres with a reduced customers and otp_sessions schema and sample numbers; not against the real schema. The Postgres tests were not run. customers.phone (the POS contact number) is not touched. Co-Authored-By: Claude Opus 5.5 --- .../wallet_exchange_processor_test.go | 20 +++--- .../processor/wallet_expiry_processor_test.go | 10 +-- internal/processor/wallet_move_db_test.go | 10 ++- .../processor/wallet_trace_processor_test.go | 10 +-- .../processor/wallet_transfer_processor.go | 18 ++++-- .../wallet_transfer_processor_test.go | 47 +++++++------- internal/util/phone.go | 41 ++++++++++++ internal/util/phone_test.go | 49 +++++++++++++++ internal/validator/customer_auth_validator.go | 22 ++++--- ..._normalize_customer_phone_numbers.down.sql | 2 + ...16_normalize_customer_phone_numbers.up.sql | 62 +++++++++++++++++++ 11 files changed, 234 insertions(+), 57 deletions(-) create mode 100644 internal/util/phone.go create mode 100644 internal/util/phone_test.go create mode 100644 migrations/000116_normalize_customer_phone_numbers.down.sql create mode 100644 migrations/000116_normalize_customer_phone_numbers.up.sql diff --git a/internal/processor/wallet_exchange_processor_test.go b/internal/processor/wallet_exchange_processor_test.go index 030f021..a722d0d 100644 --- a/internal/processor/wallet_exchange_processor_test.go +++ b/internal/processor/wallet_exchange_processor_test.go @@ -152,7 +152,7 @@ func (f *movePinFake) VerifyPin(_ context.Context, _ uuid.UUID, pin string, acti func TestWalletExchange_DefaultRateIsOneToOne(t *testing.T) { e := newWalletMoveEnv(t) - c := e.member("Budi Santoso", "081234561234") + c := e.member("Budi Santoso", "6281234561234") e.earnCoins(t, c, 50, nil) res, err := e.exchanges().Exchange(e.ctx, c, 50, "482913", "key-1", models.CustomerPinRequestInfo{}) @@ -187,7 +187,7 @@ func TestWalletExchange_DefaultRateIsOneToOne(t *testing.T) { func TestWalletExchange_TenCoinsForThreePoints(t *testing.T) { e := newWalletMoveEnv(t) e.settings.Exchange = models.LoyaltyExchangeSettings{CoinAmount: 10, PointAmount: 3} - c := e.member("Budi", "081234561234") + c := e.member("Budi", "6281234561234") e.earnCoins(t, c, 35, nil) preview, err := e.exchanges().Preview(e.ctx, c, 30) @@ -206,7 +206,7 @@ func TestWalletExchange_TenCoinsForThreePoints(t *testing.T) { func TestWalletExchange_RefusesAmountsThatAreNotAMultiple(t *testing.T) { e := newWalletMoveEnv(t) e.settings.Exchange = models.LoyaltyExchangeSettings{CoinAmount: 10, PointAmount: 3} - c := e.member("Budi", "081234561234") + c := e.member("Budi", "6281234561234") e.earnCoins(t, c, 50, nil) preview, err := e.exchanges().Preview(e.ctx, c, 25) @@ -227,7 +227,7 @@ func TestWalletExchange_RefusesAmountsThatAreNotAMultiple(t *testing.T) { func TestWalletExchange_NeverOutlivesTheCoinLot(t *testing.T) { e := newWalletMoveEnv(t) e.settings.Exchange = models.LoyaltyExchangeSettings{CoinAmount: 10, PointAmount: 3} - c := e.member("Budi", "081234561234") + c := e.member("Budi", "6281234561234") soon, later := e.at(24*time.Hour), e.at(48*time.Hour) first := e.earnCoins(t, c, 15, soon) second := e.earnCoins(t, c, 15, later) @@ -268,7 +268,7 @@ func TestWalletExchange_NeverOutlivesTheCoinLot(t *testing.T) { func TestWalletExchange_LotTooSmallForAWholePointGivesNone(t *testing.T) { e := newWalletMoveEnv(t) e.settings.Exchange = models.LoyaltyExchangeSettings{CoinAmount: 10, PointAmount: 1} - c := e.member("Budi", "081234561234") + c := e.member("Budi", "6281234561234") e.earnCoins(t, c, 5, e.at(time.Hour)) e.earnCoins(t, c, 5, nil) @@ -281,7 +281,7 @@ func TestWalletExchange_LotTooSmallForAWholePointGivesNone(t *testing.T) { func TestWalletExchange_NotEnoughCoins(t *testing.T) { e := newWalletMoveEnv(t) - c := e.member("Budi", "081234561234") + c := e.member("Budi", "6281234561234") e.earnCoins(t, c, 5, nil) preview, err := e.exchanges().Preview(e.ctx, c, 6) @@ -295,7 +295,7 @@ func TestWalletExchange_NotEnoughCoins(t *testing.T) { func TestWalletExchange_WrongPinMovesNothing(t *testing.T) { e := newWalletMoveEnv(t) - c := e.member("Budi", "081234561234") + c := e.member("Budi", "6281234561234") e.earnCoins(t, c, 5, nil) _, err := e.exchanges().Exchange(e.ctx, c, 5, "000000", "key-1", models.CustomerPinRequestInfo{}) @@ -307,7 +307,7 @@ func TestWalletExchange_WrongPinMovesNothing(t *testing.T) { func TestWalletExchange_RetryReturnsTheFirstExchangeAtItsRate(t *testing.T) { e := newWalletMoveEnv(t) - c := e.member("Budi", "081234561234") + c := e.member("Budi", "6281234561234") e.earnCoins(t, c, 100, nil) first, err := e.exchanges().Exchange(e.ctx, c, 40, "482913", "key-1", models.CustomerPinRequestInfo{}) @@ -330,7 +330,7 @@ func TestWalletExchange_RetryReturnsTheFirstExchangeAtItsRate(t *testing.T) { func TestWalletExchange_RequiresAnIdempotencyKey(t *testing.T) { e := newWalletMoveEnv(t) - c := e.member("Budi", "081234561234") + c := e.member("Budi", "6281234561234") e.earnCoins(t, c, 5, nil) _, err := e.exchanges().Exchange(e.ctx, c, 5, "482913", " ", models.CustomerPinRequestInfo{}) @@ -344,7 +344,7 @@ func TestWalletExchange_CappedByThePointExpiry(t *testing.T) { e := newWalletMoveEnv(t) e.now = wib(2026, 6, 1, 10, 0) e.settings.PointExpiry = rolling(30, "DAY", false) - c := e.member("Budi", "081234561234") + c := e.member("Budi", "6281234561234") soon, later := e.at(24*time.Hour), e.at(90*24*time.Hour) e.earnCoins(t, c, 10, soon) e.earnCoins(t, c, 10, later) diff --git a/internal/processor/wallet_expiry_processor_test.go b/internal/processor/wallet_expiry_processor_test.go index 0345313..90a8f93 100644 --- a/internal/processor/wallet_expiry_processor_test.go +++ b/internal/processor/wallet_expiry_processor_test.go @@ -54,7 +54,7 @@ func (e *walletMoveEnv) expiry(notifier customerNotifier) (*WalletExpiryProcesso func TestWalletExpiry_ExpiresWhatIsDueAndTellsTheCustomer(t *testing.T) { e := newWalletMoveEnv(t) - a := e.member("Anita", "081200005678") + a := e.member("Anita", "6281200005678") ord := earn(a, 150, e.at(-time.Hour)) ord.Description = "Belanja #ORD-0098" due := e.credit(t, ord) @@ -102,7 +102,7 @@ func TestWalletExpiry_ExpiresWhatIsDueAndTellsTheCustomer(t *testing.T) { func TestWalletExpiry_RunningAgainExpiresNothingMore(t *testing.T) { e := newWalletMoveEnv(t) - a := e.member("Anita", "081200005678") + a := e.member("Anita", "6281200005678") e.credit(t, earn(a, 150, e.at(-time.Hour))) notifier := ¬ifierFake{} @@ -128,7 +128,7 @@ func TestWalletExpiry_RunningAgainExpiresNothingMore(t *testing.T) { func TestWalletExpiry_OneFailingLotDoesNotStopTheOthers(t *testing.T) { e := newWalletMoveEnv(t) - a := e.member("Anita", "081200005678") + a := e.member("Anita", "6281200005678") e.credit(t, earn(a, 150, e.at(-time.Hour))) p, repo := e.expiry(nil) // A lot listed that ExpireLot cannot find. @@ -142,7 +142,7 @@ func TestWalletExpiry_OneFailingLotDoesNotStopTheOthers(t *testing.T) { func TestWalletExpiry_NothingDue(t *testing.T) { e := newWalletMoveEnv(t) - a := e.member("Anita", "081200005678") + a := e.member("Anita", "6281200005678") e.credit(t, earn(a, 150, e.at(time.Hour))) notifier := ¬ifierFake{} p, _ := e.expiry(notifier) @@ -206,7 +206,7 @@ func TestWalletExpiry_RemindsOncePerDayBeforeExpiry(t *testing.T) { e.now = wib(2026, 10, 25, 9, 0) e.settings.PointExpiry.ReminderDays = 7 e.settings.CoinExpiry.ReminderDays = 0 // no reminders for EnakCoin - a := e.member("Anita", "081200005678") + a := e.member("Anita", "6281200005678") oct31 := wib(2026, 10, 31, 23, 59) nov30 := wib(2026, 11, 30, 23, 59) e.credit(t, earn(a, 100, &oct31)) diff --git a/internal/processor/wallet_move_db_test.go b/internal/processor/wallet_move_db_test.go index 524c943..33e725e 100644 --- a/internal/processor/wallet_move_db_test.go +++ b/internal/processor/wallet_move_db_test.go @@ -2,6 +2,7 @@ package processor import ( "context" + "encoding/binary" "fmt" "os" "sync" @@ -42,7 +43,7 @@ func walletMoveDB(t *testing.T) (db *gorm.DB, org, a, b uuid.UUID) { require.NoError(t, err) org, a, b = uuid.New(), uuid.New(), uuid.New() - phoneA, phoneB := "08"+a.String()[:10], "08"+b.String()[:10] + phoneA, phoneB := walletTestPhone(a), walletTestPhone(b) require.NoError(t, db.Exec(`INSERT INTO organizations (id, name, plan_type) VALUES (?, 'wallet move test', 'basic')`, org).Error) require.NoError(t, db.Exec(`INSERT INTO customers (id, organization_id, name, phone_number) VALUES (?, ?, 'Anita', ?), (?, ?, 'Budi Santoso', ?)`, a, org, phoneA, b, org, phoneB).Error) @@ -117,7 +118,7 @@ func TestWalletTransfer_BothWaysAtOnceAgainstPostgres(t *testing.T) { _, err := wallet.Credit(ctx, earn(b, 100, nil)) return err })) - phone := func(id uuid.UUID) string { return "08" + id.String()[:10] } + phone := walletTestPhone const rounds = 10 errs := make(chan error, 2*rounds) @@ -235,3 +236,8 @@ func TestWalletExpiry_TwoInstancesAgainstPostgres(t *testing.T) { assert.Equal(t, int64(10), expires) assert.Zero(t, left) } + +// walletTestPhone is a phone number in its stored form, 628…, unique to the customer. +func walletTestPhone(id uuid.UUID) string { + return fmt.Sprintf("628%09d", binary.BigEndian.Uint64(id[:8])%1_000_000_000) +} diff --git a/internal/processor/wallet_trace_processor_test.go b/internal/processor/wallet_trace_processor_test.go index 47f174e..fdda0b2 100644 --- a/internal/processor/wallet_trace_processor_test.go +++ b/internal/processor/wallet_trace_processor_test.go @@ -85,8 +85,8 @@ func findRow(t *testing.T, e *walletMoveEnv, customerID uuid.UUID, txType string // redeems 30. Tracing B's redemption leads to A's order #ORD-1. func TestWalletTrace_RedemptionLeadsBackToTheSendersOrder(t *testing.T) { e := newWalletMoveEnv(t) - a := e.member("Anita", "081200005678") - b := e.member("Budi Santoso", "081234561234") + a := e.member("Anita", "6281200005678") + b := e.member("Budi Santoso", "6281234561234") ord1 := earn(a, 100, e.at(30*24*time.Hour)) ord1.Description = "Belanja #ORD-1" ord2 := earn(a, 50, e.at(60*24*time.Hour)) @@ -121,8 +121,8 @@ func TestWalletTrace_RedemptionLeadsBackToTheSendersOrder(t *testing.T) { func TestWalletTrace_DebitAndCreditOfATransfer(t *testing.T) { e := newWalletMoveEnv(t) - a := e.member("Anita", "081200005678") - b := e.member("Budi", "081234561234") + a := e.member("Anita", "6281200005678") + b := e.member("Budi", "6281234561234") e.credit(t, earn(a, 100, e.at(time.Hour))) e.credit(t, earn(a, 50, nil)) _, err := e.transfers(nil).Transfer(e.ctx, a, sendPoints(120, "081234561234"), "482913", "key-1", models.CustomerPinRequestInfo{}) @@ -153,7 +153,7 @@ func TestWalletTrace_DebitAndCreditOfATransfer(t *testing.T) { func TestWalletTrace_OtherOrganizationsRowsAreNotFound(t *testing.T) { e := newWalletMoveEnv(t) - a := e.member("Anita", "081200005678") + a := e.member("Anita", "6281200005678") res := e.credit(t, earn(a, 10, nil)) _, err := NewWalletTraceProcessor(walletTraceRepoFake{e}).Trace(e.ctx, uuid.New(), res.Transaction.ID) diff --git a/internal/processor/wallet_transfer_processor.go b/internal/processor/wallet_transfer_processor.go index 6c2d7c1..83c9307 100644 --- a/internal/processor/wallet_transfer_processor.go +++ b/internal/processor/wallet_transfer_processor.go @@ -15,6 +15,7 @@ import ( "apskel-pos-be/internal/logger" "apskel-pos-be/internal/models" "apskel-pos-be/internal/repository" + "apskel-pos-be/internal/util" ) // ErrWalletRecipientNotFound means no customer of the sender's organization has the @@ -213,10 +214,13 @@ func (p *WalletTransferProcessor) Transfer(ctx context.Context, senderID uuid.UU // them: an active customer of the same organization, not the walk-in customer, and // not the sender. func (p *WalletTransferProcessor) recipient(ctx context.Context, sender *repository.WalletMoveCustomer, phoneNumber string) (*repository.WalletMoveCustomer, error) { - phoneNumber = strings.TrimSpace(phoneNumber) - if phoneNumber == "" { + if strings.TrimSpace(phoneNumber) == "" { return nil, fmt.Errorf("%w: the recipient's phone number is required", ErrWalletMoveRejected) } + phoneNumber, err := util.NormalizePhoneNumber(phoneNumber) + if err != nil { + return nil, fmt.Errorf("%w: the recipient's phone number is not valid", ErrWalletMoveRejected) + } recipient, err := p.customers.FindCustomerByPhone(ctx, phoneNumber) if errors.Is(err, repository.ErrWalletNotFound) { return nil, ErrWalletRecipientNotFound @@ -282,10 +286,14 @@ func maskName(name string) string { return strings.Join(words, " ") } -// maskPhoneNumber keeps the first two and the last four digits: -// "081234561234" → "08**-****-1234". +// maskPhoneNumber keeps the first two and the last four digits of the number as +// customers write it, with 0 for 62: "6281234561234" → "08**-****-1234". func maskPhoneNumber(phone string) string { - runes := []rune(strings.TrimSpace(phone)) + phone = strings.TrimSpace(phone) + if strings.HasPrefix(phone, "62") { + phone = "0" + phone[2:] + } + runes := []rune(phone) if len(runes) < 8 { return "****" } diff --git a/internal/processor/wallet_transfer_processor_test.go b/internal/processor/wallet_transfer_processor_test.go index 090ca88..b942c68 100644 --- a/internal/processor/wallet_transfer_processor_test.go +++ b/internal/processor/wallet_transfer_processor_test.go @@ -37,8 +37,8 @@ func sendPoints(amount int64, phone string) models.WalletTransfer { func TestWalletTransfer_MovesBalanceWithItsExpiry(t *testing.T) { e := newWalletMoveEnv(t) - a := e.member("Anita", "081200005678") - b := e.member("Budi Santoso", "081234561234") + a := e.member("Anita", "6281200005678") + b := e.member("Budi Santoso", "6281234561234") dec, jan := e.at(30*24*time.Hour), e.at(60*24*time.Hour) first := e.credit(t, earn(a, 100, dec)) second := e.credit(t, earn(a, 50, jan)) @@ -97,8 +97,8 @@ func TestWalletTransfer_MovesBalanceWithItsExpiry(t *testing.T) { func TestWalletTransfer_Coins(t *testing.T) { e := newWalletMoveEnv(t) - a := e.member("Anita", "081200005678") - b := e.member("Budi", "081234561234") + a := e.member("Anita", "6281200005678") + b := e.member("Budi", "6281234561234") e.earnCoins(t, a, 10, nil) _, err := e.transfers(nil).Transfer(e.ctx, a, models.WalletTransfer{Currency: "coin", Amount: 4, RecipientPhone: "081234561234"}, "482913", "key-1", models.CustomerPinRequestInfo{}) @@ -109,14 +109,14 @@ func TestWalletTransfer_Coins(t *testing.T) { func TestWalletTransfer_RefusesRecipientsItMayNotSendTo(t *testing.T) { e := newWalletMoveEnv(t) - a := e.member("Anita", "081200005678") + a := e.member("Anita", "6281200005678") e.credit(t, earn(a, 100, nil)) - walkIn := e.member("Walk-in", "081100000000") + walkIn := e.member("Walk-in", "6281100000000") e.customers.byID[walkIn].IsDefault = true - inactive := e.member("Old", "081100000001") + inactive := e.member("Old", "6281100000001") e.customers.byID[inactive].IsActive = false - elsewhere := e.member("Other Org", "081100000002") + elsewhere := e.member("Other Org", "6281100000002") e.customers.byID[elsewhere].OrganizationID = uuid.New() for phone, want := range map[string]error{ @@ -126,6 +126,7 @@ func TestWalletTransfer_RefusesRecipientsItMayNotSendTo(t *testing.T) { "081100000002": ErrWalletRecipientNotFound, // another organization looks like nobody "081999999999": ErrWalletRecipientNotFound, "": ErrWalletMoveRejected, + "021-1234567": ErrWalletMoveRejected, // not a mobile number } { _, err := e.transfers(nil).Recipient(e.ctx, a, phone) assert.ErrorIs(t, err, want, phone) @@ -138,18 +139,20 @@ func TestWalletTransfer_RefusesRecipientsItMayNotSendTo(t *testing.T) { func TestWalletTransfer_RecipientIsMasked(t *testing.T) { e := newWalletMoveEnv(t) - a := e.member("Anita", "081200005678") - e.member("Budi Santoso", "081234561234") + a := e.member("Anita", "6281200005678") + e.member("Budi Santoso", "6281234561234") - got, err := e.transfers(nil).Recipient(e.ctx, a, " 081234561234 ") - require.NoError(t, err) - assert.Equal(t, &models.WalletTransferRecipient{Name: "Bu*** Sa***", PhoneNumber: "08**-****-1234"}, got) + for _, phone := range []string{" 081234561234 ", "6281234561234", "+62 812-3456-1234"} { + got, err := e.transfers(nil).Recipient(e.ctx, a, phone) + require.NoError(t, err, phone) + assert.Equal(t, &models.WalletTransferRecipient{Name: "Bu*** Sa***", PhoneNumber: "08**-****-1234"}, got, phone) + } } func TestWalletTransfer_OrganizationLimits(t *testing.T) { e := newWalletMoveEnv(t) - a := e.member("Anita", "081200005678") - e.member("Budi", "081234561234") + a := e.member("Anita", "6281200005678") + e.member("Budi", "6281234561234") e.credit(t, earn(a, 1000, nil)) e.settings.Transfer = models.LoyaltyTransferSettings{Enabled: true, MinAmount: 10, MaxPerTransaction: ptr(int64(300)), DailyLimit: ptr(int64(500))} send := func(amount int64, key string) error { @@ -177,8 +180,8 @@ func TestWalletTransfer_OrganizationLimits(t *testing.T) { func TestWalletTransfer_HeldAfterPinReset(t *testing.T) { e := newWalletMoveEnv(t) - a := e.member("Anita", "081200005678") - e.member("Budi", "081234561234") + a := e.member("Anita", "6281200005678") + e.member("Budi", "6281234561234") e.credit(t, earn(a, 100, nil)) until := e.now.Add(time.Hour) e.pins.err = &PinError{Code: PinErrTransferBlocked, Until: &until} @@ -192,8 +195,8 @@ func TestWalletTransfer_HeldAfterPinReset(t *testing.T) { func TestWalletTransfer_NotEnoughBalance(t *testing.T) { e := newWalletMoveEnv(t) - a := e.member("Anita", "081200005678") - b := e.member("Budi", "081234561234") + a := e.member("Anita", "6281200005678") + b := e.member("Budi", "6281234561234") e.credit(t, earn(a, 100, nil)) // An expired lot cannot be sent even before the expiry job takes it. e.credit(t, earn(a, 50, e.at(-time.Hour))) @@ -205,9 +208,9 @@ func TestWalletTransfer_NotEnoughBalance(t *testing.T) { func TestWalletTransfer_RetryMovesNothingAndTellsNobodyAgain(t *testing.T) { e := newWalletMoveEnv(t) - a := e.member("Anita", "081200005678") - b := e.member("Budi", "081234561234") - e.member("Citra", "081255550000") + a := e.member("Anita", "6281200005678") + b := e.member("Budi", "6281234561234") + e.member("Citra", "6281255550000") e.credit(t, earn(a, 100, nil)) notifier := ¬ifierFake{} diff --git a/internal/util/phone.go b/internal/util/phone.go new file mode 100644 index 0000000..dbc9719 --- /dev/null +++ b/internal/util/phone.go @@ -0,0 +1,41 @@ +package util + +import ( + "errors" + "regexp" + "strings" +) + +// ErrInvalidPhoneNumber means a phone number is not an Indonesian mobile number. +var ErrInvalidPhoneNumber = errors.New("invalid phone number format") + +// customerPhonePattern is an Indonesian mobile number in its stored form: 62, then the +// number without its leading 0 (628…, 10 to 14 digits in all). +var customerPhonePattern = regexp.MustCompile(`^628\d{7,11}$`) + +// NormalizePhoneNumber turns a customer's phone number into the one form it is stored +// and looked up in, 62…: "0812-3456-1234", "+62 812 3456 1234", "62812…" and "812…" +// all become "6281234561234". Spaces, dashes, dots and parentheses are dropped. +func NormalizePhoneNumber(raw string) (string, error) { + digits := strings.Map(func(r rune) rune { + switch r { + case ' ', '-', '.', '(', ')': + return -1 + } + return r + }, strings.TrimSpace(raw)) + digits = strings.TrimPrefix(digits, "+") + switch { + case strings.HasPrefix(digits, "620"): + // +62 typed in front of a number that kept its 0. + digits = "62" + digits[3:] + case strings.HasPrefix(digits, "0"): + digits = "62" + digits[1:] + case strings.HasPrefix(digits, "8"): + digits = "62" + digits + } + if !customerPhonePattern.MatchString(digits) { + return "", ErrInvalidPhoneNumber + } + return digits, nil +} diff --git a/internal/util/phone_test.go b/internal/util/phone_test.go new file mode 100644 index 0000000..fef9c97 --- /dev/null +++ b/internal/util/phone_test.go @@ -0,0 +1,49 @@ +package util + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNormalizePhoneNumber(t *testing.T) { + for _, raw := range []string{ + "6281234561234", + "+6281234561234", + "081234561234", + "81234561234", + "0812-3456-1234", + "+62 812 3456 1234", + "(+62) 812.3456.1234", + "+62081234561234", + " 081234561234 ", + } { + got, err := NormalizePhoneNumber(raw) + require.NoError(t, err, raw) + assert.Equal(t, "6281234561234", got, raw) + } + + shortest, err := NormalizePhoneNumber("0811234567") + require.NoError(t, err) + assert.Equal(t, "62811234567", shortest) + longest, err := NormalizePhoneNumber("0812345678901") + require.NoError(t, err) + assert.Equal(t, "62812345678901", longest) + _, err = NormalizePhoneNumber("08123456789012") + assert.ErrorIs(t, err, ErrInvalidPhoneNumber, "too long") + + for _, raw := range []string{ + "", + "0", + "021-1234567", // a landline, cannot get the WhatsApp OTP + "+6521234567", // not Indonesian + "+1 415 555 0100", // not Indonesian + "62812", // too short + "0812abc61234", + "0812/3456/1234", + } { + _, err := NormalizePhoneNumber(raw) + assert.ErrorIs(t, err, ErrInvalidPhoneNumber, raw) + } +} diff --git a/internal/validator/customer_auth_validator.go b/internal/validator/customer_auth_validator.go index a62f459..136be21 100644 --- a/internal/validator/customer_auth_validator.go +++ b/internal/validator/customer_auth_validator.go @@ -9,6 +9,7 @@ import ( "apskel-pos-be/internal/constants" "apskel-pos-be/internal/contract" + "apskel-pos-be/internal/util" ) type CustomerAuthValidator interface { @@ -36,7 +37,7 @@ func (v *CustomerAuthValidatorImpl) ValidateCheckPhoneRequest(req *contract.Chec return errors.New("phone number is required"), constants.ValidationErrorCode } - if !v.isValidPhoneNumber(req.PhoneNumber) { + if !v.normalizePhoneNumber(&req.PhoneNumber) { return errors.New("invalid phone number format"), constants.ValidationErrorCode } @@ -53,7 +54,7 @@ func (v *CustomerAuthValidatorImpl) ValidateRegisterStartRequest(req *contract.R return errors.New("phone number is required"), constants.ValidationErrorCode } - if !v.isValidPhoneNumber(req.PhoneNumber) { + if !v.normalizePhoneNumber(&req.PhoneNumber) { return errors.New("invalid phone number format"), constants.ValidationErrorCode } @@ -161,7 +162,7 @@ func (v *CustomerAuthValidatorImpl) ValidateCustomerLoginRequest(req *contract.C return errors.New("phone number is required"), constants.ValidationErrorCode } - if !v.isValidPhoneNumber(req.PhoneNumber) { + if !v.normalizePhoneNumber(&req.PhoneNumber) { return errors.New("invalid phone number format"), constants.ValidationErrorCode } @@ -174,10 +175,15 @@ func (v *CustomerAuthValidatorImpl) ValidateCustomerLoginRequest(req *contract.C } // Helper validation functions -func (v *CustomerAuthValidatorImpl) isValidPhoneNumber(phoneNumber string) bool { - // Basic phone number validation - adjust regex based on your requirements - phoneRegex := regexp.MustCompile(`^\+?[1-9]\d{1,14}$`) - return phoneRegex.MatchString(phoneNumber) +// normalizePhoneNumber rewrites the phone number in the form it is stored in, 62… +// (util.NormalizePhoneNumber), and reports false when it is not a valid one. +func (v *CustomerAuthValidatorImpl) normalizePhoneNumber(phoneNumber *string) bool { + normalized, err := util.NormalizePhoneNumber(*phoneNumber) + if err != nil { + return false + } + *phoneNumber = normalized + return true } func (v *CustomerAuthValidatorImpl) isValidDateFormat(date string) bool { @@ -208,7 +214,7 @@ func (v *CustomerAuthValidatorImpl) ValidateResendOtpRequest(req *contract.Resen } // Validate phone number format - if !v.isValidPhoneNumber(req.PhoneNumber) { + if !v.normalizePhoneNumber(&req.PhoneNumber) { return errors.New("invalid phone number format"), constants.CustomerEntity } diff --git a/migrations/000116_normalize_customer_phone_numbers.down.sql b/migrations/000116_normalize_customer_phone_numbers.down.sql new file mode 100644 index 0000000..6779adc --- /dev/null +++ b/migrations/000116_normalize_customer_phone_numbers.down.sql @@ -0,0 +1,2 @@ +-- Nothing to undo: the forms the numbers were stored in before are not kept, and 62… is +-- what the code before this migration accepted too. diff --git a/migrations/000116_normalize_customer_phone_numbers.up.sql b/migrations/000116_normalize_customer_phone_numbers.up.sql new file mode 100644 index 0000000..d0e62fc --- /dev/null +++ b/migrations/000116_normalize_customer_phone_numbers.up.sql @@ -0,0 +1,62 @@ +-- Customer phone numbers are stored in one form, 62… without + (util.NormalizePhoneNumber), +-- and every number a customer types is rewritten to it before it is looked up. This +-- rewrites the numbers stored before, so 0812…, +62 812… and 812… become 62812…. +-- +-- Only one customer may have a number. When several stored numbers become the same one, +-- the customer that already has it keeps it, else a registered one (with a password), +-- else the oldest; the others are left as they are. A number that is not an Indonesian +-- mobile number is left as it is too. Neither can log in until it is fixed by hand: +-- +-- SELECT id, name, phone_number FROM customers +-- WHERE phone_number IS NOT NULL AND phone_number !~ '^628[0-9]{7,11}$'; + +WITH stripped AS ( + SELECT id, phone_number, password_hash, created_at, + regexp_replace(regexp_replace(phone_number, '[[:space:]().-]', '', 'g'), '^\+', '') AS p + FROM customers + WHERE phone_number IS NOT NULL +), +normalized AS ( + SELECT id, phone_number, password_hash, created_at, + CASE + WHEN p LIKE '620%' THEN '62' || substr(p, 4) + WHEN p LIKE '0%' THEN '62' || substr(p, 2) + WHEN p LIKE '8%' THEN '62' || p + ELSE p + END AS new_phone + FROM stripped +), +ranked AS ( + SELECT id, new_phone, + row_number() OVER ( + PARTITION BY new_phone + ORDER BY phone_number = new_phone DESC, password_hash IS NOT NULL DESC, created_at, id + ) AS rank + FROM normalized + WHERE new_phone ~ '^628[0-9]{7,11}$' +) +UPDATE customers c +SET phone_number = r.new_phone, updated_at = NOW() +FROM ranked r +WHERE c.id = r.id AND r.rank = 1 AND c.phone_number <> r.new_phone; + +-- A registration started before this migration finishes with the number in its OTP +-- session, and a resent OTP is found by it. +UPDATE otp_sessions +SET phone_number = n.new_phone, updated_at = NOW() +FROM ( + SELECT id, + CASE + WHEN p LIKE '620%' THEN '62' || substr(p, 4) + WHEN p LIKE '0%' THEN '62' || substr(p, 2) + WHEN p LIKE '8%' THEN '62' || p + ELSE p + END AS new_phone + FROM ( + SELECT id, regexp_replace(regexp_replace(phone_number, '[[:space:]().-]', '', 'g'), '^\+', '') AS p + FROM otp_sessions + ) stripped +) n +WHERE otp_sessions.id = n.id + AND n.new_phone ~ '^628[0-9]{7,11}$' + AND otp_sessions.phone_number <> n.new_phone;