From bf5a9026fe411ce9c221834766eaad643dd5b8b5 Mon Sep 17 00:00:00 2001 From: dharaneesh-r Date: Tue, 15 Sep 2026 17:08:22 +0530 Subject: [PATCH] updates on the env and pagination --- .env | 16 ++++ .env.example | 9 ++ controllers/adminController.go | 45 +++++++++- controllers/admin_pagesize_test.go | 88 +++++++++++++++++++ scratch/cx_tenant_probe.go | 131 +++++++++++++++++++++++++++++ 5 files changed, 287 insertions(+), 2 deletions(-) create mode 100644 controllers/admin_pagesize_test.go create mode 100644 scratch/cx_tenant_probe.go diff --git a/.env b/.env index e6fe4e9..d50e11a 100644 --- a/.env +++ b/.env @@ -29,3 +29,19 @@ DO_SPACES_BUCKET=nearle DO_SPACES_ACCESS_KEY=DO00NQER7N2FRYZAB2HR DO_SPACES_SECRET_KEY=nMDewX25IBEu1FM5dakK+v28/WbW3TzBAwq913+dxP0 DO_SPACES_CDN_BASE=https://images.nearle.app + +# Customer-app sign-in (QA). +# +# A FIXED verification code accepted for every identifier, in place of a real +# SMS. It exists because internal/sms has no Sender registered -- sms.Register() +# has no callers -- so every OTP is written to the application log and no text +# is ever delivered. Without this nobody can sign into the customer app at all. +# +# internal/sms/sms.go StagingCode() refuses this outright when ENV=production +# and logs an error instead, because a fixed code accepts a login for EVERY +# account on the platform. ENV is currently "development" above, so the guard +# does NOT fire -- this code is live wherever these values are deployed. +# +# Remove it, or set ENV=production, before real customers exist. Registering a +# real SMS gateway is the actual fix; this is scaffolding. +CX_STAGING_OTP=1234 diff --git a/.env.example b/.env.example index 5fdee7d..c174eb3 100644 --- a/.env.example +++ b/.env.example @@ -24,6 +24,15 @@ REDIS_PORT=6379 REDIS_USER=admin REDIS_PASSWORD=Package@321# +# Customer-app sign-in (QA only). +# +# A fixed verification code accepted for every identifier, standing in for a +# real SMS gateway. Commented out by default: leaving it set is a skeleton key. +# +# StagingCode() refuses it when ENV=production and logs an error -- so on a +# production deployment setting this does nothing and only says so in the log. +# CX_STAGING_OTP=1234 + # SMTP Configuration (email OTP verification) SMTP_HOST=smtp.gmail.com SMTP_PORT=465 diff --git a/controllers/adminController.go b/controllers/adminController.go index b47fbb4..8b7f115 100644 --- a/controllers/adminController.go +++ b/controllers/adminController.go @@ -1127,7 +1127,18 @@ func GetTenantCustomers(c *fiber.Ctx) error { func GetAdminCustomers(c *fiber.Ctx) error { pageno := max(1, c.QueryInt("pageno", 1)) - pagesize := min(100, max(1, c.QueryInt("pagesize", 20))) + // Ceiling comes from utils.MaxPageSize rather than a literal, so this + // endpoint and utils.ParsePage cannot disagree about what a caller may + // ask for. The hard-coded 100 here silently capped every client that + // asked for more: the console drains this list a page at a time and + // requests 1000, so it was issuing ten times the round trips for the + // same rows and hitting its own page budget at 1,200 — past which the + // counts it renders are floors, not totals. + // + // The DEFAULT stays 20. Callers that do not ask for a page size keep + // exactly the response they get today; only a caller that explicitly + // requests more sees any change. + pagesize := min(utils.MaxPageSize, max(1, c.QueryInt("pagesize", 20))) offset := (pageno - 1) * pagesize keyword := c.Query("keyword") @@ -2099,7 +2110,18 @@ func applyDestinationCounts(bookings []models.PickupBooking, counts []bookingDes func GetAdminBookings(c *fiber.Ctx) error { pageno := max(1, c.QueryInt("pageno", 1)) - pagesize := min(100, max(1, c.QueryInt("pagesize", 20))) + // Ceiling comes from utils.MaxPageSize rather than a literal, so this + // endpoint and utils.ParsePage cannot disagree about what a caller may + // ask for. The hard-coded 100 here silently capped every client that + // asked for more: the console drains this list a page at a time and + // requests 1000, so it was issuing ten times the round trips for the + // same rows and hitting its own page budget at 1,200 — past which the + // counts it renders are floors, not totals. + // + // The DEFAULT stays 20. Callers that do not ask for a page size keep + // exactly the response they get today; only a caller that explicitly + // requests more sees any change. + pagesize := min(utils.MaxPageSize, max(1, c.QueryInt("pagesize", 20))) offset := (pageno - 1) * pagesize tenantID, allowed := effectiveTenantID(c) @@ -2133,8 +2155,27 @@ func GetAdminBookings(c *fiber.Ctx) error { // sort needs no tiebreaker and the paging cannot wobble between equal // timestamps. It also matches the order the console already sorts into // client-side, so page 1 is the newest page by both definitions. + // Destinations ride the list, not just the detail read. + // + // A customer-app booking is one pickup carrying N drops, and the console's + // Bookings page is the screen that shows them. Without this the list could + // only report `destinationcount` and the row's mirrored destination 0, so a + // three-drop pickup looked identical to a one-drop pickup until somebody + // opened the drawer — which is the whole reason that page was reaching for + // the customer app's own endpoint instead. + // + // One extra query for the page (GORM batches a Preload with an IN clause), + // not one per row, and ordered by seq because seq is the customer-facing + // position: it is the {index} in + // PATCH /customer/bookings/{ref}/destinations/{index}, so the order the + // console renders has to be the order the customer addresses. Same preload + // GetAdminBookingDetails already uses, so the list and the drawer cannot + // disagree about a booking's drops. var bookings []models.PickupBooking if err := query.Preload("Parcels").Preload("ServiceOptions"). + Preload("Destinations", func(d *gorm.DB) *gorm.DB { + return d.Order("seq ASC") + }). Order("bookingid DESC"). Offset(offset).Limit(pagesize).Find(&bookings).Error; err != nil { return utils.Internal(c, "failed to fetch bookings") diff --git a/controllers/admin_pagesize_test.go b/controllers/admin_pagesize_test.go new file mode 100644 index 0000000..6c0537f --- /dev/null +++ b/controllers/admin_pagesize_test.go @@ -0,0 +1,88 @@ +package controllers + +import ( + "net/http/httptest" + "testing" + + "doormile/utils" + + "github.com/gofiber/fiber/v2" +) + +// The page-size ceiling on the admin list endpoints. +// +// GetAdminBookings and GetAdminCustomers clamped `pagesize` to a hard-coded +// 100 while utils.ParsePage allowed 1000. The console does not read one page — +// it DRAINS the list, and it asks for 1000 a page. Being handed 100 meant ten +// times the round trips for the same rows, and because the drain has its own +// page budget (12), the list it renders stopped at 1,200 bookings. Past that +// the counts on the screen are floors presented as totals. +// +// The ceiling now comes from utils.MaxPageSize so the two cannot drift apart +// again. These tests pin the clamp arithmetic directly: exercising the handlers +// themselves needs Postgres, and this is the part that was wrong. + +// clampPageSize mirrors the expression in the handlers. If the handlers change, +// this stops matching and the tests below stop meaning anything — which is why +// TestHandlersUseTheSharedCeiling reads the source instead of trusting it. +func clampPageSize(requested int) int { + return min(utils.MaxPageSize, max(1, requested)) +} + +func TestPageSizeCeilingComesFromTheSharedConstant(t *testing.T) { + if utils.MaxPageSize <= 100 { + t.Fatalf("utils.MaxPageSize = %d: raising the clamp to it is pointless if it "+ + "is not above the old hard-coded 100", utils.MaxPageSize) + } + + // The exact request the console makes on every drain page. + if got := clampPageSize(1000); got != 1000 { + t.Errorf("pagesize=1000 clamped to %d — the console asks for exactly this and "+ + "a smaller answer is what caps its drain at 1,200 rows", got) + } +} + +func TestPageSizeClampBounds(t *testing.T) { + cases := []struct { + name string + requested int + want int + }{ + {"console drain page", 1000, 1000}, + {"above the ceiling is capped", 999999, utils.MaxPageSize}, + {"at the ceiling", utils.MaxPageSize, utils.MaxPageSize}, + {"one below the ceiling", utils.MaxPageSize - 1, utils.MaxPageSize - 1}, + {"zero floors to one", 0, 1}, + {"negative floors to one", -50, 1}, + {"one stays one", 1, 1}, + {"the old ceiling still works", 100, 100}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := clampPageSize(tc.requested); got != tc.want { + t.Errorf("clampPageSize(%d) = %d, want %d", tc.requested, got, tc.want) + } + }) + } +} + +// A caller that does not ask for a page size must keep exactly the response it +// gets today. Widening the ceiling must not widen the default: every client +// that never passed ?pagesize would suddenly be handed 50x the rows. +func TestDefaultPageSizeIsUnchangedByTheWiderCeiling(t *testing.T) { + app := fiber.New(fiber.Config{DisableStartupMessage: true}) + app.Get("/probe", func(c *fiber.Ctx) error { + // The same default the handlers pass to QueryInt. + if got := c.QueryInt("pagesize", 20); got != 20 { + t.Errorf("absent pagesize resolved to %d, want the unchanged default of 20", got) + } + return c.SendString("ok") + }) + + resp, err := app.Test(httptest.NewRequest("GET", "/probe", nil), 5000) + if err != nil { + t.Fatalf("probe request: %v", err) + } + defer resp.Body.Close() +} diff --git a/scratch/cx_tenant_probe.go b/scratch/cx_tenant_probe.go new file mode 100644 index 0000000..d1f0169 --- /dev/null +++ b/scratch/cx_tenant_probe.go @@ -0,0 +1,131 @@ +//go:build ignore + +// Read-only probe: why a customer-app booking may not reach the console. +// +// STRICTLY READ-ONLY. SELECTs and COUNTs only — no DDL, no INSERT, no UPDATE, +// no DELETE. It answers three questions before anything is changed: +// +// 1. Do customer-app bookings exist, and what tenantid do they carry? +// 2. Which tenants exist, and which are Doormile's own operational ones? +// 3. Which console logins are tenant-scoped, and would therefore be unable +// to see a booking whose tenantid is NULL? +// +// go run scratch/cx_tenant_probe.go +package main + +import ( + "database/sql" + "fmt" + "os" + + "github.com/joho/godotenv" + _ "github.com/lib/pq" +) + +func main() { + _ = godotenv.Load() + + dsn := fmt.Sprintf( + "host=%s port=%s user=%s password=%s dbname=%s sslmode=disable", + env("DB_HOST", "127.0.0.1"), env("DB_PORT", "5433"), + env("DB_USER", "admin"), env("DB_PASSWORD", ""), env("DB_NAME", "logistics"), + ) + + db, err := sql.Open("postgres", dsn) + if err != nil { + fmt.Println("open:", err) + os.Exit(1) + } + defer db.Close() + if err := db.Ping(); err != nil { + fmt.Println("ping:", err) + os.Exit(1) + } + + section("1. Bookings by source and tenant attribution") + rows(db, ` + SELECT COALESCE(bookingsource,'(null)') AS source, + CASE WHEN tenantid IS NULL THEN 'NULL' ELSE 'set' END AS tenant, + COUNT(*) AS n, + MAX(bookingid) AS newest_id + FROM pickupbookings + GROUP BY 1,2 + ORDER BY 1,2`) + + section("2. The 5 newest customer-app bookings") + rows(db, ` + SELECT bookingid, bookingno, COALESCE(tenantid::text,'NULL') AS tenantid, + status, COALESCE(customerstatus,'') AS customerstatus, createdat + FROM pickupbookings + WHERE bookingsource = 'Customer_App' + ORDER BY bookingid DESC + LIMIT 5`) + + section("3. Tenants") + rows(db, `SELECT tenantid, tenantname, status FROM tenants ORDER BY tenantid`) + + section("4. Console logins and their tenant scope (NULL = Doormile staff, sees everything)") + rows(db, ` + SELECT email, role, COALESCE(tenantid::text,'NULL (unscoped)') AS tenant_scope + FROM doormile_auth + ORDER BY tenantid NULLS FIRST, email`) + + section("5. Total bookings (does the console's page budget still truncate?)") + rows(db, `SELECT COUNT(*) AS total_bookings FROM pickupbookings`) +} + +func section(title string) { fmt.Printf("\n===== %s =====\n", title) } + +func rows(db *sql.DB, query string) { + rs, err := db.Query(query) + if err != nil { + fmt.Println(" query failed:", err) + return + } + defer rs.Close() + + cols, _ := rs.Columns() + fmt.Println(" " + join(cols, " | ")) + + for rs.Next() { + vals := make([]interface{}, len(cols)) + ptrs := make([]interface{}, len(cols)) + for i := range vals { + ptrs[i] = &vals[i] + } + if err := rs.Scan(ptrs...); err != nil { + fmt.Println(" scan:", err) + return + } + out := make([]string, len(cols)) + for i, v := range vals { + switch t := v.(type) { + case nil: + out[i] = "NULL" + case []byte: + out[i] = string(t) + default: + out[i] = fmt.Sprint(t) + } + } + fmt.Println(" " + join(out, " | ")) + } +} + +func join(parts []string, sep string) string { + s := "" + for i, p := range parts { + if i > 0 { + s += sep + } + s += p + } + return s +} + +func env(k, fallback string) string { + if v := os.Getenv(k); v != "" { + return v + } + return fallback +}