diff --git a/server/internal/api/api.go b/server/internal/api/api.go index 03aafec..60e48c2 100644 --- a/server/internal/api/api.go +++ b/server/internal/api/api.go @@ -137,6 +137,8 @@ type Store interface { // --- platform administration --- CreateClientWithOwner(ctx context.Context, in NewClientInput) (NewClientResult, error) + CreateCustomer(ctx context.Context, clientID string, in Profile, createdBy string) (Customer, error) + MergeVisitors(ctx context.Context, clientID, sourceID, targetID string) (MergeResult, error) Sales(ctx context.Context, q SaleQuery) ([]Sale, error) Sale(ctx context.Context, clientID, id string) (Sale, error) @@ -356,6 +358,12 @@ func (s *Server) Routes() *http.ServeMux { mux.HandleFunc("GET /api/visitors", s.tenantOnly(s.handleVisitors)) mux.HandleFunc("GET /api/visitors/{id}/history", s.tenantOnly(s.handleVisitorHistory)) mux.HandleFunc("PUT /api/visitors/{id}/profile", s.tenantOnly(s.handleSaveProfile)) + + // A customer registered before any camera has seen them, and the repair + // path that creates the need for: with no face template, recognition + // cannot match them later and enrols them again. + mux.HandleFunc("POST /api/customers", s.tenantOnly(s.handleCreateCustomer)) + mux.HandleFunc("POST /api/visitors/{id}/merge", s.tenantOnly(s.handleMergeCustomers)) mux.HandleFunc("POST /api/purchases", s.tenantOnly(s.handlePurchase)) // Reading sales, not just aggregating them. /api/reports/conversion has @@ -612,6 +620,11 @@ func looksLikeUUID(s string) bool { // without importing the store package. var ErrNoSecrets = errors.New("this server has no encryption key, so camera passwords cannot be stored") +// ErrSameVisitor is a merge that names one customer twice. Declared here +// rather than in the store for the reason ErrNoSecrets is: the store imports +// this package, so a sentinel the other way round is an import cycle. +var ErrSameVisitor = errors.New("a customer cannot be merged into themselves") + // ErrNoSnapshot means a camera has no stored picture. An ordinary state - a // camera added a minute ago has none - so it is reported as absence, never as // a failure. diff --git a/server/internal/api/customers_test.go b/server/internal/api/customers_test.go new file mode 100644 index 0000000..aca9714 --- /dev/null +++ b/server/internal/api/customers_test.go @@ -0,0 +1,148 @@ +package api + +import ( + "encoding/json" + "net/http" + "testing" +) + +func TestStaffCanRegisterACustomerNobodyHasPhotographed(t *testing.T) { + s, fs := newServer(t) + seedUser(fs) + sess := login(t, s, "manager@acme.com", "correct horse battery") + + rec := do(t, s, "POST", "/api/customers", sess.Token, map[string]string{ + "full_name": "Asha Menon", "phone": "9876543210", + }) + if rec.Code != http.StatusCreated { + t.Fatalf("got %d: %s", rec.Code, rec.Body.String()) + } + var c Customer + if err := json.Unmarshal(rec.Body.Bytes(), &c); err != nil { + t.Fatal(err) + } + // A reference a person can say, from the same counter the engine uses. + if c.Ref == "" || c.Label != "Asha Menon" { + t.Errorf("got ref=%q label=%q, want a V- reference and the typed name", + c.Ref, c.Label) + } +} + +// A record with no name and no phone is a number nobody can search for, and +// the customer at the counter is the only source of either. +func TestACustomerNeedsANameOrAPhone(t *testing.T) { + s, fs := newServer(t) + seedUser(fs) + sess := login(t, s, "manager@acme.com", "correct horse battery") + + rec := do(t, s, "POST", "/api/customers", sess.Token, + map[string]string{"notes": "regular, likes the window seat"}) + if rec.Code != http.StatusBadRequest { + t.Errorf("got %d, want 400: %s", rec.Code, rec.Body.String()) + } +} + +// ---------------------------------------------------------------- merge + +// Uuid-shaped on purpose: resolveVisitor takes a uuid or a V- reference and +// correctly refuses anything else, so a made-up id would 404 before reaching +// the handler under test. +const ( + vTyped = "aaaaaaaa-1111-4111-8111-aaaaaaaaaaaa" // typed in at the counter + vSeen = "bbbbbbbb-2222-4222-8222-bbbbbbbbbbbb" // enrolled by a camera +) + +func seedTwoCustomers(fs *fakeStore) { + seedUser(fs) + fs.visitors = []Customer{ + {ID: vTyped, Ref: "V-1", Label: "Asha Menon", FullName: "Asha Menon"}, + {ID: vSeen, Ref: "V-2", Label: "Visitor 2"}, + } +} + +func TestMergingFoldsOneCustomerIntoTheOtherAndSaysWhatMoved(t *testing.T) { + s, fs := newServer(t) + seedTwoCustomers(fs) + sess := login(t, s, "manager@acme.com", "correct horse battery") + + rec := do(t, s, "POST", "/api/visitors/"+vTyped+"/merge", sess.Token, + map[string]string{"into": vSeen}) + if rec.Code != http.StatusOK { + t.Fatalf("got %d: %s", rec.Code, rec.Body.String()) + } + var out MergeResult + if err := json.Unmarshal(rec.Body.Bytes(), &out); err != nil { + t.Fatal(err) + } + // The reference that STOPPED resolving has to be named. Staff write these + // on cards; discovering it at a counter is the wrong place to find out. + if out.RetiredRef != "V-1" || out.Ref != "V-2" { + t.Errorf("kept %q retired %q, want V-2 kept and V-1 retired", out.Ref, out.RetiredRef) + } +} + +// The only irreversible operation on a customer apart from erasure. Two people +// welded together cannot be separated: nothing records which visit came from +// whom. +func TestStaffCannotMerge(t *testing.T) { + s, fs := newServer(t) + seedTwoCustomers(fs) + fs.addUser("shopfloor@acme.com", "correct horse battery", UserRecord{ + ID: "u9", ClientID: "client-acme", Role: "staff", Active: true, + }) + sess := login(t, s, "shopfloor@acme.com", "correct horse battery") + + rec := do(t, s, "POST", "/api/visitors/"+vTyped+"/merge", sess.Token, + map[string]string{"into": vSeen}) + if rec.Code != http.StatusForbidden { + t.Errorf("got %d, want 403 for staff: %s", rec.Code, rec.Body.String()) + } +} + +func TestMergingACustomerIntoThemselvesIsRefused(t *testing.T) { + s, fs := newServer(t) + seedTwoCustomers(fs) + sess := login(t, s, "manager@acme.com", "correct horse battery") + + rec := do(t, s, "POST", "/api/visitors/"+vTyped+"/merge", sess.Token, + map[string]string{"into": vTyped}) + if rec.Code != http.StatusBadRequest { + t.Errorf("got %d, want 400: %s", rec.Code, rec.Body.String()) + } +} + +func TestMergingNeedsATarget(t *testing.T) { + s, fs := newServer(t) + seedTwoCustomers(fs) + sess := login(t, s, "manager@acme.com", "correct horse battery") + + for _, body := range []map[string]string{{}, {"into": " "}, {"into": "V-999"}} { + rec := do(t, s, "POST", "/api/visitors/"+vTyped+"/merge", sess.Token, body) + if rec.Code == http.StatusOK { + t.Errorf("merge with %v succeeded, want a refusal", body) + } + } +} + +// Every merge leaves a trace: it is destructive and cannot be undone. +func TestAMergeIsAudited(t *testing.T) { + s, fs := newServer(t) + seedTwoCustomers(fs) + sess := login(t, s, "manager@acme.com", "correct horse battery") + + do(t, s, "POST", "/api/visitors/"+vTyped+"/merge", sess.Token, + map[string]string{"into": vSeen}) + + found := false + for _, a := range fs.audits { + if a.Action == "customer.merge" { + found = true + if a.Detail["retired_ref"] != "V-1" { + t.Errorf("audit must name the retired reference: %+v", a.Detail) + } + } + } + if !found { + t.Error("no audit row for a merge") + } +} diff --git a/server/internal/api/fake_test.go b/server/internal/api/fake_test.go index 8091164..599be14 100644 --- a/server/internal/api/fake_test.go +++ b/server/internal/api/fake_test.go @@ -58,6 +58,8 @@ type fakeStore struct { // cameraOwner method below. siteOwner map[string]string // site id -> client id salesRows []Sale + visitorSeq int64 + lastMerge [2]string saleOwner map[string]string // sale id -> client id lastSaleQuery SaleQuery clientRows map[string]ClientDetail @@ -317,6 +319,57 @@ func (f *fakeStore) SiteHealth(_ context.Context, clientID string) ([]SiteHealth return out, nil } +func (f *fakeStore) CreateCustomer(_ context.Context, clientID string, + in Profile, createdBy string) (Customer, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.visitorSeq++ + label := in.FullName + if label == "" { + label = fmt.Sprintf("Visitor %d", f.visitorSeq) + } + c := Customer{ + ID: fmt.Sprintf("new-%d", f.visitorSeq), Ref: VisitorRef(f.visitorSeq), + Label: label, FullName: in.FullName, Phone: in.Phone, Email: in.Email, + HasProfile: true, + } + f.visitors = append(f.visitors, c) + f.lastProfile = in + return c, nil +} + +func (f *fakeStore) MergeVisitors(_ context.Context, clientID, sourceID, targetID string) ( + MergeResult, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.lastMerge = [2]string{sourceID, targetID} + if sourceID == targetID { + return MergeResult{}, ErrSameVisitor + } + var src, dst *Customer + for i := range f.visitors { + switch f.visitors[i].ID { + case sourceID: + src = &f.visitors[i] + case targetID: + dst = &f.visitors[i] + } + } + if src == nil || dst == nil { + return MergeResult{}, pgx.ErrNoRows + } + out := MergeResult{VisitorID: dst.ID, Ref: dst.Ref, Label: dst.Label, + RetiredRef: src.Ref} + var kept []Customer + for _, c := range f.visitors { + if c.ID != sourceID { + kept = append(kept, c) + } + } + f.visitors = kept + return out, nil +} + func (f *fakeStore) SetUserPassword(_ context.Context, userID, hash string) error { f.mu.Lock() defer f.mu.Unlock() diff --git a/server/internal/api/handlers_customers.go b/server/internal/api/handlers_customers.go new file mode 100644 index 0000000..4f9a185 --- /dev/null +++ b/server/internal/api/handlers_customers.go @@ -0,0 +1,134 @@ +// Registering a customer nobody has photographed, and joining two records +// that are one person. +// +// They ship together because the first creates the need for the second. A +// customer typed in at a counter has no face template, so when a camera later +// sees that person the matcher has nothing to compare against and enrols them +// as somebody new. That is the design working, not failing - and it means +// every hand-created customer is a duplicate waiting to happen. The merge is +// the way back, and without it this pair of endpoints would manufacture +// unrecoverable duplicates. +package api + +import ( + "errors" + "net/http" + + "github.com/jackc/pgx/v5" +) + +// handleCreateCustomer is staff and above - the same bar as filling in a +// profile, because that is what this is: a profile that arrives before the +// face rather than after it. +func (s *Server) handleCreateCustomer(w http.ResponseWriter, r *http.Request) { + p := PrincipalFrom(r.Context()) + if !p.CanWriteProfiles() { + writeErr(w, http.StatusForbidden, "forbidden", + "Staff and above can add a customer.") + return + } + + var in Profile + if err := decode(w, r, &in); err != nil { + badRequest(w, err.Error()) + return + } + in.FullName = clip(trim(in.FullName), 200) + in.Phone = clip(trim(in.Phone), 40) + in.Email = clip(trim(in.Email), 200) + in.Gender = clip(trim(in.Gender), 40) + in.Notes = clip(trim(in.Notes), 2000) + + // Something has to identify them to a human. A record with no name and no + // phone is a number nobody can search for, and the customer standing at + // the counter is the only source of either. + if in.FullName == "" && in.Phone == "" { + badRequest(w, "give at least a name or a phone number") + return + } + + out, err := s.Store.CreateCustomer(r.Context(), p.ClientID, in, p.UserID) + if err != nil { + s.serverError(w, "create customer", err) + return + } + + s.Store.Audit(r.Context(), AuditEntry{ + ClientID: p.ClientID, ActorID: p.UserID, ActorKind: "user", + Action: "customer.create", Entity: "visitor", EntityID: out.ID, + Detail: map[string]any{"ref": out.Ref}, + }) + writeJSON(w, http.StatusCreated, out) +} + +// handleMergeCustomers folds one customer into another. +// +// Manager and above, not staff. This is the only irreversible operation on a +// customer record apart from erasure: two people welded together cannot be +// separated afterwards, because nothing records which visit came from whom. +// The edge gallery draws the same line for the same reason. +func (s *Server) handleMergeCustomers(w http.ResponseWriter, r *http.Request) { + p := PrincipalFrom(r.Context()) + if !p.CanManageSites() { + writeErr(w, http.StatusForbidden, "forbidden", + "Merging two customers cannot be undone; a manager or owner must do it.") + return + } + + source, ok := s.resolveVisitor(w, r, r.PathValue("id")) + if !ok { + return + } + var in MergeRequest + if err := decode(w, r, &in); err != nil { + badRequest(w, err.Error()) + return + } + if trim(in.Into) == "" { + badRequest(w, `"into" must name the customer to keep`) + return + } + // Resolved through the same path, so "into" accepts V-42 as well as a + // uuid - the reference staff actually read off a screen. + target, err := s.visitorIDFor(r.Context(), p.ClientID, trim(in.Into)) + if err != nil { + s.serverError(w, "resolve customer", err) + return + } + if target == "" { + writeErr(w, http.StatusNotFound, "not_found", "No such customer to merge into.") + return + } + + out, err := s.Store.MergeVisitors(r.Context(), p.ClientID, source, target) + switch { + case errors.Is(err, ErrSameVisitor): + badRequest(w, "that is the same customer") + return + case errors.Is(err, pgx.ErrNoRows): + // One of the two is gone, erased, or another tenant's. All three read + // as absent; which one it is only helps somebody probing ids. + writeErr(w, http.StatusNotFound, "not_found", "No such customer.") + return + case err != nil: + s.serverError(w, "merge customers", err) + return + } + + // Irreversible, so it leaves a trace at WARNING as well as in the audit + // log - the same rule the edge gallery's merge follows. + s.logf("WARNING merge: customer %s (%s) folded into %s (%s) by %s: "+ + "%d visits, %d purchases, %d templates moved", + source, out.RetiredRef, out.VisitorID, out.Ref, p.Email, + out.Visits, out.Purchases, out.Embeddings) + s.Store.Audit(r.Context(), AuditEntry{ + ClientID: p.ClientID, ActorID: p.UserID, ActorKind: "user", + Action: "customer.merge", Entity: "visitor", EntityID: out.VisitorID, + Detail: map[string]any{ + "retired_ref": out.RetiredRef, "kept_ref": out.Ref, + "visits": out.Visits, "purchases": out.Purchases, + "embeddings": out.Embeddings, + }, + }) + writeJSON(w, http.StatusOK, out) +} diff --git a/server/internal/api/handlers_password.go b/server/internal/api/handlers_password.go index fc6a4de..9320bda 100644 --- a/server/internal/api/handlers_password.go +++ b/server/internal/api/handlers_password.go @@ -77,7 +77,7 @@ func (s *Server) handleChangePassword(w http.ResponseWriter, r *http.Request) { // The password IS changed. Reporting a failure here would tell the // user to try again, and the retry would fail on the current password // they just replaced. - s.Log.Printf("change password: revoke other sessions: %v", err) + s.logf("change password: revoke other sessions: %v", err) } s.Store.Audit(r.Context(), AuditEntry{ diff --git a/server/internal/api/types.go b/server/internal/api/types.go index 6cf2323..d97be8a 100644 --- a/server/internal/api/types.go +++ b/server/internal/api/types.go @@ -180,6 +180,27 @@ type ChangePassword struct { NewPassword string `json:"new_password"` } +// MergeResult says what moved, so an operator sees the size of a thing that +// cannot be undone rather than a bare "ok". +type MergeResult struct { + VisitorID string `json:"visitor_id"` + Ref string `json:"ref"` + Label string `json:"label"` + Visits int `json:"visits"` + Purchases int `json:"purchases"` + Embeddings int `json:"embeddings"` + Consents int `json:"consents"` + // RetiredRef is the reference that has STOPPED resolving. Staff write + // these on cards and read them aloud, so a merge has to say which one + // died rather than leaving somebody to discover it at a counter. + RetiredRef string `json:"retired_ref"` +} + +// MergeRequest names the record to keep. +type MergeRequest struct { + Into string `json:"into"` +} + type Customer struct { ID string `json:"id"` // Ref is the customer number - "V-42" - and is accepted anywhere this diff --git a/server/internal/store/api_customers.go b/server/internal/store/api_customers.go new file mode 100644 index 0000000..ecaba15 --- /dev/null +++ b/server/internal/store/api_customers.go @@ -0,0 +1,238 @@ +// Creating a customer nobody has photographed, and joining two records that +// turn out to be one person. +// +// These ship together on purpose. A customer created by hand has no face, so +// when a camera later sees that person the matcher has nothing to compare +// against and records them as somebody new - by construction, not by failure. +// Shipping the create without the merge would mean manufacturing duplicates +// with no way back, which is the state CLAUDE.md already flags for the server: +// "there is no merge endpoint server-side, so its duplicates would be +// unrecoverable." +package store + +import ( + "context" + "fmt" + "time" + + "github.com/jackc/pgx/v5" + + "github.com/loyaly/behavision-server/internal/api" +) + +// CreateCustomer registers a person before any camera has seen them. +// +// The number comes from the same counter, taken the same way, as a customer +// the engine enrols: `UPDATE ... RETURNING` inside the transaction. Two +// sources of visitor numbers that could disagree would be worse than none, +// and V-42 has to mean one person whichever way they arrived. +func (s *Store) CreateCustomer(ctx context.Context, clientID string, + in api.Profile, createdBy string) (api.Customer, error) { + + tx, err := s.pool.Begin(ctx) + if err != nil { + return api.Customer{}, err + } + defer tx.Rollback(ctx) + + var number int64 + if err := tx.QueryRow(ctx, ` + UPDATE clients SET visitor_seq = visitor_seq + 1 + WHERE id = $1::uuid RETURNING visitor_seq`, clientID).Scan(&number); err != nil { + return api.Customer{}, fmt.Errorf("next visitor number: %w", err) + } + + // The label is the person's name when they gave one, and "Visitor N" + // otherwise - the same string the engine would have written, so a record + // created by hand is indistinguishable from an enrolled one afterwards. + // Formatted in Go, never as `'Visitor ' || $2::text` beside `number = $2`: + // one parameter used as a bigint and as a string operand makes Postgres + // deduce two types for it and refuse the whole insert. + label := in.FullName + if label == "" { + label = fmt.Sprintf("Visitor %d", number) + } + + now := time.Now().UTC() + var id string + if err := tx.QueryRow(ctx, ` + INSERT INTO visitors (client_id, number, label, first_seen_at, visit_count) + VALUES ($1::uuid, $2, $3, $4, 0) RETURNING id::text`, + clientID, number, label, now).Scan(&id); err != nil { + return api.Customer{}, err + } + + if _, err := tx.Exec(ctx, ` + INSERT INTO visitor_profiles (visitor_id, client_id, full_name, phone, + email, gender, notes, collected_by) + VALUES ($1::uuid, $2::uuid, $3, $4, $5, $6, $7, NULLIF($8,'')::uuid)`, + id, clientID, in.FullName, in.Phone, in.Email, in.Gender, in.Notes, + createdBy); err != nil { + return api.Customer{}, err + } + + if err := tx.Commit(ctx); err != nil { + return api.Customer{}, err + } + return api.Customer{ + ID: id, Ref: api.VisitorRef(number), Label: label, + FullName: in.FullName, Phone: in.Phone, Email: in.Email, + VisitCount: 0, HasProfile: true, + FirstSeenAt: now.Format(time.RFC3339), + }, nil +} + +// ErrSameVisitor is the API package's sentinel, aliased rather than +// redeclared - two values would compare unequal and errors.Is would miss. +var ErrSameVisitor = api.ErrSameVisitor + +// MergeVisitors folds `sourceID` into `targetID` and deletes the source. +// +// One transaction, because a half-merge - visits moved, profile not - leaves +// two records each holding part of one person, which is strictly worse than +// the duplicate it was called to fix. +// +// Five tables reference visitors and every one is re-pointed here. A merge +// that misses a table is the same half-merge arrived at by omission, and +// ON DELETE CASCADE means the miss is not an error: the rows are silently +// destroyed with the source row. +func (s *Store) MergeVisitors(ctx context.Context, clientID, sourceID, targetID string) ( + api.MergeResult, error) { + + var out api.MergeResult + if sourceID == targetID { + return out, ErrSameVisitor + } + + tx, err := s.pool.Begin(ctx) + if err != nil { + return out, err + } + defer tx.Rollback(ctx) + + // Both must exist, belong to this tenant, and not already be erased. + // Locked in a stable order so two operators merging the same pair in + // opposite directions deadlock on nothing and one simply loses. + var srcNum, dstNum int64 + var srcLabel, dstLabel string + var srcFirst, dstFirst time.Time + rows, err := tx.Query(ctx, ` + SELECT id::text, number, label, first_seen_at FROM visitors + WHERE client_id = $1::uuid AND id::text IN ($2, $3) + AND deleted_at IS NULL + ORDER BY id FOR UPDATE`, clientID, sourceID, targetID) + if err != nil { + return out, err + } + found := 0 + for rows.Next() { + var id, label string + var num int64 + var first time.Time + if err := rows.Scan(&id, &num, &label, &first); err != nil { + rows.Close() + return out, err + } + found++ + if id == sourceID { + srcNum, srcLabel, srcFirst = num, label, first + } else { + dstNum, dstLabel, dstFirst = num, label, first + } + } + rows.Close() + if err := rows.Err(); err != nil { + return out, err + } + if found != 2 { + return out, pgx.ErrNoRows + } + + // Profile: visitor_profiles is UNIQUE on visitor_id, so the two cannot + // simply both move. Blanks on the survivor are filled from the source and + // nothing the survivor already holds is overwritten - which is exactly + // right for the case this exists for, a hand-typed name and phone being + // joined to the face that was recognised later. + if _, err := tx.Exec(ctx, ` + INSERT INTO visitor_profiles (visitor_id, client_id, full_name, phone, + email, gender, notes, collected_by, collected_at) + SELECT $2::uuid, client_id, full_name, phone, email, gender, notes, + collected_by, collected_at + FROM visitor_profiles WHERE visitor_id = $1::uuid + ON CONFLICT (visitor_id) DO UPDATE SET + full_name = CASE WHEN visitor_profiles.full_name = '' THEN EXCLUDED.full_name ELSE visitor_profiles.full_name END, + phone = CASE WHEN visitor_profiles.phone = '' THEN EXCLUDED.phone ELSE visitor_profiles.phone END, + email = CASE WHEN visitor_profiles.email = '' THEN EXCLUDED.email ELSE visitor_profiles.email END, + gender = CASE WHEN visitor_profiles.gender = '' THEN EXCLUDED.gender ELSE visitor_profiles.gender END, + notes = CASE WHEN visitor_profiles.notes = '' THEN EXCLUDED.notes ELSE visitor_profiles.notes END, + updated_at = now()`, sourceID, targetID); err != nil { + return out, fmt.Errorf("merge profile: %w", err) + } + + for _, q := range []struct { + name, sql string + count *int + }{ + {"visits", `UPDATE visits SET visitor_id = $2::uuid + WHERE visitor_id = $1::uuid AND client_id = $3::uuid`, &out.Visits}, + {"purchases", `UPDATE purchases SET visitor_id = $2::uuid + WHERE visitor_id = $1::uuid AND client_id = $3::uuid`, &out.Purchases}, + {"embeddings", `UPDATE visitor_embeddings SET visitor_id = $2::uuid + WHERE visitor_id = $1::uuid AND client_id = $3::uuid`, &out.Embeddings}, + {"consents", `UPDATE consents SET visitor_id = $2::uuid + WHERE visitor_id = $1::uuid AND client_id = $3::uuid`, &out.Consents}, + } { + tag, err := tx.Exec(ctx, q.sql, sourceID, targetID, clientID) + if err != nil { + return out, fmt.Errorf("merge %s: %w", q.name, err) + } + *q.count = int(tag.RowsAffected()) + } + + // A human-assigned name outranks an auto "Visitor N", whichever direction + // the operator merged in. Silently turning "Alice" back into "Visitor 3" + // is data loss they cannot see happen. + label := dstLabel + if isAutoLabel(dstLabel, dstNum) && !isAutoLabel(srcLabel, srcNum) { + label = srcLabel + } + // first_seen_at takes the earlier of the two: it is one person and always + // was. visit_count is recomputed with COUNT(*), never summed - the stored + // counters may themselves be stale, and the row count cannot be. + first := dstFirst + if srcFirst.Before(first) { + first = srcFirst + } + if _, err := tx.Exec(ctx, ` + UPDATE visitors SET + label = $2, first_seen_at = $3, + last_seen_at = GREATEST(last_seen_at, + (SELECT max(occurred_at) FROM visits WHERE visitor_id = $1::uuid)), + visit_count = (SELECT count(*) FROM visits WHERE visitor_id = $1::uuid) + WHERE id = $1::uuid`, targetID, label, first); err != nil { + return out, fmt.Errorf("merge totals: %w", err) + } + + // The source goes for real. A soft delete would leave its number resolving + // to a record with nothing in it, which reads as "this customer exists and + // has never been here" - a worse answer than "no such customer". + if _, err := tx.Exec(ctx, `DELETE FROM visitors WHERE id = $1::uuid`, sourceID); err != nil { + return out, fmt.Errorf("delete merged customer: %w", err) + } + if err := tx.Commit(ctx); err != nil { + return out, err + } + + out.VisitorID = targetID + out.Ref = api.VisitorRef(dstNum) + out.RetiredRef = api.VisitorRef(srcNum) + out.Label = label + return out, nil +} + +// isAutoLabel reports whether a label is the one the system writes itself. +// Compared against the record's OWN number: "Visitor 7" on customer 42 was +// typed by a person and is a name, however unhelpful. +func isAutoLabel(label string, number int64) bool { + return label == fmt.Sprintf("Visitor %d", number) +} diff --git a/server/internal/store/api_customers_live_test.go b/server/internal/store/api_customers_live_test.go new file mode 100644 index 0000000..8cb1170 --- /dev/null +++ b/server/internal/store/api_customers_live_test.go @@ -0,0 +1,227 @@ +package store + +import ( + "context" + "testing" + "time" + + "github.com/loyaly/behavision-server/internal/api" +) + +// The case the whole feature exists for: a customer typed in at a counter, +// then recognised by a camera a week later as somebody new, then joined. +// +// Live, because every property below is in the SQL and because the failure is +// SILENT: five tables reference visitors with ON DELETE CASCADE, so a table +// this merge forgets to re-point is not an error - those rows are destroyed +// with the source row and nobody finds out until a customer's history is +// short. +func TestLiveMergeMovesEverythingAndLosesNothing(t *testing.T) { + st := liveStore(t) + ctx := context.Background() + clientID, siteID := seedTenant(t, st, "merge"+stamp(), 0, false) + + // The hand-typed record: a name and a phone, no face, no visits. + typed, err := st.CreateCustomer(ctx, clientID, + api.Profile{FullName: "Asha Menon", Phone: "9876543210"}, "") + if err != nil { + t.Fatalf("create customer: %v", err) + } + if typed.Ref == "" { + t.Fatal("a hand-created customer must get a speakable reference") + } + + // The record a camera made later. Two visits, a purchase, a template and a + // consent - one row in every table that references visitors. + seen, visitIDs := seedRecognisedVisitor(t, st, clientID, siteID, 2) + seedSale(t, st, clientID, visitIDs[0], seen, 250, "INR") + seedEmbedding(t, st, clientID, seen) + seedConsent(t, st, clientID, seen) + + out, err := st.MergeVisitors(ctx, clientID, typed.ID, seen) + if err != nil { + t.Fatalf("merge: %v", err) + } + if out.VisitorID != seen { + t.Fatalf("survivor %s, want %s", out.VisitorID, seen) + } + if out.RetiredRef != typed.Ref { + t.Errorf("retired ref %q, want %q - staff write these down", + out.RetiredRef, typed.Ref) + } + + // Nothing orphaned, nothing cascaded away. + for _, c := range []struct { + what, sql string + want int + }{ + {"visits", `SELECT count(*) FROM visits WHERE visitor_id = $1::uuid`, 2}, + {"purchases", `SELECT count(*) FROM purchases WHERE visitor_id = $1::uuid`, 1}, + {"embeddings", `SELECT count(*) FROM visitor_embeddings WHERE visitor_id = $1::uuid`, 1}, + {"consents", `SELECT count(*) FROM consents WHERE visitor_id = $1::uuid`, 1}, + {"profiles", `SELECT count(*) FROM visitor_profiles WHERE visitor_id = $1::uuid`, 1}, + } { + var n int + if err := st.pool.QueryRow(ctx, c.sql, seen).Scan(&n); err != nil { + t.Fatalf("%s: %v", c.what, err) + } + if n != c.want { + t.Errorf("%s on the survivor = %d, want %d", c.what, n, c.want) + } + } + + // The typed-in name reached the record that has the face. That IS the + // feature: the survivor had no profile, so the source's fills it. + var name, phone string + if err := st.pool.QueryRow(ctx, + `SELECT full_name, phone FROM visitor_profiles WHERE visitor_id = $1::uuid`, + seen).Scan(&name, &phone); err != nil { + t.Fatalf("profile: %v", err) + } + if name != "Asha Menon" || phone != "9876543210" { + t.Errorf("profile = %q / %q, want the typed-in details", name, phone) + } + + // A human-assigned name outranks an auto "Visitor N", whichever way round + // the operator merged. + var label string + var visitCount int + if err := st.pool.QueryRow(ctx, + `SELECT label, visit_count FROM visitors WHERE id = $1::uuid`, + seen).Scan(&label, &visitCount); err != nil { + t.Fatal(err) + } + if label != "Asha Menon" { + t.Errorf("label %q, want the human name to survive the auto one", label) + } + // Recomputed with COUNT(*), never summed: the stored counters may be stale + // and the row count cannot be. + if visitCount != 2 { + t.Errorf("visit_count = %d, want 2 counted from the rows", visitCount) + } + + // The source is gone for real. A soft delete would leave its number + // resolving to a record holding nothing. + var left int + if err := st.pool.QueryRow(ctx, + `SELECT count(*) FROM visitors WHERE id = $1::uuid`, typed.ID).Scan(&left); err != nil { + t.Fatal(err) + } + if left != 0 { + t.Error("the merged-away customer is still there") + } +} + +// The survivor's own details are never overwritten. Merging must not silently +// replace a name somebody checked with one they did not. +func TestLiveMergeFillsBlanksAndOverwritesNothing(t *testing.T) { + st := liveStore(t) + ctx := context.Background() + clientID, _ := seedTenant(t, st, "keep"+stamp(), 0, false) + + src, _ := st.CreateCustomer(ctx, clientID, + api.Profile{FullName: "Wrong Name", Phone: "1111111111", Email: "a@b.c"}, "") + dst, _ := st.CreateCustomer(ctx, clientID, + api.Profile{FullName: "Right Name"}, "") + + if _, err := st.MergeVisitors(ctx, clientID, src.ID, dst.ID); err != nil { + t.Fatalf("merge: %v", err) + } + var name, phone, email string + if err := st.pool.QueryRow(ctx, + `SELECT full_name, phone, email FROM visitor_profiles WHERE visitor_id = $1::uuid`, + dst.ID).Scan(&name, &phone, &email); err != nil { + t.Fatal(err) + } + if name != "Right Name" { + t.Errorf("name = %q, want the survivor's own kept", name) + } + if phone != "1111111111" || email != "a@b.c" { + t.Errorf("blanks not filled: phone=%q email=%q", phone, email) + } +} + +// Another tenant's customer is not mergeable, and reads as absent. +func TestLiveMergeRefusesAcrossTenants(t *testing.T) { + st := liveStore(t) + ctx := context.Background() + aID, _ := seedTenant(t, st, "ta"+stamp(), 0, false) + bID, _ := seedTenant(t, st, "tb"+stamp(), 0, false) + + a, _ := st.CreateCustomer(ctx, aID, api.Profile{FullName: "A"}, "") + b, _ := st.CreateCustomer(ctx, bID, api.Profile{FullName: "B"}, "") + + if _, err := st.MergeVisitors(ctx, aID, a.ID, b.ID); err == nil { + t.Fatal("merged a customer into another tenant's record") + } + var n int + st.pool.QueryRow(ctx, `SELECT count(*) FROM visitors WHERE id = $1::uuid`, a.ID).Scan(&n) + if n != 1 { + t.Error("a refused merge must change nothing") + } +} + +// -------------------------------------------------------------- fixtures + +func seedRecognisedVisitor(t *testing.T, st *Store, clientID, siteID string, visits int) ( + visitorID string, visitIDs []string) { + t.Helper() + ctx := context.Background() + at := time.Date(2026, 9, 10, 9, 0, 0, 0, time.UTC) + var number int64 + if err := st.pool.QueryRow(ctx, ` + UPDATE clients SET visitor_seq = visitor_seq + 1 + WHERE id = $1::uuid RETURNING visitor_seq`, clientID).Scan(&number); err != nil { + t.Fatal(err) + } + if err := st.pool.QueryRow(ctx, ` + INSERT INTO visitors (client_id, number, label, first_seen_at, visit_count) + VALUES ($1::uuid, $2, 'Visitor '||$2, $3, 0) RETURNING id::text`, + clientID, number, at).Scan(&visitorID); err != nil { + t.Fatal(err) + } + for i := 0; i < visits; i++ { + var id string + if err := st.pool.QueryRow(ctx, ` + INSERT INTO visits (client_id, site_id, visitor_id, source_event_id, + occurred_at, camera_id, is_new_visitor) + VALUES ($1::uuid, $2::uuid, $3::uuid, $4, $5, 'door', $6) + RETURNING id::text`, + clientID, siteID, visitorID, stamp()+string(rune('a'+i)), + at.Add(time.Duration(i)*time.Hour), i == 0).Scan(&id); err != nil { + t.Fatal(err) + } + visitIDs = append(visitIDs, id) + } + return visitorID, visitIDs +} + +func seedEmbedding(t *testing.T, st *Store, clientID, visitorID string) { + t.Helper() + v := make([]float32, 512) + for i := range v { + v[i] = 0.04 + } + var siteID string + if err := st.pool.QueryRow(context.Background(), + `SELECT id::text FROM sites WHERE client_id = $1::uuid LIMIT 1`, + clientID).Scan(&siteID); err != nil { + t.Fatalf("site for embedding: %v", err) + } + if _, err := st.pool.Exec(context.Background(), ` + INSERT INTO visitor_embeddings + (visitor_id, client_id, model, embedding, quality, source_site_id) + VALUES ($1::uuid, $2::uuid, 'w600k_r50', $3::vector, 0.8, $4::uuid)`, + visitorID, clientID, pgVector(v), siteID); err != nil { + t.Fatalf("seed embedding: %v", err) + } +} + +func seedConsent(t *testing.T, st *Store, clientID, visitorID string) { + t.Helper() + if _, err := st.pool.Exec(context.Background(), ` + INSERT INTO consents (client_id, visitor_id, scope, granted) + VALUES ($1::uuid, $2::uuid, 'marketing', true)`, clientID, visitorID); err != nil { + t.Fatalf("seed consent: %v", err) + } +}