fix: per-site attribution needs its own column, not pickuplocationid

Caught by testing the previous commit against production: creating a booking
with a resolved site failed with

  pickupbookings_pickuplocationid_fkey
  FOREIGN KEY (pickuplocationid) REFERENCES appcustomerlocations(...)

pickuplocationid is the *customer's* saved address, a B2C concept. It never
referred to the client company's own kitchens or branches. The pre-existing
code that validated an incoming pickuplocationid against TenantLocation was
wrong on the same point and would have 500'd for any caller that used it — it
had simply never been called with a value.

Adds tenantlocationid to pickupbookings and consignments (nullable, indexed,
additive via AutoMigrate), carried across at pickup, and points the reporting
filter, the by_location breakdown and the Unattributed bucket at it.

The booking request accepts tenantlocationid, and still accepts
pickuplocationid as an alias so anything written against the earlier docs
starts working instead of failing.

Also gofmt on the two model files touched; booking.go was already failing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Suriya
2026-08-06 13:08:57 +05:30
parent 0c407e5b27
commit 90fa4fbb74
6 changed files with 108 additions and 70 deletions

View File

@@ -356,7 +356,7 @@ func GetAdminReports(c *fiber.Ctx) error {
bookingScope := func() *gorm.DB {
q := scopeToTenant(db.DB.Model(&models.PickupBooking{}), "tenantid", ownTenant)
return scopeToLocation(q, "pickuplocationid", locationID)
return scopeToLocation(q, "tenantlocationid", locationID)
}
var totalBookings int64
@@ -374,7 +374,7 @@ func GetAdminReports(c *fiber.Ctx) error {
consignmentQuery := scopeToLocation(
scopeToTenant(db.DB.Model(&models.Consignment{}), "tenantid", ownTenant),
"pickuplocationid", locationID).
"tenantlocationid", locationID).
Where("createdat BETWEEN ? AND ?", from, to)
if hubID != "" {
consignmentQuery = consignmentQuery.Where("currenthubid = ?", hubID)
@@ -639,11 +639,11 @@ func GetLocationSummary(c *fiber.Ctx) error {
// each of the client's own locations raised in the range, how many reached a
// consignment, and what COD came back.
//
// Bookings with no pickuplocationid are reported as a single "Unattributed"
// Bookings with no tenantlocationid are reported as a single "Unattributed"
// row rather than dropped, so the per-site figures still add up to the summary
// total. That row is not noise — it is every booking created without naming
// its site, and it should shrink to zero as the console starts sending
// pickuplocationid.
// tenantlocationid.
func locationBreakdown(tenantID, locationID int, from, to time.Time) []fiber.Map {
// Only meaningful within one client. Across the whole network the rows
// would be a mix of every tenant's sites, which is a screen nobody asked
@@ -680,7 +680,7 @@ func locationBreakdown(tenantID, locationID int, from, to time.Time) []fiber.Map
COALESCE(SUM(p.paid), 0) AS cod_collected
FROM tenantlocations tl
LEFT JOIN pickupbookings b
ON b.pickuplocationid = tl.tenantlocationid
ON b.tenantlocationid = tl.tenantlocationid
AND b.createdat BETWEEN ? AND ?
LEFT JOIN (
SELECT bookingid, SUM(amount) AS paid
@@ -723,7 +723,7 @@ func locationBreakdown(tenantID, locationID int, from, to time.Time) []fiber.Map
if locationID == 0 {
var unattributed int64
db.DB.Model(&models.PickupBooking{}).
Where("tenantid = ? AND pickuplocationid IS NULL AND createdat BETWEEN ? AND ?",
Where("tenantid = ? AND tenantlocationid IS NULL AND createdat BETWEEN ? AND ?",
tenantID, from, to).Count(&unattributed)
if unattributed > 0 {
out = append(out, fiber.Map{
@@ -1977,10 +1977,16 @@ type AdminBookingRequest struct {
Appcustomerid int `json:"appcustomerid"`
CustomerPhone string `json:"customer_phone"`
CustomerName string `json:"customer_name"`
// Pickuplocationid names the client site the parcel is collected from — a
// Tenantlocationid names the client site the parcel is collected from — a
// DailyGrubs kitchen, for instance. Optional, but supplying it lets the
// address/pincode/coordinates be filled from the stored location instead of
// retyped, and is the only thing that makes per-site reporting possible.
// address/pincode/coordinates be filled from the stored site instead of
// retyped, and is what per-site reporting groups by. When it is omitted the
// site is inferred from the pickup coordinates or address.
Tenantlocationid *int `json:"tenantlocationid"`
// Pickuplocationid is accepted only as an alias for the field above, for
// callers written against the earlier docs. It is never stored as-is: the
// column of that name foreign-keys to appcustomerlocations, not to a
// client's sites.
Pickuplocationid *int `json:"pickuplocationid"`
Pickupaddress string `json:"pickupaddress"`
Pickuppincode string `json:"pickuppincode"`
@@ -2027,18 +2033,30 @@ func createExpressBooking(req AdminBookingRequest) (*models.PickupBooking, error
return nil, &expressBookingValidationError{"tenantid does not match a known tenant"}
}
// A named pickup location fills in whatever the caller left blank, so the
// console can send a kitchen id instead of restating its address every time.
// It must belong to the booking's tenant — otherwise one client could book
// against another client's site.
if req.Pickuplocationid != nil {
// A named pickup site fills in whatever the caller left blank, so the console
// can send a kitchen id instead of restating its address every time. It must
// belong to the booking's tenant — otherwise one client could book against
// another client's site.
//
// `pickuplocationid` is accepted as an alias here purely for callers written
// against the earlier documentation. It is the wrong column: it foreign-keys
// to appcustomerlocations, so a tenantlocations id in it fails the insert.
// Both names resolve to Tenantlocationid.
siteID := req.Tenantlocationid
if siteID == nil {
siteID = req.Pickuplocationid
}
req.Pickuplocationid = nil
if siteID != nil {
var loc models.TenantLocation
if err := db.DB.Where("tenantlocationid = ?", *req.Pickuplocationid).First(&loc).Error; err != nil {
return nil, &expressBookingValidationError{"pickuplocationid does not match a known location"}
if err := db.DB.Where("tenantlocationid = ?", *siteID).First(&loc).Error; err != nil {
return nil, &expressBookingValidationError{"tenantlocationid does not match a known location"}
}
if loc.Tenantid != req.Tenantid {
return nil, &expressBookingValidationError{"pickuplocationid does not belong to this tenant"}
return nil, &expressBookingValidationError{"tenantlocationid does not belong to this tenant"}
}
req.Tenantlocationid = siteID
if req.Pickupaddress == "" {
req.Pickupaddress = loc.Address
}
@@ -2060,10 +2078,8 @@ func createExpressBooking(req AdminBookingRequest) (*models.PickupBooking, error
// pickup actually is. Without this the field stays null — as it did on every
// booking in the system — and per-site reporting has nothing to group by,
// because the console sends a kitchen's address rather than its id.
if req.Pickuplocationid == nil {
if matched := matchTenantLocation(req.Tenantid, req.Pickupaddress, req.Pickuplatitude, req.Pickuplongitude); matched != nil {
req.Pickuplocationid = matched
}
if req.Tenantlocationid == nil {
req.Tenantlocationid = matchTenantLocation(req.Tenantid, req.Pickupaddress, req.Pickuplatitude, req.Pickuplongitude)
}
tx := db.DB.Begin()
@@ -2099,6 +2115,7 @@ func createExpressBooking(req AdminBookingRequest) (*models.PickupBooking, error
Tenantid: &tenantID,
Appcustomerid: customerID,
Pickuplocationid: req.Pickuplocationid,
Tenantlocationid: req.Tenantlocationid,
Pickupaddress: req.Pickupaddress,
Pickuppincode: req.Pickuppincode,
Pickuplatitude: req.Pickuplatitude,