diff --git a/controllers/stockRequestBatch.go b/controllers/stockRequestBatch.go new file mode 100644 index 0000000..f074e2b --- /dev/null +++ b/controllers/stockRequestBatch.go @@ -0,0 +1,68 @@ +package controllers + +import ( + "encoding/json" + "fmt" + + "nearle/models" + + "github.com/gofiber/fiber/v2" +) + +// parseStockRequests reads either one request or a list of them. +// +// Sniffing the first byte rather than trying one shape and falling back to the +// other: BodyParser consumes what it reads, so a failed first attempt can leave +// nothing for the second. The body is JSON here and `[` is unambiguous. +func parseStockRequests(c *fiber.Ctx) ([]models.StockRequest, error) { + body := c.Body() + + for _, b := range body { + switch b { + case ' ', '\t', '\r', '\n': + continue + case '[': + var many []models.StockRequest + if err := json.Unmarshal(body, &many); err != nil { + return nil, err + } + return many, nil + } + break + } + + var one models.StockRequest + if err := c.BodyParser(&one); err != nil { + return nil, err + } + return []models.StockRequest{one}, nil +} + +// dedupeIDs drops repeats and anything non-positive, preserving order. +// +// Repeats matter here rather than being tidiness: approving adds stock, so the +// same id twice in one batch would try to receive the same delivery twice. The +// service has its own guard, but a batch should not be relying on it. +func dedupeIDs(ids []int) []int { + seen := make(map[int]bool, len(ids)) + out := make([]int, 0, len(ids)) + for _, id := range ids { + if id <= 0 || seen[id] { + continue + } + seen[id] = true + out = append(out, id) + } + return out +} + +// stockBatchMessage says what happened in words a merchant can act on. +// +// "8 approved, 2 could not be" beats "Success" when two of ten did not land — +// the whole point of reporting per row is that the person can go and look. +func stockBatchMessage(ok, failed int, verb string) string { + if failed == 0 { + return fmt.Sprintf("%d %s", ok, verb) + } + return fmt.Sprintf("%d %s, %d could not be", ok, verb, failed) +} diff --git a/controllers/stockRequestBatch_test.go b/controllers/stockRequestBatch_test.go new file mode 100644 index 0000000..67baf1f --- /dev/null +++ b/controllers/stockRequestBatch_test.go @@ -0,0 +1,102 @@ +package controllers + +import ( + "net/http/httptest" + "strings" + "testing" + + "github.com/gofiber/fiber/v2" +) + +/* +Batch decisions on stock requests. + +Approving is not a status write — the service adds the requested quantity to the +branch's stock — so a batch has to behave like a list of separate actions that +each really happened, not like one all-or-nothing write. These tests pin the +parts that would quietly move stock twice or lose a row. +*/ + +func parseVia(t *testing.T, body string) ([]int, error) { + t.Helper() + app := fiber.New() + var got []int + var perr error + app.Post("/", func(c *fiber.Ctx) error { + reqs, err := parseStockRequests(c) + perr = err + for _, r := range reqs { + got = append(got, r.Productid) + } + return nil + }) + req := httptest.NewRequest("POST", "/", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + if _, err := app.Test(req); err != nil { + t.Fatalf("Test: %v", err) + } + return got, perr +} + +func TestOneRequestStillWorksUnwrapped(t *testing.T) { + // The existing caller sends an object, and must keep working untouched. + got, err := parseVia(t, `{"productid":42,"qty":5}`) + if err != nil { + t.Fatalf("single: %v", err) + } + if len(got) != 1 || got[0] != 42 { + t.Errorf("got %v, want [42]", got) + } +} + +func TestAListOfRequestsIsRead(t *testing.T) { + got, err := parseVia(t, `[{"productid":1,"qty":2},{"productid":2,"qty":3}]`) + if err != nil { + t.Fatalf("list: %v", err) + } + if len(got) != 2 { + t.Errorf("got %v, want two requests", got) + } +} + +func TestLeadingWhitespaceDoesNotHideAList(t *testing.T) { + // A client that pretty-prints its body still sends a list. + got, err := parseVia(t, " \n\t[{\"productid\":7,\"qty\":1}]") + if err != nil { + t.Fatalf("padded list: %v", err) + } + if len(got) != 1 || got[0] != 7 { + t.Errorf("got %v, want [7]", got) + } +} + +func TestTheSameRequestCannotBeApprovedTwiceInOneBatch(t *testing.T) { + // Approving adds stock. The same id twice would receive one delivery twice. + got := dedupeIDs([]int{5, 5, 6, 5}) + if len(got) != 2 || got[0] != 5 || got[1] != 6 { + t.Errorf("got %v, want [5 6]", got) + } +} + +func TestNonPositiveIdsAreDropped(t *testing.T) { + // 0 is what an unparsed or missing field becomes; it is not a request. + if got := dedupeIDs([]int{0, -3, 9}); len(got) != 1 || got[0] != 9 { + t.Errorf("got %v, want [9]", got) + } +} + +func TestOrderIsPreservedSoTheReportMatchesTheScreen(t *testing.T) { + got := dedupeIDs([]int{3, 1, 2}) + if got[0] != 3 || got[1] != 1 || got[2] != 2 { + t.Errorf("got %v, want the order sent", got) + } +} + +func TestThePartialOutcomeIsStatedNotHidden(t *testing.T) { + if msg := stockBatchMessage(8, 2, "approved"); msg != "8 approved, 2 could not be" { + t.Errorf("got %q", msg) + } + if msg := stockBatchMessage(5, 0, "approved"); msg != "5 approved" { + t.Errorf("got %q", msg) + } +} diff --git a/controllers/stockrequestController.go b/controllers/stockrequestController.go index 2db4278..43ad6f6 100644 --- a/controllers/stockrequestController.go +++ b/controllers/stockrequestController.go @@ -18,22 +18,59 @@ func NewStockRequestController(stockRequestService services.StockRequestService) return &StockRequestController{stockRequestService: stockRequestService} } +// CreateStockRequest accepts one request or a list of them. +// +// A shop restocking after a delivery is asking for twenty things at once, and +// sending twenty HTTP requests to say so is slow, gives no single answer, and +// leaves a half-sent batch behind when the connection drops. The single-object +// form is unchanged, so every existing caller keeps working. +// +// Each row is reported individually rather than the batch failing whole: a +// request for a product that no longer exists should not discard the other +// nineteen, and the requester needs to know WHICH one it was. func (ctl *StockRequestController) CreateStockRequest(c *fiber.Ctx) error { - var input models.StockRequest - if err := c.BodyParser(&input); err != nil { + inputs, err := parseStockRequests(c) + if err != nil { return c.JSON(fiber.Map{"code": http.StatusBadRequest, "message": "Invalid input", "status": false}) } - - if input.Status == "" { - input.Status = "Pending" + if len(inputs) == 0 { + return c.JSON(fiber.Map{"code": http.StatusBadRequest, "message": "send at least one request", "status": false}) } - err := ctl.stockRequestService.CreateStockRequest(&input) - if err != nil { - return c.JSON(fiber.Map{"code": http.StatusInternalServerError, "message": err.Error(), "status": false}) + created := make([]models.StockRequest, 0, len(inputs)) + failed := make([]fiber.Map, 0) + + for i := range inputs { + if inputs[i].Status == "" { + inputs[i].Status = "Pending" + } + if err := ctl.stockRequestService.CreateStockRequest(&inputs[i]); err != nil { + failed = append(failed, fiber.Map{ + "productid": inputs[i].Productid, + "reason": err.Error(), + }) + continue + } + created = append(created, inputs[i]) } - return c.JSON(fiber.Map{"code": 200, "message": "Stock request created", "status": true, "details": input}) + // A single-object caller gets the object back, exactly as before. + if len(inputs) == 1 && len(failed) == 0 { + return c.JSON(fiber.Map{"code": 200, "message": "Stock request created", "status": true, "details": created[0]}) + } + + if len(created) == 0 { + return c.JSON(fiber.Map{ + "code": http.StatusInternalServerError, "status": false, + "message": "no requests could be created", "details": fiber.Map{"failed": failed}, + }) + } + + return c.JSON(fiber.Map{ + "code": 200, "status": true, + "message": stockBatchMessage(len(created), len(failed), "created"), + "details": fiber.Map{"created": created, "failed": failed}, + }) } func (ctl *StockRequestController) GetStockRequests(c *fiber.Ctx) error { @@ -52,19 +89,70 @@ func (ctl *StockRequestController) GetStockRequests(c *fiber.Ctx) error { return c.JSON(fiber.Map{"code": 200, "message": "Success", "status": true, "details": data}) } +// UpdateStockRequest decides one request or a batch of them. +// +// `requestid` for one, `requestids` for many, one status for the batch. That +// shape rather than a list of {id,status} pairs because the action a merchant +// takes is "approve these" or "reject these" — a mixed batch is two actions, +// and letting one call do both makes an accidental mass-approve possible. +// +// Approving is not a status write: the service adds the requested quantity to +// the branch’s stock. So each id is applied on its own and reported on its own. +// If the fourth of ten fails, the first three have really been received and the +// merchant has to know that, rather than being told the batch failed and +// approving it a second time. func (ctl *StockRequestController) UpdateStockRequest(c *fiber.Ctx) error { var input struct { - RequestID int `json:"requestid"` - Status string `json:"status"` + RequestID int `json:"requestid"` + RequestIDs []int `json:"requestids"` + Status string `json:"status"` } if err := c.BodyParser(&input); err != nil { return c.JSON(fiber.Map{"code": http.StatusBadRequest, "message": "Invalid input", "status": false}) } - err := ctl.stockRequestService.UpdateStockRequest(input.RequestID, input.Status) - if err != nil { - return c.JSON(fiber.Map{"code": http.StatusInternalServerError, "message": err.Error(), "status": false}) + if input.Status == "" { + return c.JSON(fiber.Map{"code": http.StatusBadRequest, "status": false, + "message": "status is required"}) } - return c.JSON(fiber.Map{"code": 200, "message": "Stock request updated", "status": true}) + ids := input.RequestIDs + if input.RequestID != 0 { + ids = append([]int{input.RequestID}, ids...) + } + ids = dedupeIDs(ids) + if len(ids) == 0 { + return c.JSON(fiber.Map{"code": http.StatusBadRequest, "status": false, + "message": "send requestid, or requestids for several"}) + } + + updated := make([]int, 0, len(ids)) + failed := make([]fiber.Map, 0) + for _, id := range ids { + if err := ctl.stockRequestService.UpdateStockRequest(id, input.Status); err != nil { + failed = append(failed, fiber.Map{"requestid": id, "reason": err.Error()}) + continue + } + updated = append(updated, id) + } + + // The single-id caller keeps the answer it has always had. + if len(ids) == 1 { + if len(failed) > 0 { + return c.JSON(fiber.Map{"code": http.StatusInternalServerError, "status": false, + "message": failed[0]["reason"]}) + } + return c.JSON(fiber.Map{"code": 200, "message": "Stock request updated", "status": true}) + } + + if len(updated) == 0 { + return c.JSON(fiber.Map{"code": http.StatusInternalServerError, "status": false, + "message": "no requests could be updated", "details": fiber.Map{"failed": failed}}) + } + + return c.JSON(fiber.Map{ + "code": 200, "status": true, + "message": stockBatchMessage(len(updated), len(failed), "updated"), + "details": fiber.Map{"updated": updated, "failed": failed}, + }) }