diff --git a/controllers/appRequest.go b/controllers/appRequest.go new file mode 100644 index 0000000..47b62bd --- /dev/null +++ b/controllers/appRequest.go @@ -0,0 +1,26 @@ +package controllers + +import ( + "strings" + + "github.com/gofiber/fiber/v2" +) + +// isAppRequest reports whether a request arrived on the customer app's base. +// +// Several handlers are registered on both `/v1/web/...` and `/v1/mob/...`, and +// a few of them owe the two callers different answers. The clearest case is +// stock: the console lists a product with an empty shelf so a merchant can +// refill it, and the app must not list the same product at all, because a +// shopper can only choose it and be refused at checkout. +// +// The base is read from the path rather than passed down through the service, +// so the difference stays where it belongs — at the edge, in the one place that +// knows which audience asked. Services keep answering the same question the +// same way for everyone. +// +// Matched with the surrounding slashes so a tenant, product or keyword +// containing "mob" cannot make a console request look like an app one. +func isAppRequest(c *fiber.Ctx) bool { + return strings.Contains(c.Path(), "/v1/mob/") +} diff --git a/controllers/appRequest_test.go b/controllers/appRequest_test.go new file mode 100644 index 0000000..2bd0309 --- /dev/null +++ b/controllers/appRequest_test.go @@ -0,0 +1,50 @@ +package controllers + +import ( + "net/http/httptest" + "testing" + + "github.com/gofiber/fiber/v2" +) + +/* +The console and the customer app share handlers, and a few of those handlers owe +the two callers different answers. This is the switch that tells them apart, so +it is worth a test of its own: get it wrong in the permissive direction and +shoppers are offered empty shelves again; get it wrong in the strict direction +and a merchant's restocking screen goes blank, hiding the work they came to do. +*/ + +func pathSays(t *testing.T, path string, want bool) { + t.Helper() + app := fiber.New() + got := false + app.Get("/*", func(c *fiber.Ctx) error { + got = isAppRequest(c) + return nil + }) + if _, err := app.Test(httptest.NewRequest("GET", path, nil)); err != nil { + t.Fatalf("Test(%q): %v", path, err) + } + if got != want { + t.Errorf("isAppRequest(%q) = %v, want %v", path, got, want) + } +} + +func TestTheAppBaseIsRecognised(t *testing.T) { + pathSays(t, "/live/api/v1/mob/products/getallproducts", true) +} + +func TestTheConsoleBaseIsNotTreatedAsTheApp(t *testing.T) { + // The console must keep seeing empty shelves — restocking them is the whole + // point of that screen. + pathSays(t, "/live/api/v1/web/products/getallproducts", false) +} + +func TestAKeywordContainingMobDoesNotImpersonateTheApp(t *testing.T) { + // Matched with its slashes, so a search for "mobil" or a shop called + // "Mobius" cannot flip a console request into an app one and empty the + // merchant's screen. + pathSays(t, "/live/api/v1/web/products/getallproducts?keyword=mobile", false) + pathSays(t, "/live/api/v1/web/tenants/search?keyword=mob", false) +} diff --git a/controllers/productController.go b/controllers/productController.go index 0ab4728..50a4070 100644 --- a/controllers/productController.go +++ b/controllers/productController.go @@ -385,6 +385,23 @@ func (ctl *ProductController) GetAllProducts(c *fiber.Ctx) error { categoryID, subcategoryID, productID, applocationID, tenantID, locationID, keyword, productStatus, approve, pageno, pagesize, ) + + // The customer app is not shown what the shop cannot sell. + // + // Scoped to the /v1/mob base rather than applied in the service, because + // this one handler answers on BOTH bases and the two callers want opposite + // things. The console reads it to restock — an empty line is exactly what a + // merchant needs to see and act on, so filtering there would hide the work. + // A shopper reading the same list can only be misled by it: measured + // 2026-09-02 on R mart, four products were on offer in the app with a zero + // balance, including Cadbury Bournvita 500g and two Amul packs. Ordering one + // gets a 409 at checkout, after the shopper has chosen it. + // + // It filters on Productstock, the same number the response carries, so the + // list and the figure beside it cannot disagree. + if err == nil && isAppRequest(c) { + details = services.InStockOnlyGrouped(details) + } if err != nil { return c.JSON(fiber.Map{ "status": false, diff --git a/repositories/orderRepository.go b/repositories/orderRepository.go index 0a1c7b9..0395064 100644 --- a/repositories/orderRepository.go +++ b/repositories/orderRepository.go @@ -54,6 +54,34 @@ func NewOrderRepository(db *gorm.DB) OrderRepository { // It made a real revenue figure indistinguishable from an order that genuinely // booked nothing, which is why the historic orders on several tenants all look // worthless. +// ── Why every join below is a LEFT JOIN ────────────────────────────────────── +// +// These queries list orders. An order that exists is an order that must be +// listable, so a lookup that cannot be resolved may blank a column — it must +// never remove the row. +// +// They were INNER JOINs against customers, tenants, tenantlocations, +// app_location and app_locationconfig, which made each one a silent filter on a +// column the caller never asked to filter by. An order written with +// applocationid 0 or customerid 0 was created successfully, deducted stock, and +// then appeared in no screen that lists orders — not the shopper’s own history, +// not the shop’s. Measured 2026-09-02: two identical orders one field apart, +// both in the database, one of them visible. +// +// Some of these joins contributed no columns at all to the query they sat in, +// so they were pure filters by accident. +// +// The workaround is still in the tree and shows what this cost: the offline +// sales importer builds "header scaffolding" and invents a walk-in customer +// solely so its orders would not vanish (offlineLocationContext, +// resolveOfflineCustomer). Populating those fields stays good practice, but it +// is no longer what keeps an order visible. +// +// Deliberate filtering is unaffected. A query with a WHERE on a joined table — +// GetUserOrders filters e.status and e.userid — still excludes rows the join +// could not resolve, because WHERE runs after the join. GetDistinctLocations +// keeps its INNER JOIN on purpose: it asks which app locations have orders, so +// one that cannot be resolved is genuinely not an answer. const ( base = `SELECT DISTINCT a.orderheaderid, a.applocationid, h.locationname AS applocation, a.tenantid, a.locationid, a.partnerid, a.configid, a.categoryid, a.subcategoryid, a.moduleid, a.orderid, a.orderstatus, a.orderdate, a.ordernotes, a.itemcount, a.deliverytime AS deliverydate, @@ -67,12 +95,12 @@ const ( c.tenantname, c.tenanttoken, c.primarycontact AS tenantcontactno, c.postcode AS tenantpostcode, c.suburb AS tenantsuburb, c.city AS tenantcity, d.locationname, d.contactno AS locationcontactno, d.postcode AS locationpostcode, d.suburb AS locationsuburb, d.city AS locationcity FROM orders a - INNER JOIN customers b ON a.customerid = b.customerid - INNER JOIN tenants c ON a.tenantid = c.tenantid - INNER JOIN tenantlocations d ON a.locationid = d.locationid + LEFT JOIN customers b ON a.customerid = b.customerid + LEFT JOIN tenants c ON a.tenantid = c.tenantid + LEFT JOIN tenantlocations d ON a.locationid = d.locationid - INNER JOIN app_location h ON a.applocationid = h.applocationid - INNER JOIN app_locationconfig i ON a.applocationid = i.applocationid` + LEFT JOIN app_location h ON a.applocationid = h.applocationid + LEFT JOIN app_locationconfig i ON a.applocationid = i.applocationid` orderdetails = `SELECT DISTINCT a.orderheaderid, a.applocationid, a.tenantid, a.locationid, a.partnerid, a.configid, a.categoryid, a.subcategoryid, a.moduleid, @@ -87,10 +115,10 @@ const ( c.locationname, c.contactno AS locationcontactno, c.postcode AS locationpostcode, c.suburb AS locationsuburb, c.city AS locationcity, d.locationname AS applocation FROM orders a - INNER JOIN tenants b ON a.tenantid = b.tenantid - INNER JOIN tenantlocations c ON a.locationid = c.locationid - INNER JOIN app_location d ON a.applocationid = d.applocationid - INNER JOIN app_locationconfig e ON d.applocationid = e.applocationid` + LEFT JOIN tenants b ON a.tenantid = b.tenantid + LEFT JOIN tenantlocations c ON a.locationid = c.locationid + LEFT JOIN app_location d ON a.applocationid = d.applocationid + LEFT JOIN app_locationconfig e ON d.applocationid = e.applocationid` ) func (r *orderRepository) GetTenantOrders(input models.DeliveryQuery) ([]models.OrderInfo, error) { @@ -102,15 +130,33 @@ func (r *orderRepository) GetTenantOrders(input models.DeliveryQuery) ([]models. baseQuery := base + ` WHERE a.tenantid = ?` params = append(params, input.Tenantid) + // Status filters only when one was asked for. + // + // The else branch ran unconditionally, so a request naming no status became + // `orderstatus = ''` — which matches nothing. That is precisely the + // console’s default view: All branches, no status chosen. It listed no + // orders at all, however many the shop had taken. + // + // Compared case-insensitively because the ladder is stored capitalised + // ("Pending") while every literal here is lower case. Postgres agrees: + // `SELECT 'Pending' IN ('pending','processing','ready')` is false, so + // "ongoing" matched nothing either. + query = baseQuery if input.Status == "ongoing" { - query = baseQuery + ` AND a.orderstatus IN ('pending','processing','ready')` - } else { - query = baseQuery + ` AND a.orderstatus = ?` + query += ` AND LOWER(a.orderstatus) IN ('pending','processing','ready')` + } else if input.Status != "" { + query += ` AND LOWER(a.orderstatus) = LOWER(?)` params = append(params, input.Status) } - query += ` AND a.deliverytime::date BETWEEN ? AND ?` - params = append(params, input.Fromdate, input.ToDate) + // A date range narrows the list; it is not a precondition for having one. + // Applied unconditionally it cast the empty string to a date and answered + // 500, so the console’s order list failed outright until somebody picked a + // range. + if input.Fromdate != "" && input.ToDate != "" { + query += ` AND a.deliverytime::date BETWEEN ? AND ?` + params = append(params, input.Fromdate, input.ToDate) + } if input.Keyword != "" { query += ` AND ( @@ -160,7 +206,7 @@ func (r *orderRepository) GetPartnerOrders(stat, fdate, tdate string, pid, pagen params = append(params, fdate, tdate) } if stat != "" { - query += ` AND a.orderstatus = ?` + query += ` AND LOWER(a.orderstatus) = LOWER(?)` params = append(params, stat) } if keyword != "" { @@ -180,10 +226,10 @@ func (r *orderRepository) GetPartnerOrders(stat, fdate, tdate string, pid, pagen countQuery := ` SELECT COUNT(DISTINCT a.orderheaderid) FROM orders a - INNER JOIN tenants b ON a.tenantid = b.tenantid - INNER JOIN tenantlocations c ON a.locationid = c.locationid - INNER JOIN app_location d ON a.applocationid = d.applocationid - INNER JOIN app_locationconfig e ON d.applocationid = e.applocationid + LEFT JOIN tenants b ON a.tenantid = b.tenantid + LEFT JOIN tenantlocations c ON a.locationid = c.locationid + LEFT JOIN app_location d ON a.applocationid = d.applocationid + LEFT JOIN app_locationconfig e ON d.applocationid = e.applocationid LEFT JOIN deliveries f ON a.orderheaderid = f.orderheaderid LEFT JOIN app_users g ON f.userid = g.userid WHERE a.partnerid = ?` @@ -274,10 +320,10 @@ func (r *orderRepository) GetCustomerOrders(stat, fdate, tdate string, cid, mid, countQuery := ` SELECT COUNT(DISTINCT a.orderheaderid) FROM orders a - INNER JOIN tenants b ON a.tenantid = b.tenantid - INNER JOIN tenantlocations c ON a.locationid = c.locationid - INNER JOIN app_location d ON a.applocationid = d.applocationid - INNER JOIN app_locationconfig e ON d.applocationid = e.applocationid + LEFT JOIN tenants b ON a.tenantid = b.tenantid + LEFT JOIN tenantlocations c ON a.locationid = c.locationid + LEFT JOIN app_location d ON a.applocationid = d.applocationid + LEFT JOIN app_locationconfig e ON d.applocationid = e.applocationid WHERE a.customerid = ?` countParams := []interface{}{cid} @@ -376,10 +422,10 @@ func (r *orderRepository) GetAdminOrders(stat, fdate, tdate string, aid, pageno, countQuery := ` SELECT COUNT(DISTINCT a.orderheaderid) FROM orders a - INNER JOIN tenants b ON a.tenantid = b.tenantid - INNER JOIN tenantlocations c ON a.locationid = c.locationid - INNER JOIN app_location d ON a.applocationid = d.applocationid - INNER JOIN app_locationconfig e ON d.applocationid = e.applocationid + LEFT JOIN tenants b ON a.tenantid = b.tenantid + LEFT JOIN tenantlocations c ON a.locationid = c.locationid + LEFT JOIN app_location d ON a.applocationid = d.applocationid + LEFT JOIN app_locationconfig e ON d.applocationid = e.applocationid LEFT JOIN deliveries f ON a.orderheaderid = f.orderheaderid LEFT JOIN app_users g ON f.userid = g.userid WHERE 1=1 @@ -471,10 +517,10 @@ func (r *orderRepository) GetUserOrders(stat, fdate, tdate string, uid, pageno, countQuery := ` SELECT COUNT(DISTINCT a.orderheaderid) FROM orders a - INNER JOIN tenants b ON a.tenantid = b.tenantid - INNER JOIN tenantlocations c ON a.locationid = c.locationid - INNER JOIN app_location d ON a.applocationid = d.applocationid - INNER JOIN app_locationconfig e ON d.applocationid = e.applocationid + LEFT JOIN tenants b ON a.tenantid = b.tenantid + LEFT JOIN tenantlocations c ON a.locationid = c.locationid + LEFT JOIN app_location d ON a.applocationid = d.applocationid + LEFT JOIN app_locationconfig e ON d.applocationid = e.applocationid LEFT JOIN deliveries f ON a.orderheaderid = f.orderheaderid LEFT JOIN app_users g ON f.userid = g.userid WHERE e.status = 'Active' AND e.userid = ? @@ -564,10 +610,10 @@ func (r *orderRepository) GetAllOrders(stat, fdate, tdate string, pageno, pagesi countQuery := ` SELECT COUNT(DISTINCT a.orderheaderid) FROM orders a - INNER JOIN tenants b ON a.tenantid = b.tenantid - INNER JOIN tenantlocations c ON a.locationid = c.locationid - INNER JOIN app_location d ON a.applocationid = d.applocationid - INNER JOIN app_locationconfig e ON d.applocationid = e.applocationid + LEFT JOIN tenants b ON a.tenantid = b.tenantid + LEFT JOIN tenantlocations c ON a.locationid = c.locationid + LEFT JOIN app_location d ON a.applocationid = d.applocationid + LEFT JOIN app_locationconfig e ON d.applocationid = e.applocationid LEFT JOIN deliveries f ON a.orderheaderid = f.orderheaderid LEFT JOIN app_users g ON f.userid = g.userid WHERE 1=1 @@ -2314,16 +2360,16 @@ func (r *orderRepository) GetCustomerOrdersv3(customerID, tenantID, moduleID, fr a.pickupcity, a.deliveryid AS deliverycustomerid, a.deliveryaddress, a.deliverylat, a.deliverylong, a.deliverytype, a.deliverycustomer, a.deliverycontactno, a.deliverylocation AS deliverysuburb, a.deliverycity, a.paymenttype, a.smsdelivery, - a.taxamount, + a.orderamount, a.taxamount, b.tenantname, b.tenanttoken, b.primarycontact AS tenantcontactno, b.postcode AS tenantpostcode, b.suburb AS tenantsuburb, b.city AS tenantcity, b.registrationno, c.locationname, c.contactno AS locationcontactno, c.postcode AS locationpostcode, c.suburb AS locationsuburb, c.city AS locationcity, d.locationname AS applocation FROM orders a - INNER JOIN tenants b ON a.tenantid = b.tenantid - INNER JOIN tenantlocations c ON a.locationid = c.locationid - INNER JOIN app_location d ON a.applocationid = d.applocationid + LEFT JOIN tenants b ON a.tenantid = b.tenantid + LEFT JOIN tenantlocations c ON a.locationid = c.locationid + LEFT JOIN app_location d ON a.applocationid = d.applocationid WHERE 1=1 ` @@ -2402,10 +2448,15 @@ func (r *orderRepository) attachOrderDetails(orders []models.CustomerOrder) ([]m for i := range orders { orders[i].OrderDetails = detailMap[orders[i].Orderheaderid] - if len(orders[i].OrderDetails) > 0 { - orders[i].Orderamount = orders[i].OrderDetails[0].Orderamount - orders[i].Totaltaxamount = orders[i].OrderDetails[0].Totaltaxamount - } + // The header already carries the order total, and it is the only place + // that has one. This used to overwrite it with the first LINE’s + // Orderamount and Totaltaxamount — columns that do not exist on the + // orderdetails table, so both scanned as zero and clobbered a correct + // value. Every order in a shopper’s history read ₹0 as a result, however + // much they had actually spent. + // + // Taking the first line’s figure could not have been right even if the + // columns existed: one line is not the order. } return orders, nil @@ -2427,7 +2478,7 @@ func (r *orderRepository) GetTenantLocationOrders(input models.DeliveryQuery) ([ // Status filter if input.Status == "ongoing" { - query += ` AND a.orderstatus IN ('pending','processing','ready')` + query += ` AND LOWER(a.orderstatus) IN ('pending','processing','ready')` } else if input.Status != "" { query += ` AND a.orderstatus = ?` params = append(params, input.Status) @@ -2446,8 +2497,12 @@ func (r *orderRepository) GetTenantLocationOrders(input models.DeliveryQuery) ([ } // Date filter - query += ` AND DATE(a.deliverytime) BETWEEN ? AND ?` - params = append(params, input.Fromdate, input.ToDate) + // Narrows the list rather than gating it — same reasoning as + // GetTenantOrders above, where empty dates cast to a date and answered 500. + if input.Fromdate != "" && input.ToDate != "" { + query += ` AND DATE(a.deliverytime) BETWEEN ? AND ?` + params = append(params, input.Fromdate, input.ToDate) + } // Keyword filter if input.Keyword != "" { diff --git a/services/productVisibility.go b/services/productVisibility.go index 36a6323..e278fde 100644 --- a/services/productVisibility.go +++ b/services/productVisibility.go @@ -51,3 +51,37 @@ func InStockOnly(products []models.Products, locationID int) []models.Products { } return inStock } + +// InStockOnlyGrouped is InStockOnly for the grouped shape `getallproducts` +// returns: a tenant with its products, rather than a flat list. +// +// ── Why this one does NOT skip when locationID is 0 ────────────────────────── +// +// InStockOnly above returns early without an outlet, because the query behind +// it computes stock scoped to that outlet and answers 0 for everything when +// there is none — filtering that would empty the catalogue rather than filter +// it. FetchFilteredProducts is built differently: its stock subquery reads +// `(? = 0 OR locationid = ?)`, so with no outlet it aggregates the ledger across +// the tenant's outlets and still returns a real number. Measured 2026-09-02 on +// R mart: 24 products unscoped, 20 with a live balance and 4 at zero. +// +// So the number is trustworthy either way, and the rule can be applied either +// way: a shop that holds none of something anywhere is not offering it. +// +// A tenant left with no sellable products keeps its group, with an empty list. +// Dropping the group would tell the app the shop does not exist, which is a +// different and wrong statement — the shop is open, it has nothing on the shelf. +func InStockOnlyGrouped(groups []models.Tenantproducts) []models.Tenantproducts { + out := make([]models.Tenantproducts, 0, len(groups)) + for _, group := range groups { + kept := make([]models.Products, 0, len(group.Products)) + for _, product := range group.Products { + if product.Productstock > 0 { + kept = append(kept, product) + } + } + group.Products = kept + out = append(out, group) + } + return out +} diff --git a/services/productVisibility_test.go b/services/productVisibility_test.go index 9e2c73c..ea1974c 100644 --- a/services/productVisibility_test.go +++ b/services/productVisibility_test.go @@ -606,3 +606,64 @@ func TestImportPublishesAfterTheRowsExist(t *testing.T) { t.Fatalf("want CreateProductLocation before PublishPricedLocations, got %v", repo.calls) } } + +/* +The grouped shape, which is what `getallproducts` answers with. + +Measured 2026-09-02 on R mart: the app was offering four products with a zero +balance — Cadbury Bournvita 500g, Amul 90g, Amul 150g and a Dabur chewing gum. +A shopper could pick any of them and only find out at checkout, where the stock +check answers 409. +*/ + +func groupOf(stocks ...int) []models.Tenantproducts { + products := make([]models.Products, 0, len(stocks)) + for i, s := range stocks { + products = append(products, models.Products{Productid: 100 + i, Productstock: s}) + } + return []models.Tenantproducts{{Products: products}} +} + +func TestAnEmptyShelfIsNotOfferedToShoppers(t *testing.T) { + got := InStockOnlyGrouped(groupOf(5, 0, 3, 0)) + if len(got) != 1 { + t.Fatalf("expected the tenant group to survive, got %d groups", len(got)) + } + if len(got[0].Products) != 2 { + t.Fatalf("expected 2 sellable products, got %d", len(got[0].Products)) + } + for _, p := range got[0].Products { + if p.Productstock <= 0 { + t.Errorf("product %d has no stock and was still offered", p.Productid) + } + } +} + +func TestAShopWithNothingOnTheShelfStillExists(t *testing.T) { + // The group is kept with an empty list. Dropping it would tell the app the + // shop is not there, which is a different and wrong statement: it is open, + // it just has nothing to sell today. + got := InStockOnlyGrouped(groupOf(0, 0)) + if len(got) != 1 { + t.Fatalf("the shop disappeared: got %d groups", len(got)) + } + if len(got[0].Products) != 0 { + t.Errorf("expected nothing sellable, got %d", len(got[0].Products)) + } +} + +func TestANegativeBalanceIsNotStockWhenGrouped(t *testing.T) { + // A ledger can go negative through correction. Negative is not "some". + got := InStockOnlyGrouped(groupOf(-4)) + if len(got[0].Products) != 0 { + t.Errorf("a negative balance was treated as stock") + } +} + +func TestFilteringGroupsDoesNotDisturbTheCallersSlice(t *testing.T) { + original := groupOf(1, 0) + _ = InStockOnlyGrouped(original) + if len(original[0].Products) != 2 { + t.Errorf("the caller's slice was modified: %d products left", len(original[0].Products)) + } +}