Hold web-created staff to the same rules the till applies

Two paths write `app_users`: the console's `tenants/createstaff`, and the
terminal's `/pos/users`. Only one of them checked anything.

`createstaff` wrote whatever it was handed. A cashier could be created there
with PIN "0451" — which a bigint column stores as 451 — and would then type four
digits at the counter and be refused for ever, with nothing on either screen to
explain it. Or with 1234, which live data already has on eleven accounts. Or
with a PIN somebody at the same outlet already had, which attributes a bill to
whichever row is read first. Or with no way to sign in at all.

None of that surfaced where it was caused. It surfaced at a counter, days later,
as "the new person cannot log in".

So the rules move into `ValidateStaffUser`, and both paths use it: a name, a
role that is actually a role, a PIN the schema can hold and nobody guesses
first, and at least one way to sign in. The duplicate-PIN check runs too, when
the row names an outlet.

The handler also stops answering 500 with a body claiming 409. Every one of
these is something the person filling in the form can fix, so it is a 400
carrying the reason.

`GetStaffs` now returns `rolename` alongside `roleid`, so a console can show
"Supervisor" without mapping ids itself — `app_roles` has six rows for four
back-office roles and most accounts carry an id absent from it, so any mapping
written client-side would be wrong.

This is what makes the two role systems one. A supervisor or cashier created
from the web behaves at the till exactly like one created at the till, because
there is now a single definition of what those are.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

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{
+		// A rejected PIN, a missing name, a role nobody set — these are things
-			"code":    http.StatusConflict,
+		// 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,
 		})

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"`

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
+}

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")
+			}
+		})
+	}
+}

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
 	}