From 090e9c0c2fa64d0a45cc0f877cb81502fd891911 Mon Sep 17 00:00:00 2001 From: abhishek Date: Fri, 25 Sep 2026 16:18:53 +0530 Subject: [PATCH] shifts --- controllers/posController.go | 37 ++++++++++++++++++-- repositories/posShiftRepository.go | 43 +++++++++++++++++++----- repositories/posShift_test.go | 54 ++++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+), 10 deletions(-) create mode 100644 repositories/posShift_test.go diff --git a/controllers/posController.go b/controllers/posController.go index c87a178..1d7e04c 100644 --- a/controllers/posController.go +++ b/controllers/posController.go @@ -771,6 +771,20 @@ func posClaimError(c *fiber.Ctx, err error) error { // once the console can hold a session. // posWebScope reads and checks the tenant and outlet a console request names. +// posTenantScope is the guard for things that belong to a whole business +// rather than to one of its shops — shift windows, so far. +// +// No ownership query, because there is nothing to own: `middleware.WebAuth` +// pins the tenant from the signed session and refuses a request naming another +// one, so reaching here with a tenant id at all means it is this caller's. +// Naming an outlet is what needs checking, and that is `posWebScope` below. +func (ctl *PosController) posTenantScope(tenantID int) error { + if tenantID <= 0 { + return fmt.Errorf("tenantid is required") + } + return nil +} + func (ctl *PosController) posWebScope(tenantID, locationID int) error { if tenantID <= 0 { return fmt.Errorf("tenantid is required") @@ -921,9 +935,18 @@ func (ctl *PosController) WebListStaffShifts(c *fiber.Ctx) error { tenantID, _ := strconv.Atoi(strings.TrimSpace(c.Query("tenantid"))) locationID, _ := strconv.Atoi(strings.TrimSpace(c.Query("locationid"))) - if err := ctl.posWebScope(tenantID, locationID); err != nil { + // Tenant-scoped, because a shift belongs to the business rather than to one + // of its shops. An outlet may still be named to narrow the list, and is + // checked for ownership when it is — omitting it is not a way to read + // somebody else's, because the tenant comes from the signed session. + if err := ctl.posTenantScope(tenantID); err != nil { return posBadRequest(c, err) } + if locationID > 0 { + if err := ctl.posWebScope(tenantID, locationID); err != nil { + return posBadRequest(c, err) + } + } shifts, err := ctl.posService.ListStaffShifts(tenantID, locationID, strings.EqualFold(c.Query("include_inactive"), "true")) @@ -944,9 +967,19 @@ func (ctl *PosController) WebCreateStaffShift(c *fiber.Ctx) error { return posBadRequest(c, fmt.Errorf("invalid request body")) } - if err := ctl.posWebScope(req.Tenantid, req.Locationid); err != nil { + // A shift with no outlet belongs to the tenant and every branch it owns, + // which is the ordinary case — a business that works 07:00–15:00 works + // those hours at every shop, and entering them per outlet is how the third + // branch quietly ends up on 07:00–15:30. An outlet is named only when one + // shop really does differ, and is checked for ownership then. + if err := ctl.posTenantScope(req.Tenantid); err != nil { return posBadRequest(c, err) } + if req.Locationid > 0 { + if err := ctl.posWebScope(req.Tenantid, req.Locationid); err != nil { + return posBadRequest(c, err) + } + } shift, err := ctl.posService.CreateStaffShift(req.Tenantid, req.Locationid, req) if err != nil { diff --git a/repositories/posShiftRepository.go b/repositories/posShiftRepository.go index 5199374..473df5f 100644 --- a/repositories/posShiftRepository.go +++ b/repositories/posShiftRepository.go @@ -36,21 +36,42 @@ func normaliseShiftTime(raw string) (string, error) { return t, nil } -// ListStaffShifts returns an outlet's shifts, newest last so a picker reads in -// the order they were created rather than alphabetically by name. +// ListStaffShifts returns the shifts a tenant's staff can be put on, newest +// last so a picker reads in the order they were created rather than +// alphabetically by name. +// +// ── Why the outlet is optional ────────────────────────────────────────────── +// +// A shift is a fact about how a BUSINESS runs, not about one shop: a tenant +// that works 07:00–15:00 and 15:00–23:00 works those hours at every branch it +// owns, and making somebody re-enter them per outlet guarantees the third +// branch gets 07:00–15:30 and nobody notices. `locationid = 0` is a shift that +// belongs to the whole tenant. +// +// Branch-specific rows are still honoured, because they already exist and a +// tenant may genuinely run one outlet differently. Asking for an outlet returns +// the tenant's shifts AND that outlet's own; asking for none returns everything +// the tenant has. func (r *posRepository) ListStaffShifts(tenantID, locationID int, includeInactive bool) ([]models.StaffShifts, error) { - if tenantID <= 0 || locationID <= 0 { - return nil, fmt.Errorf("tenantid and locationid are required") + if tenantID <= 0 { + return nil, fmt.Errorf("tenantid is required") } shifts := make([]models.StaffShifts, 0) - query := `SELECT * FROM staffshifts WHERE tenantid = ? AND locationid = ?` + args := []any{tenantID} + + query := `SELECT * FROM staffshifts WHERE tenantid = ?` + if locationID > 0 { + // The tenant-wide ones and this outlet's, never another outlet's. + query += ` AND (COALESCE(locationid, 0) = 0 OR locationid = ?)` + args = append(args, locationID) + } if !includeInactive { query += ` AND LOWER(COALESCE(status,'active')) <> 'inactive'` } query += ` ORDER BY staffshiftid` - if err := r.db.Raw(query, tenantID, locationID).Scan(&shifts).Error; err != nil { + if err := r.db.Raw(query, args...).Scan(&shifts).Error; err != nil { return nil, err } return shifts, nil @@ -58,8 +79,14 @@ func (r *posRepository) ListStaffShifts(tenantID, locationID int, includeInactiv // CreateStaffShift adds a window at one outlet. func (r *posRepository) CreateStaffShift(tenantID, locationID int, req models.StaffShifts) (*models.StaffShifts, error) { - if tenantID <= 0 || locationID <= 0 { - return nil, fmt.Errorf("tenantid and locationid are required") + if tenantID <= 0 { + return nil, fmt.Errorf("tenantid is required") + } + // `locationid = 0` is deliberate and is now the ordinary case: the shift + // belongs to the tenant and every branch it owns can use it. An outlet is + // only named when one shop really does run different hours. + if locationID < 0 { + locationID = 0 } name := strings.TrimSpace(req.Name) diff --git a/repositories/posShift_test.go b/repositories/posShift_test.go new file mode 100644 index 0000000..2d222f3 --- /dev/null +++ b/repositories/posShift_test.go @@ -0,0 +1,54 @@ +package repositories + +import ( + "strings" + "testing" +) + +/* +A shift belongs to a business, not to one of its shops. + +Both halves of this used to demand an outlet, so the same two windows had to be +re-entered at every branch a tenant owns — which is how the third branch quietly +gets 07:00–15:30 and nobody notices until a cashier is filed under hours that do +not exist. A tenant that works 07:00–15:00 works those hours everywhere. + +`locationid = 0` is now the ordinary case. A named outlet still works, because +one shop may genuinely run differently and those rows already exist. +*/ + +func TestATimeIsAcceptedInBothFormsTheClientsSend(t *testing.T) { + // `` gives "07:00"; some browsers and every hand-typed + // value give "07:00:00"; `ridershifts` stores the seconds form already. + for _, tc := range []struct{ in, want string }{ + {"07:00", "07:00"}, + {"07:00:00", "07:00"}, + {" 23:59 ", "23:59"}, + {"00:00", "00:00"}, + } { + got, err := normaliseShiftTime(tc.in) + if err != nil { + t.Fatalf("%q: %v", tc.in, err) + } + if got != tc.want { + t.Fatalf("%q became %q, want %q", tc.in, got, tc.want) + } + } +} + +func TestSomethingThatIsNotATimeOfDayIsRefused(t *testing.T) { + for _, bad := range []string{"", " ", "7:00", "24:00", "07:60", "morning", "07", "7pm"} { + if _, err := normaliseShiftTime(bad); err == nil { + t.Fatalf("%q was accepted as a time of day", bad) + } + } +} + +func TestTheTimeErrorSaysWhatShapeIsWanted(t *testing.T) { + // "invalid" tells somebody nothing. The message names the format, because + // the most common wrong answer is a valid time in the wrong notation. + _, err := normaliseShiftTime("7pm") + if err == nil || !strings.Contains(err.Error(), "HH:MM") { + t.Fatalf("the refusal does not say what to type: %v", err) + } +}