feat(product): require a unit for weight-based products
A product sold by weight with no unit produces order lines with nothing to print: the receipt would read "4,2" with no idea of what. Until now nothing stopped that — the mistake only surfaced at the cashier. Enforce it in two places, because neither alone sees the whole picture. On create, the validator has everything it needs. On update, the request may omit unit_id for a product that already has one, so the check runs in the processor against the merged product: what is rejected is the end state, a product sold by weight with no unit. Also fixes two things this uncovered: The struct tags on the product contracts are decorative — this validator is hand-written and never calls validator.Struct — so `oneof=unit weight` was never enforced, and an unknown sell_by was silently rewritten to "unit" by the mapper. It is now rejected with a message that names the valid values. The update validator's "at least one field" guard did not list unit_id, sell_by or print_to_checker, so an update carrying only one of those was turned away as an empty request. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -71,10 +71,14 @@ pendek yang benar-benar ingin ditampilkan. Daftar satuan dibaca lewat `GET /api/
|
||||
|
||||
| Field | Tipe | Keterangan |
|
||||
|---|---|---|
|
||||
| `sell_by` | `"unit"` \| `"weight"` | Opsional, default `"unit"`. Nilai lain ditolak validator. |
|
||||
| `unit_id` | UUID | Opsional di backend, tapi **wajib secara praktik** untuk `sell_by: "weight"` — lihat §8. |
|
||||
| `sell_by` | `"unit"` \| `"weight"` | Opsional, default `"unit"`. Nilai lain **ditolak** dengan pesan jelas. |
|
||||
| `unit_id` | UUID | **Wajib** saat `sell_by: "weight"`, ditolak backend bila kosong. Opsional untuk produk satuan. |
|
||||
| `price` | number | Harga per satu satuan. Rp 4.500 per ons, bukan harga per ikan. |
|
||||
|
||||
Pada `PUT /api/v1/products/:id`, `unit_id` **tidak perlu dikirim ulang** bila produknya
|
||||
sudah punya satuan — mengubah `sell_by` menjadi `"weight"` saja sudah cukup. Yang ditolak
|
||||
adalah kondisi akhirnya: produk yang dijual per timbangan tanpa satuan.
|
||||
|
||||
Keduanya juga bisa diubah lewat `PUT /api/v1/products/:id` dengan bentuk yang sama, dan
|
||||
ikut terbaca di setiap response produk (`GET /api/v1/products`, `/products/all`,
|
||||
`/products/:id`).
|
||||
@@ -84,8 +88,11 @@ ikut terbaca di setiap response produk (`GET /api/v1/products`, `/products/all`,
|
||||
- Kunci `sell_by` **setelah produk punya transaksi**. Mengubah produk lama dari `unit`
|
||||
ke `weight` tidak mengubah baris order yang sudah ada — baris lama tetap dihitung per
|
||||
cacah — tapi akan membingungkan pengguna yang melihat riwayatnya.
|
||||
- Saat `weight` dipilih, jadikan pemilih satuan sebagai field **wajib** di form.
|
||||
- Saat `weight` dipilih, jadikan pemilih satuan sebagai field **wajib** di form. Backend
|
||||
juga menolaknya, tapi ditangkap di form lebih baik daripada baru gagal saat simpan.
|
||||
- Ubah label harga mengikuti satuan yang dipilih: *"Harga per ons"*.
|
||||
- Untuk produk satuan, pemilih satuan boleh disembunyikan — `unit_id` opsional dan belum
|
||||
dikonsumsi apa pun di POS.
|
||||
|
||||
---
|
||||
|
||||
@@ -213,6 +220,14 @@ Semua error mengikuti amplop standar. Pesan validasi baru muncul dengan kode `90
|
||||
|
||||
Pesan menyebut **nama produk**, sehingga bisa ditampilkan apa adanya ke kasir.
|
||||
|
||||
### Setup produk (Backoffice)
|
||||
|
||||
| Pesan (`cause`) | Penyebab | Perbaikan di klien |
|
||||
|---|---|---|
|
||||
| `unit_id is required when sell_by is 'weight'` | Produk timbangan dibuat tanpa satuan | Wajibkan pemilih satuan saat Timbangan dipilih |
|
||||
| `sell_by must be either 'unit' or 'weight'` | Nilai `sell_by` di luar dua itu | Kirim persis `"unit"` atau `"weight"` |
|
||||
| `product '…' is sold by weight and requires a unit_id` | Update membuat produk jadi timbangan tanpa satuan | Kirim `unit_id` bersama perubahan `sell_by` |
|
||||
|
||||
---
|
||||
|
||||
## 7. Kompatibilitas mundur
|
||||
@@ -231,11 +246,6 @@ permintaannya ditolak dengan pesan jelas, bukan gagal diam-diam.
|
||||
|
||||
## 8. Batasan yang diketahui
|
||||
|
||||
> **Perlu ditangani di frontend.** Backend **belum** memaksa `unit_id` terisi saat
|
||||
> `sell_by: "weight"`. Produk timbangan tanpa satuan akan tersimpan, tapi baris ordernya
|
||||
> keluar dengan `unit_abbreviation: null` — struk tidak bisa mencetak "ons". Sampai
|
||||
> validasi itu ditambahkan di backend, **Backoffice wajib mewajibkannya di form**.
|
||||
|
||||
- **Pembulatan uang ke 2 desimal.** `4,237 ons × Rp 4.500` tersimpan `Rp 19.066,50`,
|
||||
bukan dibulatkan ke rupiah utuh. Bila kasir harus menerima rupiah penuh, ini perlu
|
||||
diputuskan dan diubah di backend lebih dulu (`RoundMoney`, satu tempat).
|
||||
|
||||
@@ -25,7 +25,9 @@ func ProductEntityToModel(entity *entities.Product) *models.Product {
|
||||
BusinessType: constants.BusinessType(entity.BusinessType),
|
||||
ImageURL: entity.ImageURL,
|
||||
PrinterType: entity.PrinterType,
|
||||
UnitID: entity.UnitID,
|
||||
SellBy: entity.SellBy,
|
||||
HasIngredients: entity.HasIngredients,
|
||||
Metadata: map[string]interface{}(entity.Metadata),
|
||||
IsActive: entity.IsActive,
|
||||
CreatedAt: entity.CreatedAt,
|
||||
@@ -50,6 +52,9 @@ func ProductModelToEntity(model *models.Product) *entities.Product {
|
||||
BusinessType: string(model.BusinessType),
|
||||
ImageURL: model.ImageURL,
|
||||
PrinterType: model.PrinterType,
|
||||
UnitID: model.UnitID,
|
||||
SellBy: model.SellBy,
|
||||
HasIngredients: model.HasIngredients,
|
||||
Metadata: entities.Metadata(model.Metadata),
|
||||
IsActive: model.IsActive,
|
||||
CreatedAt: model.CreatedAt,
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"context"
|
||||
"fmt"
|
||||
|
||||
"apskel-pos-be/internal/constants"
|
||||
"apskel-pos-be/internal/entities"
|
||||
"apskel-pos-be/internal/logger"
|
||||
"apskel-pos-be/internal/mappers"
|
||||
@@ -191,6 +192,13 @@ func (p *ProductProcessorImpl) UpdateProduct(ctx context.Context, id uuid.UUID,
|
||||
|
||||
mappers.UpdateProductEntityFromRequest(existingProduct, req)
|
||||
|
||||
// Checked after the merge, not on the request: switching a product to sell_by
|
||||
// "weight" is valid when it already carries a unit, and clearing the unit is
|
||||
// invalid when it is already sold by weight. Only the merged product shows either.
|
||||
if existingProduct.SellBy == constants.SellByWeight && existingProduct.UnitID == nil {
|
||||
return nil, fmt.Errorf("product '%s' is sold by weight and requires a unit_id", existingProduct.Name)
|
||||
}
|
||||
|
||||
if err := p.productRepo.Update(ctx, existingProduct); err != nil {
|
||||
return nil, fmt.Errorf("failed to update product: %w", err)
|
||||
}
|
||||
|
||||
@@ -63,6 +63,32 @@ func (v *ProductValidatorImpl) ValidateCreateProductRequest(req *contract.Create
|
||||
return errors.New("printer_type cannot exceed 50 characters"), constants.MalformedFieldErrorCode
|
||||
}
|
||||
|
||||
if err, code := validateSellBy(req.SellBy, req.UnitID); err != nil {
|
||||
return err, code
|
||||
}
|
||||
|
||||
return nil, ""
|
||||
}
|
||||
|
||||
// validateSellBy checks how a product is sold and that it carries what that choice
|
||||
// needs. A weight-based product without a unit would produce order lines with no unit
|
||||
// to print, so the receipt could show "4,2" with no idea of what.
|
||||
//
|
||||
// unitID is the unit the request would leave on the product: for an update that does
|
||||
// not touch unit_id, pass the product's current one.
|
||||
func validateSellBy(sellBy *string, unitID *uuid.UUID) (error, string) {
|
||||
if sellBy == nil {
|
||||
return nil, ""
|
||||
}
|
||||
|
||||
if !constants.IsValidSellBy(*sellBy) {
|
||||
return errors.New("sell_by must be either 'unit' or 'weight'"), constants.MalformedFieldErrorCode
|
||||
}
|
||||
|
||||
if *sellBy == constants.SellByWeight && unitID == nil {
|
||||
return errors.New("unit_id is required when sell_by is 'weight'"), constants.MissingFieldErrorCode
|
||||
}
|
||||
|
||||
return nil, ""
|
||||
}
|
||||
|
||||
@@ -74,7 +100,8 @@ func (v *ProductValidatorImpl) ValidateUpdateProductRequest(req *contract.Update
|
||||
// At least one field should be provided for update
|
||||
if req.CategoryID == nil && req.SKU == nil && req.Name == nil && req.Description == nil &&
|
||||
req.Price == nil && req.Cost == nil && req.BusinessType == nil && req.ImageURL == nil &&
|
||||
req.PrinterType == nil && req.Metadata == nil && req.IsActive == nil {
|
||||
req.PrinterType == nil && req.PrintToChecker == nil && req.UnitID == nil &&
|
||||
req.SellBy == nil && req.Metadata == nil && req.IsActive == nil {
|
||||
return errors.New("at least one field must be provided for update"), constants.MissingFieldErrorCode
|
||||
}
|
||||
|
||||
@@ -111,6 +138,13 @@ func (v *ProductValidatorImpl) ValidateUpdateProductRequest(req *contract.Update
|
||||
return errors.New("printer_type cannot exceed 50 characters"), constants.MalformedFieldErrorCode
|
||||
}
|
||||
|
||||
// Only the value is checked here. Whether the product ends up with a unit depends on
|
||||
// what it already has, which this request cannot see — the processor checks that
|
||||
// against the stored product.
|
||||
if req.SellBy != nil && !constants.IsValidSellBy(*req.SellBy) {
|
||||
return errors.New("sell_by must be either 'unit' or 'weight'"), constants.MalformedFieldErrorCode
|
||||
}
|
||||
|
||||
return nil, ""
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,130 @@
|
||||
package validator
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"apskel-pos-be/internal/constants"
|
||||
"apskel-pos-be/internal/contract"
|
||||
|
||||
"github.com/google/uuid"
|
||||
)
|
||||
|
||||
func strPtr(s string) *string { return &s }
|
||||
|
||||
func baseCreateRequest() *contract.CreateProductRequest {
|
||||
return &contract.CreateProductRequest{
|
||||
CategoryID: uuid.New(),
|
||||
Name: "Ikan Tude",
|
||||
Price: 4500,
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateCreateProductRequestSellBy(t *testing.T) {
|
||||
unitID := uuid.New()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
sellBy *string
|
||||
unitID *uuid.UUID
|
||||
wantErr bool
|
||||
wantMsg string
|
||||
}{
|
||||
{
|
||||
name: "omitted sell_by is allowed and defaults to unit",
|
||||
sellBy: nil, unitID: nil,
|
||||
},
|
||||
{
|
||||
name: "unit product needs no unit_id",
|
||||
sellBy: strPtr(constants.SellByUnit), unitID: nil,
|
||||
},
|
||||
{
|
||||
name: "weight product with a unit is accepted",
|
||||
sellBy: strPtr(constants.SellByWeight), unitID: &unitID,
|
||||
},
|
||||
{
|
||||
name: "weight product without a unit is rejected",
|
||||
// Otherwise its order lines would have no unit to print on the receipt.
|
||||
sellBy: strPtr(constants.SellByWeight), unitID: nil,
|
||||
wantErr: true,
|
||||
wantMsg: "unit_id is required when sell_by is 'weight'",
|
||||
},
|
||||
{
|
||||
name: "unknown sell_by is rejected rather than silently corrected",
|
||||
// The struct tags on this contract are not enforced — this validator is
|
||||
// hand-written — so the check has to be explicit.
|
||||
sellBy: strPtr("pisang"), unitID: &unitID,
|
||||
wantErr: true,
|
||||
wantMsg: "sell_by must be either 'unit' or 'weight'",
|
||||
},
|
||||
}
|
||||
|
||||
v := NewProductValidator()
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
req := baseCreateRequest()
|
||||
req.SellBy = tt.sellBy
|
||||
req.UnitID = tt.unitID
|
||||
|
||||
err, code := v.ValidateCreateProductRequest(req)
|
||||
|
||||
if !tt.wantErr {
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
return
|
||||
}
|
||||
if err == nil {
|
||||
t.Fatal("expected an error, got none")
|
||||
}
|
||||
if err.Error() != tt.wantMsg {
|
||||
t.Errorf("message = %q, want %q", err.Error(), tt.wantMsg)
|
||||
}
|
||||
if code == "" {
|
||||
t.Error("expected an error code")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// An update carrying only sell_by must not be turned away as an empty update.
|
||||
func TestValidateUpdateProductRequestAcceptsSellByAlone(t *testing.T) {
|
||||
v := NewProductValidator()
|
||||
|
||||
err, _ := v.ValidateUpdateProductRequest(&contract.UpdateProductRequest{
|
||||
SellBy: strPtr(constants.SellByWeight),
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
|
||||
err, _ = v.ValidateUpdateProductRequest(&contract.UpdateProductRequest{})
|
||||
if err == nil {
|
||||
t.Error("an update with no fields at all should be rejected")
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateUpdateProductRequestRejectsUnknownSellBy(t *testing.T) {
|
||||
v := NewProductValidator()
|
||||
|
||||
err, _ := v.ValidateUpdateProductRequest(&contract.UpdateProductRequest{
|
||||
SellBy: strPtr("timbangan"),
|
||||
})
|
||||
if err == nil {
|
||||
t.Fatal("expected an error for an unknown sell_by")
|
||||
}
|
||||
}
|
||||
|
||||
// The update path deliberately does NOT require unit_id on the request: a product that
|
||||
// already has a unit can be switched to sell_by "weight" without resending it. That
|
||||
// pairing is checked by the processor against the stored product.
|
||||
func TestValidateUpdateProductRequestDefersUnitCheck(t *testing.T) {
|
||||
v := NewProductValidator()
|
||||
|
||||
err, _ := v.ValidateUpdateProductRequest(&contract.UpdateProductRequest{
|
||||
SellBy: strPtr(constants.SellByWeight),
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("update should not require unit_id on the request, got: %v", err)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user