updates on the env and pagination
This commit is contained in:
@@ -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")
|
||||
|
||||
88
controllers/admin_pagesize_test.go
Normal file
88
controllers/admin_pagesize_test.go
Normal file
@@ -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()
|
||||
}
|
||||
Reference in New Issue
Block a user