diff --git a/controllers/adminController.go b/controllers/adminController.go index a902c49..7e20abc 100644 --- a/controllers/adminController.go +++ b/controllers/adminController.go @@ -76,6 +76,41 @@ func scopeToTenant(query *gorm.DB, column string, tenantID int) *gorm.DB { return query.Where(column+" = ?", tenantID) } +// requestedLocationID reads ?locationid= and proves the site belongs to the +// tenant the request is scoped to. Returns 0 for "all sites". The second value +// is "ok", not an error, for the reason on effectiveTenantID — utils.NotFound +// returns nil, so it cannot be used as a refusal signal. +// +// The ownership check matters because a location id is just an integer in a +// query string: without it a client could read another client's per-site +// numbers by guessing ids, which is the whole tenant boundary undone by one +// unvalidated param. +func requestedLocationID(c *fiber.Ctx, tenantID int) (locationID int, ok bool) { + locationID = c.QueryInt("locationid", 0) + if locationID == 0 { + return 0, true + } + + var loc models.TenantLocation + if err := db.DB.Select("tenantlocationid", "tenantid"). + Where("tenantlocationid = ?", locationID).First(&loc).Error; err != nil { + return 0, false + } + if tenantID != 0 && loc.Tenantid != tenantID { + return 0, false + } + return locationID, true +} + +// scopeToLocation narrows a query to one client site, no-oping when locationID +// is 0. +func scopeToLocation(query *gorm.DB, column string, locationID int) *gorm.DB { + if locationID == 0 { + return query + } + return query.Where(column+" = ?", locationID) +} + // milerUserIDsForTenant lists the appusers.userid of a tenant's riders. Riders // belong to a client through their appusers row, not the miler profile. func milerUserIDsForTenant(tenantID int) []int { @@ -310,21 +345,36 @@ func GetAdminReports(c *fiber.Ctx) error { return utils.Forbidden(c, "you can only view your own tenant") } + // ?locationid= narrows every figure to one of the client's sites — a single + // kitchen, branch or depot. This was jupiter's getreportsummary locationid + // param; without it a food client with 23 kitchens can only see one number + // for all of them. + locationID, locOK := requestedLocationID(c, ownTenant) + if !locOK { + return utils.NotFound(c, "location not found") + } + + bookingScope := func() *gorm.DB { + q := scopeToTenant(db.DB.Model(&models.PickupBooking{}), "tenantid", ownTenant) + return scopeToLocation(q, "pickuplocationid", locationID) + } + var totalBookings int64 - scopeToTenant(db.DB.Model(&models.PickupBooking{}), "tenantid", ownTenant). - Where("createdat BETWEEN ? AND ?", from, to).Count(&totalBookings) + bookingScope().Where("createdat BETWEEN ? AND ?", from, to).Count(&totalBookings) var delivered int64 - scopeToTenant(db.DB.Model(&models.PickupBooking{}), "tenantid", ownTenant). + bookingScope(). Where("status = ? AND updatedat BETWEEN ? AND ?", constants.BookingConvertedConsignment, from, to). Count(&delivered) var cancelled int64 - scopeToTenant(db.DB.Model(&models.PickupBooking{}), "tenantid", ownTenant). + bookingScope(). Where("status = ? AND updatedat BETWEEN ? AND ?", constants.BookingCancelled, from, to). Count(&cancelled) - consignmentQuery := scopeToTenant(db.DB.Model(&models.Consignment{}), "tenantid", ownTenant). + consignmentQuery := scopeToLocation( + scopeToTenant(db.DB.Model(&models.Consignment{}), "tenantid", ownTenant), + "pickuplocationid", locationID). Where("createdat BETWEEN ? AND ?", from, to) if hubID != "" { consignmentQuery = consignmentQuery.Where("currenthubid = ?", hubID) @@ -414,6 +464,11 @@ func GetAdminReports(c *fiber.Ctx) error { byTenant = append(byTenant, fiber.Map{"tenantid": r.Tenantid, "tenantname": r.Tenantname, "bookings": r.Bookings}) } + // ---- by location: what went out of each of the client's own sites ---- + // For a food client this is "how many orders left which kitchen", which is + // the question a single network-wide total cannot answer. + byLocation := locationBreakdown(ownTenant, locationID, from, to) + // ---- by rider: completed stops/kms/earnings per rider in range, // optionally scoped to one hub ---- type riderRow struct { @@ -437,14 +492,23 @@ func GetAdminReports(c *fiber.Ctx) error { AND ba.assignmentstatus = ? AND ba.completedat BETWEEN ? AND ? ` args := []interface{}{constants.AssignmentCompleted, from, to} + where := []string{} if hubID != "" { - riderQuery += " WHERE mp.hubid = ? " + where = append(where, "mp.hubid = ?") args = append(args, hubID) } - riderQuery += " GROUP BY mp.userid, mp.displayname ORDER BY completed_stops DESC LIMIT 50" - if isDoormileConsoleStaff(c) { - db.DB.Raw(riderQuery, args...).Scan(&riderRows) + // A client sees its own riders' numbers, not the network's. Previously this + // list was simply empty for every client login, which reads as "your riders + // did nothing" rather than "this view isn't for you". + if ownTenant != 0 { + where = append(where, "mp.userid IN (SELECT userid FROM appusers WHERE tenantid = ? AND roleid = 5)") + args = append(args, ownTenant) } + if len(where) > 0 { + riderQuery += " WHERE " + strings.Join(where, " AND ") + } + riderQuery += " GROUP BY mp.userid, mp.displayname ORDER BY completed_stops DESC LIMIT 50" + db.DB.Raw(riderQuery, args...).Scan(&riderRows) byRider := make([]fiber.Map, 0, len(riderRows)) for _, r := range riderRows { @@ -455,8 +519,10 @@ func GetAdminReports(c *fiber.Ctx) error { } return utils.OK(c, fiber.Map{ - "from": from.Format("2006-01-02"), - "to": to.Format("2006-01-02"), + "from": from.Format("2006-01-02"), + "to": to.Format("2006-01-02"), + "tenantid": ownTenant, + "locationid": locationID, "summary": fiber.Map{ "total_bookings": totalBookings, "delivered": delivered, @@ -466,12 +532,216 @@ func GetAdminReports(c *fiber.Ctx) error { "open_exceptions": openExceptions, "completion_rate": completionRate, }, - "by_hub": byHub, - "by_tenant": byTenant, - "by_rider": byRider, + "by_hub": byHub, + "by_tenant": byTenant, + "by_location": byLocation, + "by_rider": byRider, }) } +// matchTenantLocation resolves which of a client's sites a pickup came from, +// for callers that send an address instead of a location id. Returns nil when +// nothing matches confidently — a wrong attribution is worse than none, since +// it silently moves orders between kitchens on the report. +// +// Coordinates first: they are unambiguous where an address string is not +// ("Vidhya kitchen, Ritham Tours & Travels, Peelamedu" versus the same site +// stored as "Ritham Tours and Travels, Peelamedu"). Address matching is only a +// fallback for bookings that arrive without coordinates. +func matchTenantLocation(tenantID int, address string, lat, lon float64) *int { + var locations []models.TenantLocation + if err := db.DB.Where("tenantid = ?", tenantID).Find(&locations).Error; err != nil || len(locations) == 0 { + return nil + } + + // 150m: tight enough that two kitchens on the same street stay distinct, + // loose enough to absorb the drift between a stored pin and the one the + // console sends. + const matchRadiusKM = 0.15 + + if lat != 0 || lon != 0 { + best := -1 + bestDist := matchRadiusKM + for i, loc := range locations { + if loc.Latitude == 0 && loc.Longitude == 0 { + continue + } + if d := haversineKM(lat, lon, loc.Latitude, loc.Longitude); d < bestDist { + best, bestDist = i, d + } + } + if best >= 0 { + id := locations[best].Tenantlocationid + return &id + } + } + + if address == "" { + return nil + } + needle := strings.ToLower(strings.TrimSpace(address)) + for _, loc := range locations { + stored := strings.ToLower(strings.TrimSpace(loc.Address)) + if stored == "" { + continue + } + // Exact either way round, so "Vidhya kitchen,
" still resolves + // when the stored row holds just the address. Substring matching is + // deliberately not loosened past this — a short stored address would + // otherwise swallow unrelated pickups. + if stored == needle || strings.Contains(needle, stored) { + id := loc.Tenantlocationid + return &id + } + } + return nil +} + +// GetLocationSummary is the standalone per-site table — jupiter's +// getlocationsummary. Same rows as the report's by_location block, without +// pulling the whole report, so a "Kitchens" screen can load on its own. +// +// GET /admin/locations/summary?tenantid=&locationid=&from=&to= +func GetLocationSummary(c *fiber.Ctx) error { + from, to, err := parseHubDateRange(c) + if err != nil { + return utils.BadRequest(c, err.Error()) + } + + tenantID, allowed := effectiveTenantID(c) + if !allowed { + return utils.Forbidden(c, "you can only view your own tenant") + } + // Doormile staff must name a client: per-site rows across every tenant at + // once are a mix of unrelated sites, not a report. + if tenantID == 0 { + return utils.BadRequest(c, "tenantid is required — per-site figures are reported within one client") + } + + locationID, locOK := requestedLocationID(c, tenantID) + if !locOK { + return utils.NotFound(c, "location not found") + } + + rows := locationBreakdown(tenantID, locationID, from, to) + return c.JSON(fiber.Map{ + "success": true, + "data": rows, + "total": len(rows), + "tenantid": tenantID, + "locationid": locationID, + "from": from.Format("2006-01-02"), + "to": to.Format("2006-01-02"), + }) +} + +// locationBreakdown returns per-site totals for a tenant: how many bookings +// 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" +// 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. +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 + // for; Doormile staff pass ?tenantid= to get this. + if tenantID == 0 { + return []fiber.Map{} + } + + type locRow struct { + Tenantlocationid int `gorm:"column:tenantlocationid"` + Locationname string `gorm:"column:locationname"` + Address string `gorm:"column:address"` + Pincode string `gorm:"column:pincode"` + Bookings int64 `gorm:"column:bookings"` + Delivered int64 `gorm:"column:delivered"` + Cancelled int64 `gorm:"column:cancelled"` + CodCollected float64 `gorm:"column:cod_collected"` + } + + // The date range sits in the JOIN, not a WHERE, so a site with no orders in + // the window still appears with zeros instead of vanishing from the report. + // + // Payments are pre-aggregated per booking before joining. Joining + // bookingpayments directly would fan out — a booking with two payment rows + // would be counted as two bookings. + sql := ` + SELECT tl.tenantlocationid AS tenantlocationid, + tl.locationname AS locationname, + tl.address AS address, + tl.pincode AS pincode, + COUNT(b.bookingid) AS bookings, + COUNT(b.bookingid) FILTER (WHERE b.status = ?) AS delivered, + COUNT(b.bookingid) FILTER (WHERE b.status = ?) AS cancelled, + COALESCE(SUM(p.paid), 0) AS cod_collected + FROM tenantlocations tl + LEFT JOIN pickupbookings b + ON b.pickuplocationid = tl.tenantlocationid + AND b.createdat BETWEEN ? AND ? + LEFT JOIN ( + SELECT bookingid, SUM(amount) AS paid + FROM bookingpayments + WHERE paymentstatus = ? + GROUP BY bookingid + ) p ON p.bookingid = b.bookingid + WHERE tl.tenantid = ?` + args := []interface{}{ + constants.BookingConvertedConsignment, constants.BookingCancelled, + from, to, constants.PaymentStatusPaid, tenantID, + } + if locationID != 0 { + sql += ` AND tl.tenantlocationid = ?` + args = append(args, locationID) + } + sql += ` + GROUP BY tl.tenantlocationid, tl.locationname, tl.address, tl.pincode + ORDER BY bookings DESC, tl.locationname` + + var rows []locRow + db.DB.Raw(sql, args...).Scan(&rows) + + out := make([]fiber.Map, 0, len(rows)+1) + for _, r := range rows { + out = append(out, fiber.Map{ + "tenantlocationid": r.Tenantlocationid, + "locationname": r.Locationname, + "address": r.Address, + "pincode": r.Pincode, + "bookings": r.Bookings, + "delivered": r.Delivered, + "cancelled": r.Cancelled, + "cod_collected": r.CodCollected, + }) + } + + // Anything the client raised without naming a site. Suppressed when the + // caller asked for one specific location. + if locationID == 0 { + var unattributed int64 + db.DB.Model(&models.PickupBooking{}). + Where("tenantid = ? AND pickuplocationid IS NULL AND createdat BETWEEN ? AND ?", + tenantID, from, to).Count(&unattributed) + if unattributed > 0 { + out = append(out, fiber.Map{ + "tenantlocationid": nil, + "locationname": "Unattributed", + "address": "", + "pincode": "", + "bookings": unattributed, + "delivered": 0, + "cancelled": 0, + "cod_collected": 0, + }) + } + } + + return out +} + // -------------------- // APP USERS MANAGEMENT // -------------------- @@ -1786,6 +2056,16 @@ func createExpressBooking(req AdminBookingRequest) (*models.PickupBooking, error return nil, &expressBookingValidationError{"pickup address and pincode are required"} } + // If the caller did not name a site, try to recognise it from where the + // 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 + } + } + tx := db.DB.Begin() customerID := req.Appcustomerid diff --git a/docs/express-console-api.md b/docs/express-console-api.md index 247fd66..349d8d3 100644 --- a/docs/express-console-api.md +++ b/docs/express-console-api.md @@ -1,6 +1,6 @@ # Doormile Express Console — API reference -The console surface only (`/admin/*`). 94 routes: 1 login + 93 authenticated. +The console surface only (`/admin/*`). 95 routes: 1 login + 94 authenticated. Everything is under `https://api.doormile.com/api/v1`. Verified against the build deployed 2026-08-06 12:41 IST. @@ -65,7 +65,7 @@ comment in `routes.go`; the comment is wrong, `AuthMiddleware` is applied. | Method | Path | Notes | |---|---|---| | GET | `/admin/dashboard` | counts + today's numbers | -| GET | `/admin/reports` | `?from=YYYY-MM-DD&to=YYYY-MM-DD`, defaults to today (IST) | +| GET | `/admin/reports` | `?from=&to=&tenantid=&locationid=&hubid=`, defaults to today (IST) | | GET | `/admin/profile` | current account | | GET | `/admin/me` | alias of the above | | PUT | `/admin/profile/password` | `{ "current_password": "...", "new_password": "..." }` — snake_case | @@ -102,12 +102,36 @@ is a client Doormile delivers for. | PUT | `/admin/tenants/:id` | `requiredeliveryotp` is a pointer — omit it to leave the setting alone | | DELETE | `/admin/tenants/:id` | hard delete | | GET | `/admin/tenants/:id/locations` | the client's sites (kitchens, branches, depots) | +| GET | `/admin/locations/summary` | **per-site performance** — `?tenantid=&locationid=&from=&to=` | | POST | `/admin/tenants/:id/locations` | `{ locationname, address, city, state, pincode, latitude, longitude, isprimary, status }` | | PUT | `/admin/tenantlocations/:id` | note: **not** nested under the tenant | `requiredeliveryotp` is opt-in per tenant and **off by default**. DailyGrubs runs without delivery OTP by decision. +### Per-site reporting + +`GET /admin/locations/summary?tenantid=13&from=&to=` returns one row per site: + +```jsonc +{ "tenantlocationid": 13, "locationname": "Vidhya kitchen", + "address": "…", "pincode": "641015", + "bookings": 12, "delivered": 11, "cancelled": 1, "cod_collected": 840 } +``` + +Sites with no orders in the range still appear, with zeros. A trailing +`"Unattributed"` row (`tenantlocationid: null`) carries bookings that never +named a site, so the rows always add up to the report's summary total. + +Doormile staff **must** pass `?tenantid=` here — per-site rows across all +tenants at once aren't a meaningful report, so it 400s without one. + +The same rows appear as `by_location` inside `GET /admin/reports`. + +**Send `pickuplocationid` on bookings.** Attribution depends on it. The server +will try to recognise the site from the pickup coordinates (within 150m) or a +matching address, but an explicit id is exact and always wins. + ## Tenant customers (a client's own end customers) | Method | Path | Body | diff --git a/docs/jupiter2doormile.md b/docs/jupiter2doormile.md index af911bd..6f84a75 100644 --- a/docs/jupiter2doormile.md +++ b/docs/jupiter2doormile.md @@ -90,13 +90,26 @@ Two jupiter bugs that do not carry over, by construction: | jupiter | Doormile | Status | |---|---|---| -| ✅ `GET /deliveries/getreportsummary/?applocationid=&tenantid=&locationid=&fromdate=&todate=` | `GET /admin/reports?from=&to=&tenantid=&hubid=` | **Done** | +| ✅ `GET /deliveries/getreportsummary/?applocationid=&tenantid=&locationid=&fromdate=&todate=` | `GET /admin/reports?from=&to=&tenantid=&locationid=&hubid=` | **Done** | +| ~ `getlocationsummary` | `GET /admin/locations/summary?tenantid=&locationid=&from=&to=` | **Done** | | — | `GET /admin/dashboard?tenantid=` | **Done** | -**One parameter has no equivalent yet: `locationid`.** jupiter could report per -client *site* — per kitchen, for a food client. Doormile carries -`pickuplocationid` on both the booking and the consignment, so the data is -there, but `/admin/reports` does not group or filter by it. See §6. +`locationid` is supported: it narrows every figure to one client site, and +`/admin/reports` now carries a `by_location` block alongside `by_hub`, +`by_tenant` and `by_rider`. + +**Attribution caveat.** Per-site figures depend on `pickuplocationid` being set +on the booking. Every booking created before 2026-08-06 has it null, and the +console sends a kitchen's *address*, not its id. So `createExpressBooking` now +resolves the site itself — nearest stored location within 150m, falling back to +an address match — and unattributed bookings are reported as their own +`"Unattributed"` row rather than dropped, so the per-site rows still add up to +the summary total. Sending `pickuplocationid` explicitly is still better and +always wins. + +**`applocationid` (city) is still not a report parameter.** jupiter had it; +Doormile filters by `hubid` instead. Only matters once one client runs in more +than one city. ### 2.4 Tenants and their sites @@ -207,8 +220,7 @@ rewriting, not repointing. Beyond that: | What | Detail | |---|---| -| **Per-site reporting** | jupiter's `getreportsummary` took `locationid`. Doormile stores `pickuplocationid` on bookings and consignments but neither groups nor filters by it. For a food client this is "how many orders went out of which kitchen" — likely the first thing DailyGrubs asks for. | -| `getlocationsummary` | No per-site rollup endpoint. | +| `applocationid` on reports | jupiter could filter a report by city. Doormile filters by `hubid`. Only bites when one client operates in several cities. | | **Route optimisation** | jupiter used external paid services (`routes.workolik.com`) for multi-stop sequencing. Nothing in Doormile replaces true stop-ordering. `HubBatchAssign` decides *who* gets a booking, not *what order* to run stops in. | | Notifications read-state | `PATCH /miler/notifications/:id/read` is a stub; no table exists. | | `riderkms` / `ridercharges` backfill | Populated on new deliveries only. Rows completed before 2026-08-06 read 0 and will not backfill themselves. | diff --git a/routes/routes.go b/routes/routes.go index 31461cd..f1cee94 100644 --- a/routes/routes.go +++ b/routes/routes.go @@ -222,6 +222,9 @@ func RegisterRoutes(app *fiber.App, cfg *config.Config) { adminAuth.Get("/tenants/:id/locations", controllers.GetTenantLocations) adminAuth.Post("/tenants/:id/locations", controllers.CreateTenantLocation) adminAuth.Put("/tenantlocations/:id", controllers.UpdateTenantLocation) + // Per-site performance — jupiter's getlocationsummary. Needs ?tenantid= for + // Doormile staff; a client login is already pinned to its own sites. + adminAuth.Get("/locations/summary", controllers.GetLocationSummary) // Tenant customers adminAuth.Get("/tenantcustomers", controllers.GetTenantCustomers)