Merge remote-tracking branch 'origin/main'

repositories/customerRepository.go

@@ -188,10 +188,21 @@ func (r *customerRepository) GetTenantCustomers(tid, lid, pageno, pagesize int,
 	var args []interface{}
 	searchLike := "%" + keyword + "%"
 
+	// DISTINCT ON collapses to one row per customer BEFORE the LIMIT is applied.
+	// Without it the store-scoped branch below paginated the joined
+	// customerlocations rows — one per saved address — so `pagesize` bought a
+	// page of addresses, not of customers. Live example: locationid 1185 returned
+	// 12 rows that were only 2 people, 11 of them one customer's addresses. A
+	// store with a page size of 20 therefore listed roughly three customers and
+	// gave no hint that the rest existed.
+	//
+	// The ORDER BY must lead with the DISTINCT ON expression, so customerid sorts
+	// first; the trailing keys only decide WHICH address represents a customer,
+	// preferring the one flagged primary.
 	if lid != 0 {
-		q1 = `SELECT a.customerid,a.firstname,a.lastname,a.contactno,a.email,
+		q1 = `SELECT DISTINCT ON (a.customerid) a.customerid,a.firstname,a.lastname,a.contactno,a.email,
 			b.locationid as deliverylocationid,b.address,b.suburb,b.city,b.state,b.landmark,b.doorno,b.postcode,
-			b.latitude,b.longitude,a.applocationid,c.locationid as tenantlocationid,a.status 
+			b.latitude,b.longitude,a.applocationid,c.locationid as tenantlocationid,a.status
 			FROM customers a
 			LEFT JOIN customerlocations b ON a.customerid=b.customerid
 			INNER JOIN tenantcustomers c ON a.customerid=c.customerid
@@ -204,13 +215,17 @@ func (r *customerRepository) GetTenantCustomers(tid, lid, pageno, pagesize int,
 			args = append(args, searchLike, searchLike, searchLike)
 		}
 
-		q1 += ` ORDER BY a.customerid DESC LIMIT ? OFFSET ?`
+		q1 += ` ORDER BY a.customerid DESC, b.primaryaddress DESC NULLS LAST, b.locationid ASC
+			LIMIT ? OFFSET ?`
 		args = append(args, pagesize, offset)
 
 	} else {
-		q1 = `SELECT a.customerid,a.firstname,a.lastname,a.contactno,a.email,
+		// A customer linked to several outlets of the same tenant has one
+		// tenantcustomers row per outlet, so this branch double-counted them
+		// against the LIMIT too.
+		q1 = `SELECT DISTINCT ON (a.customerid) a.customerid,a.firstname,a.lastname,a.contactno,a.email,
 			a.address,a.suburb,a.city,a.state,a.landmark,a.doorno,a.postcode,
-			a.latitude,a.longitude,a.applocationid,c.locationid as tenantlocationid,a.status 
+			a.latitude,a.longitude,a.applocationid,c.locationid as tenantlocationid,a.status
 			FROM customers a
 			INNER JOIN tenantcustomers c ON a.customerid=c.customerid
 			WHERE c.tenantid = ?`
@@ -223,12 +238,10 @@ func (r *customerRepository) GetTenantCustomers(tid, lid, pageno, pagesize int,
 			args = append(args, searchLike, searchLike, searchLike)
 		}
 
-		q1 += ` ORDER BY a.customerid DESC LIMIT ? OFFSET ?`
+		q1 += ` ORDER BY a.customerid DESC, c.locationid ASC LIMIT ? OFFSET ?`
 		args = append(args, pagesize, offset)
 	}
 
-	print(q1)
-
 	r.db.Raw(q1, args...).Find(&data)
 	return data
 }

repositories/deliveriesRepository.go

@@ -1,11 +1,13 @@
 package repositories
 
 import (
+	"errors"
 	"fmt"
 	"log"
 	"nearle/models"
 	"strconv"
 	"strings"
+	"time"
 
 	"github.com/jinzhu/copier"
 	"gorm.io/gorm"
@@ -147,18 +149,75 @@ func (r *deliveriesRepository) UpdateDelivery(data models.UpdateDeliveryStatus)
 	var ord models.Updateorderstatus
 	var cloc models.Customerlocations
 
+	if data.Deliveryid == 0 {
+		return errors.New("deliveryid is required")
+	}
+
 	tx := r.db.Begin()
+	if tx.Error != nil {
+		return tx.Error
+	}
 
 	if err := tx.Table("deliveries").Where("deliveryid = ?", data.Deliveryid).Updates(&data).Error; err != nil {
 		tx.Rollback()
 		return err
 	}
 
+	// The parent order is resolved from the delivery row rather than taken from
+	// the request. Every status branch below writes the order with
+	// "WHERE orderheaderid = ?", and a client that omits orderheaderid made that
+	// "WHERE orderheaderid = 0", matching nothing. GORM reports no error for an
+	// update that affects no rows, so the handler still answered 201 Success
+	// while the order silently kept its old status — 635 deliveries are marked
+	// delivered against an order still reading pending because of this.
+	//
+	// deliveryid is the one field every caller must send (it is how the row
+	// above is found), so deriving the link from it makes the sync independent
+	// of how complete the client's payload is.
+	orderHeaderID := data.Orderheaderid
+	if orderHeaderID == 0 {
+		if err := tx.Table("deliveries").
+			Select("orderheaderid").
+			Where("deliveryid = ?", data.Deliveryid).
+			Scan(&orderHeaderID).Error; err != nil {
+			tx.Rollback()
+			return err
+		}
+	}
+	if orderHeaderID == 0 {
+		tx.Rollback()
+		return fmt.Errorf("delivery %d has no order attached", data.Deliveryid)
+	}
+
+	// syncOrder applies the status to the parent order and fails loudly if the
+	// row is not there, instead of reporting success for a write that landed
+	// nowhere.
+	syncOrder := func() error {
+		res := tx.Table("orders").Where("orderheaderid = ?", orderHeaderID).Updates(&ord)
+		if res.Error != nil {
+			return res.Error
+		}
+		if res.RowsAffected == 0 {
+			return fmt.Errorf("order %d not found for delivery %d", orderHeaderID, data.Deliveryid)
+		}
+		return nil
+	}
+
+	// The lifecycle timestamp mirrored onto the order. Clients frequently send
+	// the status without one, and because Updates() skips zero-valued struct
+	// fields the order's own column was left blank while its status moved on.
+	stamp := func(supplied string) string {
+		if strings.TrimSpace(supplied) != "" {
+			return supplied
+		}
+		return time.Now().Format("2006-01-02 15:04:05")
+	}
+
 	switch data.Orderstatus {
 	case "pending":
 		ord.Orderstatus = data.Orderstatus
-		ord.Pending = data.Assigntime
-		if err := tx.Table("orders").Where("orderheaderid = ?", data.Orderheaderid).Updates(&ord).Error; err != nil {
+		ord.Pending = stamp(data.Assigntime)
+		if err := syncOrder(); err != nil {
 			tx.Rollback()
 			return err
 		}
@@ -181,8 +240,8 @@ func (r *deliveriesRepository) UpdateDelivery(data models.UpdateDeliveryStatus)
 
 	case "delivered":
 		ord.Orderstatus = data.Orderstatus
-		ord.Delivered = data.Deliverytime
-		if err := tx.Table("orders").Where("orderheaderid = ?", data.Orderheaderid).Updates(&ord).Error; err != nil {
+		ord.Delivered = stamp(data.Deliverytime)
+		if err := syncOrder(); err != nil {
 			tx.Rollback()
 			return err
 		}
@@ -204,8 +263,8 @@ func (r *deliveriesRepository) UpdateDelivery(data models.UpdateDeliveryStatus)
 
 	case "cancelled":
 		ord.Orderstatus = data.Orderstatus
-		ord.Cancelled = data.Canceltime
-		if err := tx.Table("orders").Where("orderheaderid = ?", data.Orderheaderid).Updates(&ord).Error; err != nil {
+		ord.Cancelled = stamp(data.Canceltime)
+		if err := syncOrder(); err != nil {
 			tx.Rollback()
 			return err
 		}

repositories/orderRepository.go

@@ -1785,29 +1785,52 @@ func (r *orderRepository) resolveOfflineCustomer(tx *gorm.DB, ctx *offlineLocati
 // instead of a race: two uploads of the same file arriving together would
 // otherwise both read "not yet imported" and both commit.
 func (r *orderRepository) UploadOfflineSales(input models.OfflineSalesUpload) (*models.OfflineSalesUploadResponse, error) {
+	if input.Tenantid <= 0 {
+		return nil, errors.New("tenantid is required")
+	}
 	if len(input.Bills) == 0 {
 		return nil, errors.New("no sales rows found in the upload")
 	}
 
-	ctx, err := r.resolveOfflineLocationContext(input.Tenantid, input.Locationid)
-	if err != nil {
-		return nil, err
+	// When the caller pins the upload to one branch, that branch is resolved
+	// (and authorised) up front so an outlet the merchant does not own fails
+	// the whole request rather than each bill in turn.
+	if input.Locationid > 0 {
+		if _, err := r.resolveOfflineLocationContext(input.Tenantid, input.Locationid); err != nil {
+			return nil, err
+		}
 	}
 
-	products, err := r.loadOfflineProducts(input.Tenantid, input.Locationid)
-	if err != nil {
-		return nil, err
-	}
-	if len(products) == 0 {
-		return nil, fmt.Errorf("outlet '%s' has no products stocked against it", ctx.Locationname)
+	// Branch context and catalogue are resolved once per branch and reused. A
+	// workbook covering six outlets would otherwise re-run both queries for
+	// every bill in it.
+	contexts := make(map[int]*offlineLocationContext)
+	catalogues := make(map[int]map[int]offlineProduct)
+
+	resolve := func(locationID int) (*offlineLocationContext, map[int]offlineProduct, error) {
+		if ctx, ok := contexts[locationID]; ok {
+			return ctx, catalogues[locationID], nil
+		}
+		ctx, err := r.resolveOfflineLocationContext(input.Tenantid, locationID)
+		if err != nil {
+			return nil, nil, err
+		}
+		products, err := r.loadOfflineProducts(input.Tenantid, locationID)
+		if err != nil {
+			return nil, nil, err
+		}
+		if len(products) == 0 {
+			return nil, nil, fmt.Errorf("outlet '%s' has no products stocked against it", ctx.Locationname)
+		}
+		contexts[locationID] = ctx
+		catalogues[locationID] = products
+		return ctx, products, nil
 	}
 
 	resp := &models.OfflineSalesUploadResponse{Results: make([]models.OfflineSaleResult, 0, len(input.Bills))}
 
-	for _, bill := range input.Bills {
-		result := r.importOfflineBill(ctx, products, input.Userid, bill)
+	record := func(result models.OfflineSaleResult) {
 		resp.Results = append(resp.Results, result)
-
 		switch result.Status {
 		case models.OfflineSaleImported:
 			resp.Imported++
@@ -1819,6 +1842,49 @@ func (r *orderRepository) UploadOfflineSales(input models.OfflineSalesUpload) (*
 		}
 	}
 
+	for _, bill := range input.Bills {
+		billLocation := bill.Locationid
+		if billLocation <= 0 {
+			billLocation = input.Locationid
+		}
+
+		if billLocation <= 0 {
+			record(models.OfflineSaleResult{
+				Billno:  strings.TrimSpace(bill.Billno),
+				Status:  models.OfflineSaleFailed,
+				Message: "no locationid on these rows — the sheet must say which branch the sale belongs to",
+			})
+			continue
+		}
+
+		// A pinned upload refuses bills for anywhere else. This is what keeps a
+		// store user inside their own branch: editing the locationid column in
+		// the spreadsheet changes nothing, because the pin is set from their
+		// session and not from the file.
+		if input.Locationid > 0 && billLocation != input.Locationid {
+			record(models.OfflineSaleResult{
+				Locationid: billLocation,
+				Billno:     strings.TrimSpace(bill.Billno),
+				Status:     models.OfflineSaleFailed,
+				Message:    fmt.Sprintf("this upload is limited to outlet %d, but these rows are for outlet %d", input.Locationid, billLocation),
+			})
+			continue
+		}
+
+		ctx, products, err := resolve(billLocation)
+		if err != nil {
+			record(models.OfflineSaleResult{
+				Locationid: billLocation,
+				Billno:     strings.TrimSpace(bill.Billno),
+				Status:     models.OfflineSaleFailed,
+				Message:    err.Error(),
+			})
+			continue
+		}
+
+		record(r.importOfflineBill(ctx, products, input.Userid, bill))
+	}
+
 	return resp, nil
 }
 
@@ -1836,9 +1902,11 @@ func (r *orderRepository) importOfflineBill(
 
 	fail := func(format string, args ...any) models.OfflineSaleResult {
 		return models.OfflineSaleResult{
-			Billno:  billLabel,
-			Status:  models.OfflineSaleFailed,
-			Message: fmt.Sprintf(format, args...),
+			Locationid:   ctx.Locationid,
+			Locationname: ctx.Locationname,
+			Billno:       billLabel,
+			Status:       models.OfflineSaleFailed,
+			Message:      fmt.Sprintf(format, args...),
 		}
 	}
 
@@ -1968,9 +2036,11 @@ func (r *orderRepository) importOfflineBill(
 	if already > 0 {
 		tx.Rollback()
 		return models.OfflineSaleResult{
-			Billno:  billLabel,
-			Status:  models.OfflineSaleDuplicate,
-			Message: fmt.Sprintf("bill %s was already imported for this outlet; stock was not deducted again", billLabel),
+			Locationid:   ctx.Locationid,
+			Locationname: ctx.Locationname,
+			Billno:       billLabel,
+			Status:       models.OfflineSaleDuplicate,
+			Message:      fmt.Sprintf("bill %s was already imported for %s; stock was not deducted again", billLabel, ctx.Locationname),
 		}
 	}
 
@@ -2021,13 +2091,15 @@ func (r *orderRepository) importOfflineBill(
 	}
 
 	return models.OfflineSaleResult{
+		Locationid:    ctx.Locationid,
+		Locationname:  ctx.Locationname,
 		Billno:        billLabel,
 		Status:        models.OfflineSaleImported,
 		Orderid:       created.Orderid,
 		Orderheaderid: created.Orderheaderid,
 		Itemcount:     len(items),
 		Amount:        orderAmount,
-		Message:       fmt.Sprintf("imported as order %s", created.Orderid),
+		Message:       fmt.Sprintf("imported as order %s at %s", created.Orderid, ctx.Locationname),
 	}
 }
 

repositories/productRepository.go

@@ -503,44 +503,63 @@ func (r *productRepository) GetLocationProducts(tenantID, locationID, subcategor
 	return data, nil
 }
 
-// GetSaleTemplate lists every product stocked at one outlet, with its live
-// ledger balance, so the web app can generate a pre-filled offline-sales
-// spreadsheet. It deliberately returns the whole catalogue for the outlet
-// unpaged — a spreadsheet the user is meant to fill in and hand back is only
-// useful if it contains every product they could have sold.
+// GetSaleTemplate lists products stocked at a tenant's branches, with each
+// one's live ledger balance, so the web app can generate a pre-filled
+// offline-sales spreadsheet.
+//
+// locationID = 0 means "every branch this tenant runs", which is the normal
+// case: a merchant with several outlets gets ONE workbook covering all of them,
+// with tenantid and locationid stamped on every row. The row's own locationid
+// is what later decides which branch a sale is deducted from, so the operator
+// never has to pick a store or juggle a file per outlet. Passing a specific
+// locationID narrows it to that branch, which is what a store user gets.
+//
+// It deliberately returns the whole catalogue unpaged — a spreadsheet meant to
+// be filled in and handed back is only useful if it contains every product that
+// could have been sold.
 //
 // The balance is the same SUM(in) - SUM(out) expression CreateOrder validates
-// against, so the "currentstock" the user reads in the sheet is exactly the
-// number the import will later check their quantity against. LOWER() covers
-// the mixed-case stocktype values in production ('out', 'IN', 'in').
+// against, so the "currentstock" read in the sheet is exactly the number the
+// import will check the typed quantity against. LOWER() covers the mixed-case
+// stocktype values in production ('out', 'IN', 'in').
 //
-// A location that does not belong to the tenant yields no template rather than
-// another tenant's catalogue: the caller treats that as "not your outlet".
+// The INNER JOIN on tenantlocations is load-bearing: it confines the result to
+// branches the tenant actually owns, so a template can never disclose another
+// merchant's catalogue even if a stray productlocations row pointed at one.
 func (r *productRepository) GetSaleTemplate(tenantID, locationID int) (*models.SaleTemplate, error) {
-	if tenantID <= 0 || locationID <= 0 {
-		return nil, errors.New("tenantid and locationid are required")
+	if tenantID <= 0 {
+		return nil, errors.New("tenantid is required")
+	}
+	if locationID < 0 {
+		locationID = 0
 	}
 
-	var loc struct {
-		Locationname string
-	}
-	err := r.db.Raw(
-		`SELECT locationname FROM tenantlocations WHERE tenantid = ? AND locationid = ?`,
-		tenantID, locationID,
-	).Scan(&loc).Error
-	if err != nil {
-		return nil, err
-	}
-	if strings.TrimSpace(loc.Locationname) == "" {
-		return nil, fmt.Errorf("location %d does not belong to tenant %d", locationID, tenantID)
+	// Only checked when the caller narrowed to one branch. Without it a
+	// mistyped locationid would silently yield an empty template rather than
+	// saying the outlet is not theirs.
+	if locationID > 0 {
+		var locationName string
+		err := r.db.Raw(
+			`SELECT COALESCE(locationname, '') FROM tenantlocations WHERE tenantid = ? AND locationid = ?`,
+			tenantID, locationID,
+		).Scan(&locationName).Error
+		if err != nil {
+			return nil, err
+		}
+		if strings.TrimSpace(locationName) == "" {
+			return nil, fmt.Errorf("location %d does not belong to tenant %d", locationID, tenantID)
+		}
 	}
 
 	rows := make([]models.SaleTemplateRow, 0)
 	query := `
-		SELECT a.productid,
+		SELECT a.tenantid,
+		       b.locationid,
+		       COALESCE(tl.locationname, '') AS locationname,
+		       a.productid,
 		       a.productname,
-		       COALESCE(a.productunit, '') AS productunit,
-		       COALESCE(a.unitvalue, '')   AS unitvalue,
+		       COALESCE(a.productunit, '')  AS productunit,
+		       COALESCE(a.unitvalue, '')    AS unitvalue,
 		       COALESCE(d.categoryname, '') AS categoryname,
 		       COALESCE(SUM(CASE WHEN LOWER(c.stocktype) = 'in'  THEN c.quantity ELSE 0 END) -
 		                SUM(CASE WHEN LOWER(c.stocktype) = 'out' THEN c.quantity ELSE 0 END), 0) AS currentstock,
@@ -548,23 +567,41 @@ func (r *productRepository) GetSaleTemplate(tenantID, locationID int) (*models.S
 		       COALESCE(a.taxpercent, 0) AS taxpercent
 		FROM products a
 		INNER JOIN productlocations b ON a.productid = b.productid AND a.tenantid = b.tenantid
+		INNER JOIN tenantlocations tl ON tl.locationid = b.locationid AND tl.tenantid = a.tenantid
 		LEFT JOIN productstocks c
 		       ON a.productid = c.productid AND b.locationid = c.locationid AND a.tenantid = c.tenantid
 		LEFT JOIN productcategories d ON a.categoryid = d.categoryid
-		WHERE a.approve = 1 AND a.tenantid = ? AND b.locationid = ?
-		GROUP BY a.productid, a.productname, a.productunit, a.unitvalue, d.categoryname,
-		         b.price, a.retailprice, a.taxpercent
-		ORDER BY a.productname ASC`
+		WHERE a.approve = 1 AND a.tenantid = ? AND (? = 0 OR b.locationid = ?)
+		GROUP BY a.tenantid, b.locationid, tl.locationname, a.productid, a.productname,
+		         a.productunit, a.unitvalue, d.categoryname, b.price, a.retailprice, a.taxpercent
+		ORDER BY tl.locationname ASC, a.productname ASC`
 
-	if err := r.db.Raw(query, tenantID, locationID).Scan(&rows).Error; err != nil {
+	if err := r.db.Raw(query, tenantID, locationID, locationID).Scan(&rows).Error; err != nil {
 		return nil, err
 	}
 
+	// Summarised from the rows themselves rather than queried separately, so
+	// the branch list can never disagree with what the sheet actually contains.
+	locations := make([]models.SaleTemplateLocation, 0)
+	seen := make(map[int]int)
+	for _, row := range rows {
+		if idx, ok := seen[row.Locationid]; ok {
+			locations[idx].Productcount++
+			continue
+		}
+		seen[row.Locationid] = len(locations)
+		locations = append(locations, models.SaleTemplateLocation{
+			Locationid:   row.Locationid,
+			Locationname: strings.TrimSpace(row.Locationname),
+			Productcount: 1,
+		})
+	}
+
 	return &models.SaleTemplate{
-		Tenantid:     tenantID,
-		Locationid:   locationID,
-		Locationname: strings.TrimSpace(loc.Locationname),
-		Products:     rows,
+		Tenantid:   tenantID,
+		Locationid: locationID,
+		Locations:  locations,
+		Products:   rows,
 	}, nil
 }