diff --git a/controllers/booking_assignment_service.go b/controllers/booking_assignment_service.go index 71c7e2a..852dfd5 100644 --- a/controllers/booking_assignment_service.go +++ b/controllers/booking_assignment_service.go @@ -121,6 +121,12 @@ func publishAssignmentUpdate(booking *models.PickupBooking, milerUserID int) { "miler_id", milerUserID, "booking_id", booking.Bookingid, "error", err) } } + + // Customer push, the same path auto-assign uses (cxstage → doormile_cx device + // tokens). Manual assignment recorded the CxStageAssigned stage but sent the + // customer nothing, so a console/hub assignment left the customer with no + // "miler assigned" notification while auto-assign sent one. + cxstage.Notify(booking.Bookingid, nil, constants.CxStageAssigned) } // AssignMilerToBooking is the single source of truth for manually assigning a diff --git a/controllers/cxAuthController.go b/controllers/cxAuthController.go index 9fa56cd..83c6763 100644 --- a/controllers/cxAuthController.go +++ b/controllers/cxAuthController.go @@ -504,9 +504,14 @@ func CxLogout(c *fiber.Ctx) error { now := time.Now() if strings.TrimSpace(req.RefreshToken) != "" { - db.DB.Model(&models.CustomerRefreshToken{}). + // A failed revoke must not answer signedOut — the 60-day refresh token + // would stay valid while the customer believes they logged out. + if err := db.DB.Model(&models.CustomerRefreshToken{}). Where("tokenhash = ? AND appcustomerid = ?", hashToken(req.RefreshToken), customerID). - Update("revokedat", now) + Update("revokedat", now).Error; err != nil { + utils.Error("CxLogout: could not revoke the presented token", "customer_id", customerID, "error", err) + return utils.CxInternal(c) + } } else { // No token supplied — sign out everywhere rather than leave a session // the customer believes they ended. @@ -514,8 +519,12 @@ func CxLogout(c *fiber.Ctx) error { } if strings.TrimSpace(req.DeviceToken) != "" { - db.DB.Where("appcustomerid = ? AND token = ?", customerID, req.DeviceToken). - Delete(&models.CustomerDevice{}) + if err := db.DB.Where("appcustomerid = ? AND token = ?", customerID, req.DeviceToken). + Delete(&models.CustomerDevice{}).Error; err != nil { + // The session is already revoked above; a stuck device row only means a + // stray push, so log and still report signed out rather than fail. + utils.Warn("CxLogout: could not remove device token", "customer_id", customerID, "error", err) + } } return utils.CxOK(c, fiber.Map{"signedOut": true}) diff --git a/controllers/cxBookingController.go b/controllers/cxBookingController.go index 10571fc..492015d 100644 --- a/controllers/cxBookingController.go +++ b/controllers/cxBookingController.go @@ -91,6 +91,32 @@ type cxCreateBookingRequest struct { Remarks string `json:"remarks"` } +// cxEstimateMatchesQuote reports whether a customer-supplied price band is close +// enough to the server's own quote to be trusted as the agreed price. It guards +// the stored Estimatedprice — which becomes ridercharges at completion — against +// a tampered request body while still honouring an honest estimate that came +// from our estimate endpoint. Rejects negatives, inverted bands, and — when the +// server could not price the pickup (zero quote) — any client number at all, +// since with no server figure to check against the client's would be unbounded. +func cxEstimateMatchesQuote(clientMin, clientMax, quoteMin, quoteMax int) bool { + if clientMin < 0 || clientMax < clientMin { + return false + } + serverMid := float64(quoteMin+quoteMax) / 2 + if serverMid <= 0 { + return false + } + clientMid := float64(clientMin+clientMax) / 2 + diff := clientMid - serverMid + if diff < 0 { + diff = -diff + } + // 15% of the server midpoint absorbs rounding and minor pricing drift between + // the estimate call and confirm, without letting a materially different + // number through. + return diff <= 0.15*serverMid +} + // CreateCxBooking creates the pickup. func CreateCxBooking(c *fiber.Ctx) error { customerID := c.Locals("userid").(int) @@ -228,9 +254,22 @@ func CreateCxBooking(c *fiber.Ctx) error { quote := quoteCxPickup(req.Pickup.Lat, req.Pickup.Lng, estimateDestinations) estimateMin, estimateMax := quote.Min, quote.Max if req.Estimate != nil && req.Estimate.Max > 0 { - // The customer's number wins. They agreed to what was on their screen, - // and re-pricing at confirm time would quietly change the deal. - estimateMin, estimateMax = req.Estimate.Min, req.Estimate.Max + if cxEstimateMatchesQuote(req.Estimate.Min, req.Estimate.Max, quote.Min, quote.Max) { + // The customer's number wins — but only when it agrees with what the + // server independently prices for this pickup. An honest app took its + // estimate from our own estimate endpoint, so it matches; re-pricing at + // confirm time would quietly change the deal for that customer. A + // tampered body (e.g. {min:1,max:1}) does NOT match and must never + // stand, because this midpoint becomes Estimatedprice, which the pickup + // and every handover leg copy verbatim into bookingassignments.ridercharges + // — the miler's pay and the tenant's bill — with no weight re-price. + estimateMin, estimateMax = req.Estimate.Min, req.Estimate.Max + } else { + utils.Warn("CreateCxBooking: client estimate rejected, pricing from server quote", + "customer_id", customerID, + "client_min", req.Estimate.Min, "client_max", req.Estimate.Max, + "quote_min", quote.Min, "quote_max", quote.Max) + } } now := utils.DBNow() diff --git a/controllers/hubController.go b/controllers/hubController.go index 99de847..e4e106d 100644 --- a/controllers/hubController.go +++ b/controllers/hubController.go @@ -561,7 +561,9 @@ func CreateInboundScan(c *fiber.Ctx) error { } } - now := time.Now() + // IST wall-clock (DBNow), so inwardedat matches createdat/updatedat and the + // earnings/reconciliation windows that compare against it. + now := utils.DBNow() consignment.Status = constants.ConsignmentInwardedAtHub consignment.Currenthubid = &hubID consignment.Condition = req.Condition diff --git a/controllers/hubInboundController.go b/controllers/hubInboundController.go index dd3aa69..f2695e7 100644 --- a/controllers/hubInboundController.go +++ b/controllers/hubInboundController.go @@ -4,7 +4,6 @@ import ( "fmt" "strconv" "strings" - "time" "doormile/constants" "doormile/db" @@ -204,7 +203,11 @@ func ReconcileHubInbound(c *fiber.Ctx) error { return utils.NotFound(c, "consignment not found") } - now := time.Now() + // IST wall-clock (DBNow) to match the other consignment timestamps. A single + // transaction so a failed audit insert can't leave a state change with no + // history row and still answer 200. + now := utils.DBNow() + tx := db.DB.Begin() if !*req.Received { description := strings.TrimSpace(req.Remarks) @@ -224,16 +227,23 @@ func ReconcileHubInbound(c *fiber.Ctx) error { Status: constants.ExceptionOpen, Createdby: staffAccountID, } - if err := db.DB.Create(&exception).Error; err != nil { + if err := tx.Create(&exception).Error; err != nil { + tx.Rollback() return utils.Internal(c, "failed to raise handover exception") } - db.DB.Create(&models.ConsignmentHistory{ + if err := tx.Create(&models.ConsignmentHistory{ Consignmentid: consignment.Consignmentid, Hubid: &hubID, Eventstatus: consignment.Status, Remarks: fmt.Sprintf("Handover disputed at base by hub staff account %d: %s", staffAccountID, description), - }) + }).Error; err != nil { + tx.Rollback() + return utils.Internal(c, "failed to record dispute history") + } + if err := tx.Commit().Error; err != nil { + return utils.Internal(c, "failed to record the dispute") + } return utils.OK(c, fiber.Map{ "consignmentid": consignment.Consignmentid, @@ -258,7 +268,8 @@ func ReconcileHubInbound(c *fiber.Ctx) error { if consignment.Inwardedat == nil { consignment.Inwardedat = &now } - if err := db.DB.Save(&consignment).Error; err != nil { + if err := tx.Save(&consignment).Error; err != nil { + tx.Rollback() return utils.Internal(c, "failed to record receipt") } @@ -267,12 +278,19 @@ func ReconcileHubInbound(c *fiber.Ctx) error { if remarks == "" { remarks = "Physical receipt confirmed at base" } - db.DB.Create(&models.ConsignmentHistory{ + if err := tx.Create(&models.ConsignmentHistory{ Consignmentid: consignment.Consignmentid, Hubid: &hubID, Eventstatus: constants.ConsignmentInwardedAtHub, Remarks: fmt.Sprintf("%s (hub staff account %d)", remarks, staffAccountID), - }) + }).Error; err != nil { + tx.Rollback() + return utils.Internal(c, "failed to record receipt history") + } + } + + if err := tx.Commit().Error; err != nil { + return utils.Internal(c, "failed to record the receipt") } return utils.OK(c, fiber.Map{ diff --git a/controllers/logisticsHandoverController.go b/controllers/logisticsHandoverController.go index 3344749..837ac91 100644 --- a/controllers/logisticsHandoverController.go +++ b/controllers/logisticsHandoverController.go @@ -7,7 +7,6 @@ import ( "sort" "strconv" "strings" - "time" "doormile/constants" "doormile/db" @@ -26,6 +25,12 @@ import ( // the app's wording, and nothing in the app's wording should leak back in here. // -------------------- +// maxHandoverBaseKM bounds how far a rider may be from a base they EXPLICITLY +// name at handover. A rider stands at the base they hand into, so a named base +// this far from their reported position is a wrong id (a different city), not a +// real handover. Generous enough to never reject two bases in one metro. +const maxHandoverBaseKM = 50.0 + // hubHandoverEnabled gates the two-step hub flow: a hub-routed parcel stops at // Created — collected, in the rider's hands, on its way to a base — and only // reaches Inwarded_at_Hub when the handover is actually recorded, by the rider @@ -372,6 +377,32 @@ func MilerInwardConsignmentAtHub(c *fiber.Ctx) error { return utils.Fail(c, fiber.StatusNotFound, constants.ErrHubNotFound, "hub_id does not match a known base") } + lat, lon := 0.0, 0.0 + if req.Latitude != nil { + lat = *req.Latitude + } else if req.Lat != nil { + lat = *req.Lat + } + if req.Longitude != nil { + lon = *req.Longitude + } else if req.Lon != nil { + lon = *req.Lon + } + + // Guard a fat-fingered base id from silently rerouting the parcel to a base in + // the wrong city. Only a base the rider EXPLICITLY names (not the routed + // default) is checked, and only when they report their position and the base + // has real coordinates: a rider is physically at the base they hand into, so a + // named base far from where they stand is a wrong id, not a real handover. + riderNamedHub := req.HubID != nil || req.HubIDAlt != nil + routedHub := consignment.Currenthubid != nil && hub.Hubid == *consignment.Currenthubid + if riderNamedHub && !routedHub && (lat != 0 || lon != 0) && hub.Latitude != 0 && hub.Longitude != 0 { + if km := haversineKM(lat, lon, hub.Latitude, hub.Longitude); km > maxHandoverBaseKM { + return utils.Fail(c, fiber.StatusBadRequest, constants.ErrInvalidState, + fmt.Sprintf("selected base %s is %.0f km from your location — check the base before handing over", hub.Hubname, km)) + } + } + // Already inwarded: answer with the state that stands rather than failing, so // a retry after a dropped response confirms rather than errors. This is also // what a rider on the compatibility flow hits every time — there, @@ -397,19 +428,10 @@ func MilerInwardConsignmentAtHub(c *fiber.Ctx) error { fmt.Sprintf("consignment is %s — it cannot be handed over at a base from this state", consignment.Status)) } - lat, lon := 0.0, 0.0 - if req.Latitude != nil { - lat = *req.Latitude - } else if req.Lat != nil { - lat = *req.Lat - } - if req.Longitude != nil { - lon = *req.Longitude - } else if req.Lon != nil { - lon = *req.Lon - } - - now := time.Now() + // IST wall-clock, matching createdat/updatedat and the DBNow() convention, so + // inwardedat lines up with the other timestamps base reconciliation and the + // earnings "today" window compare it against. + now := utils.DBNow() tx := db.DB.Begin() consignment.Status = constants.ConsignmentInwardedAtHub @@ -448,40 +470,69 @@ func MilerInwardConsignmentAtHub(c *fiber.Ctx) error { // multi-destination pickup, so joining on it found nothing for orders 2..N // — and an intercity rider handing in the second parcel of a three-stop // pickup had their assignment left open and their distance recorded as zero. + // Close the rider's booking-level assignment and free them ONLY once every + // parcel from this pickup has left their hands. A customer-app booking is one + // booking → N destinations → N consignments but a single BookingAssignment; + // closing on the FIRST handover freed the rider and dropped the remaining + // stops from the sequencer while parcels 2..N were still on them, crediting + // only the first leg. So finalize only when no consignment of this booking is + // still in a rider-carrying state (this one is already Inwarded_at_Hub above). + finalizeRiderLeg := true if _, bookingPtr, ok := cxDestinationForConsignment(consignment.Consignmentid); ok && bookingPtr != nil { booking := *bookingPtr - dropLat, dropLon := lat, lon - if dropLat == 0 && dropLon == 0 { - dropLat, dropLon = hub.Latitude, hub.Longitude - } - riderKms := haversineKM(consignment.Pickuplatitude, consignment.Pickuplongitude, dropLat, dropLon) - orderAmount := 0.0 - var serviceOpt models.BookingServiceOption - if tx.Where("bookingid = ?", booking.Bookingid).Order("createdat DESC"). - First(&serviceOpt).Error == nil { - orderAmount = serviceOpt.Estimatedprice - } - - if err := tx.Model(&models.BookingAssignment{}). - Where("bookingid = ? AND mileruserid = ? AND assignmentstatus IN ?", - booking.Bookingid, milerUserID, - []string{constants.AssignmentAssigned, constants.AssignmentAccepted}). - Updates(map[string]interface{}{ - "assignmentstatus": constants.AssignmentCompleted, - "completedat": now, - "riderkms": riderKms, - "ridercharges": orderAmount, - }).Error; err != nil { + var carrying int64 + if err := tx.Model(&models.Consignment{}). + Joins("JOIN bookingdestinations bd ON bd.consignmentid = consignments.consignmentid"). + Where("bd.bookingid = ? AND consignments.status IN ?", + booking.Bookingid, + []string{constants.ConsignmentCreated, constants.ConsignmentCollectedByMiler, constants.ConsignmentOutForDelivery}). + Count(&carrying).Error; err != nil { tx.Rollback() - return utils.Internal(c, "failed to close assignment") + return utils.Internal(c, "failed to check the booking's remaining parcels") + } + + if carrying > 0 { + // Rider still carries other parcels from this pickup: leave the + // assignment open and the rider on the job. The leg is credited and the + // rider freed at the final handover. + finalizeRiderLeg = false + } else { + dropLat, dropLon := lat, lon + if dropLat == 0 && dropLon == 0 { + dropLat, dropLon = hub.Latitude, hub.Longitude + } + riderKms := haversineKM(consignment.Pickuplatitude, consignment.Pickuplongitude, dropLat, dropLon) + + orderAmount := 0.0 + var serviceOpt models.BookingServiceOption + if tx.Where("bookingid = ?", booking.Bookingid).Order("createdat DESC"). + First(&serviceOpt).Error == nil { + orderAmount = serviceOpt.Estimatedprice + } + + if err := tx.Model(&models.BookingAssignment{}). + Where("bookingid = ? AND mileruserid = ? AND assignmentstatus IN ?", + booking.Bookingid, milerUserID, + []string{constants.AssignmentAssigned, constants.AssignmentAccepted}). + Updates(map[string]interface{}{ + "assignmentstatus": constants.AssignmentCompleted, + "completedat": now, + "riderkms": riderKms, + "ridercharges": orderAmount, + }).Error; err != nil { + tx.Rollback() + return utils.Internal(c, "failed to close assignment") + } } } - if err := tx.Model(&models.MilerProfile{}).Where("userid = ?", milerUserID). - Update("availabilitystatus", constants.MilerAvailable).Error; err != nil { - tx.Rollback() - return utils.Internal(c, "failed to update miler availability") + if finalizeRiderLeg { + if err := tx.Model(&models.MilerProfile{}).Where("userid = ?", milerUserID). + Update("availabilitystatus", constants.MilerAvailable).Error; err != nil { + tx.Rollback() + return utils.Internal(c, "failed to update miler availability") + } } // The customer's "In transit" milestone. Recorded against THIS order, not diff --git a/internal/assignment/crm_assignment.go b/internal/assignment/crm_assignment.go index 925920e..f099960 100644 --- a/internal/assignment/crm_assignment.go +++ b/internal/assignment/crm_assignment.go @@ -256,7 +256,9 @@ func commitAssignment(booking *models.PickupBooking, candidate *milerCandidate, publishAssignment(booking, milerUserID) notifyMilerNewAssignment(candidate.profile, booking.Bookingid) - notifyCustomerMilerAssigned(booking, candidate.profile.Displayname) + // Customer push goes through cxstage.Notify above (the doormile_cx device + // tokens). notifyCustomerMilerAssigned targeted the older AppCustomer.Devicetoken + // and firing both sent the customer two near-identical "miler assigned" pushes. // Order the rider's stops now this one is added. No-op below two active stops; // best-effort and off this goroutine's critical path.