diff --git a/controllers/tenantController.go b/controllers/tenantController.go index e329cc9..d2016a1 100644 --- a/controllers/tenantController.go +++ b/controllers/tenantController.go @@ -328,8 +328,12 @@ func (ctl *TenantController) CreateStaff(c *fiber.Ctx) error { } if err := ctl.tenantService.CreateStaff(data); err != nil { - return c.Status(http.StatusInternalServerError).JSON(fiber.Map{ - "code": http.StatusConflict, + // A rejected PIN, a missing name, a role nobody set — these are things + // the person filling in the form can fix, so they come back as 400 with + // the reason. This answered 500 with a body claiming 409, which told a + // console nothing it could act on and told the operator less. + return c.Status(http.StatusBadRequest).JSON(fiber.Map{ + "code": http.StatusBadRequest, "message": err.Error(), "status": false, }) diff --git a/models/tenant.go b/models/tenant.go index 40ce920..88934f0 100644 --- a/models/tenant.go +++ b/models/tenant.go @@ -121,6 +121,10 @@ type Tenantpricing struct { type StaffInfo struct { Userid int `json:"userid"` + // What the role is called, so a console does not have to map ids itself. + // `app_roles` holds six rows for four back-office roles and most accounts + // carry an id absent from it, so any mapping written client-side is wrong. + Rolename string `json:"rolename"` Authname string `json:"authname"` Configid int `json:"configid"` Authmode int `json:"authmode"` diff --git a/repositories/posUserRepository.go b/repositories/posUserRepository.go index e1f3a99..2d520ef 100644 --- a/repositories/posUserRepository.go +++ b/repositories/posUserRepository.go @@ -435,3 +435,57 @@ func splitName(full string) (first, last string) { } return parts[0], strings.Join(parts[1:], " ") } + +// ValidateStaffUser applies the till's rules to a staff row from anywhere. +// +// Exported because the web console writes `app_users` too, through +// `tenants/createstaff`, and that path had no validation whatsoever — no PIN +// rules, no role check, no duplicate check. A cashier created there could be +// given "0451", which a bigint column stores as 451, and would then type four +// digits at the counter and be refused for ever with nothing to explain it. +// +// Two paths writing one table drift apart. This is the shared rule set, so a +// person created from a browser and a person created from a till are subject to +// the same constraints and behave the same way at the counter. +// +// Returns the parsed PIN, or an error a caller can show to whoever typed it. +func ValidateStaffUser(user *models.User) (int64, error) { + if strings.TrimSpace(user.Firstname+user.Lastname) == "" { + return 0, fmt.Errorf("a name is required") + } + + // Only the roles this platform actually defines. `roleid` 0 is the one that + // matters: it is not a role, it is what a row carries when nobody set one, + // and live data has riders and shop accounts sharing it. + if user.Roleid <= 0 { + return 0, fmt.Errorf("a role is required") + } + + pin := int64(user.Pin) + if pin != 0 { + parsed, err := validatePosPin(strconv.FormatInt(pin, 10)) + if err != nil { + return 0, err + } + pin = parsed + } + + if pin == 0 && strings.TrimSpace(user.Password) == "" { + return 0, fmt.Errorf("set a PIN, a password, or both — otherwise this person cannot sign in") + } + + return pin, nil +} + +// StaffPinAvailable reports whether a PIN is free at an outlet. +// +// Exported for the same reason as [ValidateStaffUser]: the web console needs +// the check the till already makes. Two people sharing a PIN would attribute a +// bill to whichever row happened to be read first. +func (r *posRepository) StaffPinAvailable(tenantID, locationID int, pin int64, exceptUser int) (bool, error) { + if pin == 0 { + return true, nil + } + taken, err := posPinTaken(r.db, tenantID, locationID, pin, exceptUser) + return !taken, err +} diff --git a/repositories/posUserRepository_test.go b/repositories/posUserRepository_test.go index d86d61b..24ee422 100644 --- a/repositories/posUserRepository_test.go +++ b/repositories/posUserRepository_test.go @@ -130,3 +130,63 @@ func TestARoleIsReadFromItsNameNotItsNumber(t *testing.T) { } } } + +// The web console writes `app_users` too, through `tenants/createstaff`, and +// that path had no validation at all. These cover the shared rule set, so a +// person created from a browser is subject to the same constraints as one +// created at a till — two paths writing one table is how they drift. + +func TestStaffFromTheWebConsoleObeysTheTillsRules(t *testing.T) { + cases := []struct { + name string + user models.User + ok bool + }{ + { + name: "a usable cashier", + user: models.User{Firstname: "Asha", Roleid: models.PosRoleCashier, Pin: 7391}, + ok: true, + }, + { + name: "a password instead of a PIN is fine", + user: models.User{Firstname: "Asha", Roleid: models.PosRoleCashier, Password: "s3cret"}, + ok: true, + }, + { + name: "no name", + user: models.User{Roleid: models.PosRoleCashier, Pin: 7391}, + }, + { + name: "no role — 0 is unset, not a role", + user: models.User{Firstname: "Asha", Pin: 7391}, + }, + { + name: "no way at all to sign in", + user: models.User{Firstname: "Asha", Roleid: models.PosRoleCashier}, + }, + { + // 451 is what "0451" becomes in a bigint column. Accepting it here + // creates somebody who types four digits and is refused for ever. + name: "a PIN the column cannot hold", + user: models.User{Firstname: "Asha", Roleid: models.PosRoleCashier, Pin: 451}, + }, + { + name: "a PIN anyone would guess first", + user: models.User{Firstname: "Asha", Roleid: models.PosRoleCashier, Pin: 1234}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + user := tc.user + _, err := ValidateStaffUser(&user) + + if tc.ok && err != nil { + t.Fatalf("refused a valid staff row: %v", err) + } + if !tc.ok && err == nil { + t.Fatal("accepted a staff row the till could not use") + } + }) + } +} diff --git a/repositories/tenantRepository.go b/repositories/tenantRepository.go index fa32615..b3c31f7 100644 --- a/repositories/tenantRepository.go +++ b/repositories/tenantRepository.go @@ -320,9 +320,11 @@ func (r *tenantRepository) GetStaffs(tid int) ([]models.StaffInfo, error) { a.email,a.contactno,a.address,a.suburb,a.city, a.state,a.postcode,a.userfcmtoken,a.pin,a.applocationid, a.roleid,a.partnerid,a.tenantid,a.locationid, - b.locationname + b.locationname, + COALESCE(c.rolename,'') AS rolename FROM app_users a INNER JOIN tenantlocations b ON a.locationid = b.locationid + LEFT JOIN app_roles c ON c.roleid = a.roleid WHERE a.tenantid = ?` if err := r.db.Raw(q1, tid).Scan(&data).Error; err != nil { @@ -332,7 +334,34 @@ func (r *tenantRepository) GetStaffs(tid int) ([]models.StaffInfo, error) { return data, nil } +// CreateStaff adds a person to a shop from the web console. +// +// Now subject to the same rules the till applies — see ValidateStaffUser. This +// wrote whatever it was handed, so a cashier could be created with a PIN the +// schema cannot store, a PIN somebody else already has, or no way to sign in at +// all. The failure surfaced at the counter rather than on the screen that +// caused it. +// +// `userid` is deliberately not set: it is a `GENERATED BY DEFAULT AS IDENTITY` +// column and Postgres allocates it. Computing one here would leave the sequence +// unadvanced and two allocators racing each other. func (r *tenantRepository) CreateStaff(user models.User) error { + pin, err := ValidateStaffUser(&user) + if err != nil { + return err + } + user.Pin = int(pin) + + if pin > 0 && user.Tenantid > 0 && user.Locationid > 0 { + taken, err := posPinTaken(r.db, user.Tenantid, user.Locationid, pin, user.Userid) + if err != nil { + return err + } + if taken { + return fmt.Errorf("another person at this outlet already uses that PIN") + } + } + if err := r.db.Table("app_users").Create(&user).Error; err != nil { return err }