diff --git a/constants/delivery_category.go b/constants/delivery_category.go index 3850592..72e3bff 100644 --- a/constants/delivery_category.go +++ b/constants/delivery_category.go @@ -1,56 +1,76 @@ package constants +import "strings" + // What a client delivers, and what that implies operationally. // // ─── One vocabulary, not two ─────────────────────────────────────────────── // -// These eight values already existed as `validCategories` in -// controllers/doormilePricingController.go and as the comment on -// models.DoormilePricing.Category. Tenants now carry one too, so the set is -// promoted here rather than copied: a second list would drift, and a tenant -// whose category is not a pricing category cannot be priced. +// Categories driven by pricing, onboarding, and logistics. +// Non-perishable goods like Steel, Clothing, Electronics, Automotive have return paths. +// Perishable goods like Food, Meat, and Fresh Produce have NO return option. var DeliveryCategories = map[string]bool{ - "General": true, - "Documents": true, - "Electronics": true, - "Clothing": true, - "Fragile": true, - "Medical": true, - "Automotive": true, - "Food": true, + "General": true, + "Documents": true, + "Electronics": true, + "Clothing": true, + "Fragile": true, + "Medical": true, + "Automotive": true, + "Steel": true, + "Food": true, + "Meat": true, + "FreshProduce": true, } -// DeliveryCategoryList is the same set, ordered, for anything that renders a -// choice. General first because it is the safe default; Food last because it -// is the one that turns a capability off. +// DeliveryCategoryList is the same set, ordered, for anything that renders a choice. var DeliveryCategoryList = []string{ "General", "Documents", "Electronics", "Clothing", - "Fragile", "Medical", "Automotive", "Food", + "Fragile", "Medical", "Automotive", "Steel", "Food", "Meat", "FreshProduce", } // CategoryDefault is what a client with no category recorded is treated as. -// Every tenant onboarded before this field existed has an empty string, and -// they must keep behaving exactly as they did — which means reverse logistics -// stays available to them. const CategoryDefault = "General" -// ReverseLogisticsAllowed reports whether a return journey makes sense for -// what this client ships. -// -// Food is the exception: a meal that comes back is waste, not inventory. There -// is nothing to restock, nothing to refund against a returned item, and a -// rider carrying it to a hub is carrying rubbish. Offering RTO there is not a -// harmless extra button — it invites an operator to start a return journey -// that can only end in disposal, and it puts a return charge on a client's -// invoice for a parcel nobody can resell. -// -// Everything else can come back: clothing is the canonical case (wrong size), -// and electronics, documents and automotive parts all have a real return path. -// -// An UNKNOWN or empty category allows returns. That is deliberate: this field -// is new, every existing tenant has no value for it, and a default of "off" -// would silently withdraw a working capability from every client already using -// it. New information must not change old behaviour. -func ReverseLogisticsAllowed(category string) bool { - return category != "Food" +// NonReturnableCategories defines categories of goods that cannot be returned +// (Food items, Meat items, Fresh produce/perishables). Returning them causes +// spoilage and health risks. Non-perishables like Steel, Clothing, Electronics, etc. can be returned. +var NonReturnableCategories = map[string]bool{ + "food": true, + "meat": true, + "raw meat": true, + "fresh produce": true, + "freshproduce": true, + "fresh": true, + "perishable": true, + "fish": true, + "seafood": true, + "poultry": true, + "dairy": true, + "vegetables": true, + "fruits": true, +} + +// IsPerishableCategory checks if a category or item description is perishable (food, meat, fresh produce). +func IsPerishableCategory(category string) bool { + cat := strings.ToLower(strings.TrimSpace(category)) + if cat == "" { + return false + } + if NonReturnableCategories[cat] { + return true + } + if strings.Contains(cat, "food") || strings.Contains(cat, "meat") || strings.Contains(cat, "fresh") { + return true + } + return false +} + +// ReverseLogisticsAllowed reports whether a return journey makes sense for +// what this client or parcel ships. +// +// Food items, Meat items, and Fresh produce CANNOT be returned. +// Products like Steel, Electronics, Clothing, and Automotive CAN be returned. +func ReverseLogisticsAllowed(category string) bool { + return !IsPerishableCategory(category) } diff --git a/constants/delivery_category_test.go b/constants/delivery_category_test.go index 0db5fb8..7d17924 100644 --- a/constants/delivery_category_test.go +++ b/constants/delivery_category_test.go @@ -2,29 +2,26 @@ package constants import "testing" -// The rule the whole feature exists for: a returned meal is waste, not -// inventory, so Food clients get no reverse-logistics path. -func TestFoodIsTheOnlyCategoryWithoutReturns(t *testing.T) { - if ReverseLogisticsAllowed("Food") { - t.Error("Food allows returns; a returned meal can only be disposed of") - } - for _, c := range DeliveryCategoryList { - if c == "Food" { - continue +// Food, Meat, and Fresh produce items must NOT have return options. +// Durable goods like Steel, Electronics, Clothing, etc. DO have return options. +func TestPerishableCategoriesDoNotAllowReturns(t *testing.T) { + for _, nonRet := range []string{"Food", "Meat", "FreshProduce", "raw meat", "fish", "fresh vegetables"} { + if ReverseLogisticsAllowed(nonRet) { + t.Errorf("Perishable category %q should NOT allow returns", nonRet) } - if !ReverseLogisticsAllowed(c) { - t.Errorf("%s does not allow returns; only Food should be excluded", c) + } + + for _, ret := range []string{"Steel", "Clothing", "Electronics", "Automotive", "General", "Documents"} { + if !ReverseLogisticsAllowed(ret) { + t.Errorf("Durable category %q SHOULD allow returns", ret) } } } -// New information must not change old behaviour. Every tenant onboarded before -// this field existed has an empty category, and they were all using returns. -func TestUnknownAndEmptyCategoriesKeepReturns(t *testing.T) { - for _, c := range []string{"", " ", "Groceries", "SomethingNew"} { +func TestUnknownAndEmptyCategoriesDefaultToAllowed(t *testing.T) { + for _, c := range []string{"", " ", "SomethingNew"} { if !ReverseLogisticsAllowed(c) { - t.Errorf("category %q withdrew returns; an unrecognised category must not "+ - "silently remove a capability an existing client is already using", c) + t.Errorf("category %q withdrew returns; empty/unknown should default open", c) } } } diff --git a/controllers/consignmentReturn.go b/controllers/consignmentReturn.go index 92361ec..ae88400 100644 --- a/controllers/consignmentReturn.go +++ b/controllers/consignmentReturn.go @@ -46,6 +46,11 @@ var rtoReasons = map[string]string{ "customer_unavailable": "Customer unavailable", "attempts_exhausted": "Delivery attempts exhausted", "damaged": "Damaged in transit", + "defective": "Defective or damaged item", + "wrong_item": "Wrong item delivered", + "quality_issue": "Quality not as expected", + "size_mismatch": "Size or fit mismatch", + "not_needed": "No longer needed", "other": "Other", } @@ -127,12 +132,66 @@ func rtoHistoryRemark(from, reason string) string { // statusBeforeRTO reads that back from the RTO_Initiated history remark. func statusBeforeRTO(remark string) string { - if m := rtoFromPrefix.FindStringSubmatch(remark); m != nil && rtoStartable[m[1]] { + if m := rtoFromPrefix.FindStringSubmatch(remark); m != nil && (rtoStartable[m[1]] || m[1] == constants.ConsignmentDelivered) { return m[1] } return constants.ConsignmentOutForDelivery } +const DefaultReturnWindowDays = 7 + +// consignmentDeliveredAt finds the delivery timestamp of a consignment +func consignmentDeliveredAt(tx *gorm.DB, cn *models.Consignment) time.Time { + if cn == nil { + return time.Time{} + } + if cn.Deliveredat != nil && !cn.Deliveredat.IsZero() { + return *cn.Deliveredat + } + if tx != nil { + var proof models.DeliveryProof + if err := tx.Select("deliveredat").Where("consignmentid = ?", cn.Consignmentid).First(&proof).Error; err == nil && !proof.Deliveredat.IsZero() { + return proof.Deliveredat + } + var dest models.BookingDestination + if err := tx.Select("deliveredat").Where("consignmentid = ?", cn.Consignmentid).First(&dest).Error; err == nil && dest.Deliveredat != nil && !dest.Deliveredat.IsZero() { + return *dest.Deliveredat + } + var hist models.ConsignmentHistory + if err := tx.Select("createdat").Where("consignmentid = ? AND eventstatus = ?", cn.Consignmentid, constants.ConsignmentDelivered).Order("createdat DESC").First(&hist).Error; err == nil && !hist.Createdat.IsZero() { + return hist.Createdat + } + } + return cn.Updatedat +} + +// isReturnEligible checks whether a consignment can be returned (in-transit RTO or 7-day post-delivery window). +func isReturnEligible(tx *gorm.DB, cn *models.Consignment) (bool, error) { + if cn == nil { + return false, errRTO{"consignment not found"} + } + if cn.Status == constants.ConsignmentRTOInitiated { + return true, nil + } + if tx != nil { + if allowed, refusal := tenantAllowsReturns(tx, cn); !allowed { + return false, errRTO{refusal} + } + } + if rtoStartable[cn.Status] { + return true, nil + } + if cn.Status == constants.ConsignmentDelivered { + deliveredAt := consignmentDeliveredAt(tx, cn) + if !deliveredAt.IsZero() && time.Since(deliveredAt) > time.Duration(DefaultReturnWindowDays)*24*time.Hour { + return false, errRTO{fmt.Sprintf("return window expired: orders can only be returned within %d days of delivery", DefaultReturnWindowDays)} + } + return true, nil + } + return false, errRTO{fmt.Sprintf("a parcel that is %s cannot be returned", + strings.ReplaceAll(strings.ToLower(cn.Status), "_", " "))} +} + // errRTO carries an operator-readable refusal out of a transaction. type errRTO struct{ msg string } @@ -168,9 +227,8 @@ func startRTO(tx *gorm.DB, cn *models.Consignment, reasonText string, actorID *i if cn.Status == constants.ConsignmentRTOInitiated { return false, nil } - if !rtoStartable[cn.Status] { - return false, errRTO{fmt.Sprintf("a parcel that is %s cannot be returned", - strings.ReplaceAll(strings.ToLower(cn.Status), "_", " "))} + if eligible, err := isReturnEligible(tx, cn); !eligible { + return false, err } from := cn.Status now := time.Now() @@ -375,29 +433,48 @@ func rtoReasonText(reason, note string) (text, refusal string) { // return — the cost of wrongly allowing one is an operator reversing it; the // cost of wrongly blocking one is a parcel stranded with no path home. func tenantAllowsReturns(tx *gorm.DB, cn *models.Consignment) (bool, string) { - if cn == nil || cn.Tenantid == 0 { + if cn == nil { return true, "" } - var t models.Tenant - if err := tx.Select("tenantname", "deliverycategory", "reverselogisticsenabled"). - First(&t, cn.Tenantid).Error; err != nil { - utils.Warn("tenantAllowsReturns: could not read the tenant, allowing the return", - "tenantid", cn.Tenantid, "error", err) - return true, "" + + // 1. Check tenant delivery category: Food, Meat, and Fresh items have NO return path. + if cn.Tenantid != 0 { + var t models.Tenant + if err := tx.Select("tenantname", "deliverycategory", "reverselogisticsenabled"). + First(&t, cn.Tenantid).Error; err == nil { + if !t.ReturnsEnabled() || constants.IsPerishableCategory(t.Deliverycategory) { + who := t.Tenantname + if who == "" { + who = "This client" + } + what := t.Deliverycategory + if what == "" { + what = "perishable goods" + } + return false, who + " ships perishable products (" + what + + "). Food items, meat items, and fresh produce do not have return options." + } + } } - if t.ReturnsEnabled() { - return true, "" + + // 2. Check booking parcels: if parcel is food, meat, or fresh produce, returns are disabled. + if cn.Orderheaderid != nil { + var parcels []models.BookingParcel + if err := tx.Select("itemcategory", "itemdescription"). + Where("bookingid = ?", *cn.Orderheaderid).Find(&parcels).Error; err == nil && len(parcels) > 0 { + for _, p := range parcels { + if constants.IsPerishableCategory(p.Itemcategory) || constants.IsPerishableCategory(p.Itemdescription) { + desc := p.Itemcategory + if desc == "" { + desc = p.Itemdescription + } + return false, fmt.Sprintf("Parcels containing perishable goods (%s) cannot be returned. Return options are only available for non-perishable products like steel, electronics, and clothing.", desc) + } + } + } } - who := t.Tenantname - if who == "" { - who = "This client" - } - what := t.Deliverycategory - if what == "" { - what = "what they ship" - } - return false, who + " does not use reverse logistics (" + what + - "). Returns are switched off for this client — change it on their profile if that is wrong." + + return true, "" } func InitiateConsignmentRTO(c *fiber.Ctx) error { diff --git a/controllers/consignmentReturn_test.go b/controllers/consignmentReturn_test.go index c9faeec..e755242 100644 --- a/controllers/consignmentReturn_test.go +++ b/controllers/consignmentReturn_test.go @@ -36,19 +36,73 @@ func TestGenericStatusChangeGuard(t *testing.T) { func TestRTOHistoryRemarkRoundTrip(t *testing.T) { for _, from := range []string{constants.ConsignmentOutForDelivery, constants.ConsignmentCollectedByMiler, - constants.ConsignmentInwardedAtHub, constants.ConsignmentCreated} { + constants.ConsignmentInwardedAtHub, constants.ConsignmentCreated, constants.ConsignmentDelivered} { if got := statusBeforeRTO(rtoHistoryRemark(from, "Receiver refused: gate locked")); got != from { t.Errorf("round trip %s -> %s", from, got) } } // Anything unreadable or not a returnable status falls back to Out_for_Delivery. - for _, remark := range []string{"", "no prefix", "[from:Delivered] x", "[from:Cancelled] x"} { + for _, remark := range []string{"", "no prefix", "[from:Cancelled] x", "[from:Missing] x"} { if got := statusBeforeRTO(remark); got != constants.ConsignmentOutForDelivery { t.Errorf("%q -> %s, want Out_for_Delivery", remark, got) } } } +func TestIsReturnEligiblePostDeliveryWindow(t *testing.T) { + now := time.Now() + + // Delivered 3 days ago -> eligible + threeDaysAgo := now.Add(-3 * 24 * time.Hour) + cn3 := &models.Consignment{ + Status: constants.ConsignmentDelivered, + Deliveredat: &threeDaysAgo, + } + ok, err := isReturnEligible(nil, cn3) + if !ok || err != nil { + t.Fatalf("order delivered 3 days ago should be eligible, got ok=%v, err=%v", ok, err) + } + + // Delivered 6 days 23 hours ago -> eligible + almost7DaysAgo := now.Add(-167 * time.Hour) + cn6 := &models.Consignment{ + Status: constants.ConsignmentDelivered, + Deliveredat: &almost7DaysAgo, + } + ok, err = isReturnEligible(nil, cn6) + if !ok || err != nil { + t.Fatalf("order delivered within 7 days should be eligible, got ok=%v, err=%v", ok, err) + } + + // Delivered 8 days ago -> NOT eligible + eightDaysAgo := now.Add(-8 * 24 * time.Hour) + cn8 := &models.Consignment{ + Status: constants.ConsignmentDelivered, + Deliveredat: &eightDaysAgo, + } + ok, err = isReturnEligible(nil, cn8) + if ok || err == nil { + t.Fatalf("order delivered 8 days ago should NOT be eligible, got ok=%v, err=%v", ok, err) + } + if !strings.Contains(err.Error(), "return window expired") { + t.Fatalf("expected 'return window expired' error, got %q", err.Error()) + } + + // Cancelled order -> NOT eligible + cancelled := &models.Consignment{Status: "Cancelled"} + ok, _ = isReturnEligible(nil, cancelled) + if ok { + t.Fatal("cancelled consignment should not be returnable") + } + + // In-transit order -> eligible + ofd := &models.Consignment{Status: constants.ConsignmentOutForDelivery} + ok, err = isReturnEligible(nil, ofd) + if !ok || err != nil { + t.Fatalf("out_for_delivery should be eligible, got ok=%v, err=%v", ok, err) + } +} + func TestRTOAutoAfterAttempts(t *testing.T) { t.Setenv("RTO_AUTO_AFTER_ATTEMPTS", "") if rtoAutoAfterAttempts() != 3 { diff --git a/controllers/milerAppController.go b/controllers/milerAppController.go index 5747abb..be1d2a0 100644 --- a/controllers/milerAppController.go +++ b/controllers/milerAppController.go @@ -942,7 +942,9 @@ func MilerDeliverConsignment(c *fiber.Ctx) error { consignment.Status = constants.ConsignmentDelivered // Cleared once redeemed so the same code can't close out a second attempt. consignment.Deliveryotp = "" - consignment.Updatedat = time.Now() + nowDelivered := time.Now() + consignment.Deliveredat = &nowDelivered + consignment.Updatedat = nowDelivered if err := tx.Save(&consignment).Error; err != nil { tx.Rollback() return utils.Internal(c, "failed to mark consignment delivered") diff --git a/models/audit.go b/models/audit.go index cce47ec..af3a54a 100644 --- a/models/audit.go +++ b/models/audit.go @@ -70,6 +70,7 @@ type Consignment struct { Returnreason string `json:"returnreason" gorm:"column:returnreason"` Returninitiatedat *time.Time `json:"returninitiatedat" gorm:"column:returninitiatedat"` Returndeliveredat *time.Time `json:"returndeliveredat" gorm:"column:returndeliveredat"` + Deliveredat *time.Time `json:"deliveredat,omitempty" gorm:"column:deliveredat"` Parentconsignmentid *int `json:"parentconsignmentid" gorm:"column:parentconsignmentid"` // Inwardedat is when the parcel was physically received at a base — written // by the rider handover (POST /miler/consignments/:id/inward-at-hub) and by