From 580412402afeb776e20e0181ecc6acce1ac7b8e5 Mon Sep 17 00:00:00 2001 From: SirLouen Date: Sun, 27 Sep 2026 22:35:05 +0200 Subject: [PATCH 01/25] test(features): pin the names a field cannot take --- test/features/features/fields-catalog.feature | 47 +++++++++++- test/features/steps_fields_repeater_test.go | 16 ----- test/features/steps_fields_test.go | 71 ++++++++++++++++++- 3 files changed, 116 insertions(+), 18 deletions(-) diff --git a/test/features/features/fields-catalog.feature b/test/features/features/fields-catalog.feature index ebca012b..90703e7a 100644 --- a/test/features/features/fields-catalog.feature +++ b/test/features/features/fields-catalog.feature @@ -2,7 +2,8 @@ Feature: An operator shapes the field catalogue Contact fields are defined at runtime by an operator. A definition carries a machine name, a human label and a kind. The catalogue refuses names the compiled schema already owns, so a runtime field can never shadow a real - one. + one. It also refuses the names every JavaScript object already holds, so a + browser reads each field as it was stored. Background: Given a running AlphOne holding a user with an API token @@ -20,6 +21,42 @@ Feature: An operator shapes the field catalogue When the operator defines the field "name" labelled "Name" of kind TEXT Then the definition is refused for a reserved name + @wip + Scenario Outline: A name every JavaScript object holds is refused + When the operator defines the field "" labelled "Points" of kind NUMBER + Then the definition is refused with the reason "field_name_reserved" + And the catalogue does not list "" + + Examples: + | name | + | constructor | + | hasOwnProperty | + | isPrototypeOf | + | propertyIsEnumerable | + | toLocaleString | + | toString | + | valueOf | + + @wip + Scenario: The catalogue lists every name a field cannot take + When the operator asks which names a field cannot take + Then the names a field cannot take are: + | name | + | constructor | + | createdAt | + | field | + | hasOwnProperty | + | id | + | identities | + | isPrototypeOf | + | name | + | propertyIsEnumerable | + | tasks | + | toLocaleString | + | toString | + | valueOf | + | whatsAppConversations | + Scenario: A malformed name is refused When the operator defines the field "birth date" labelled "Birth date" of kind DATE Then the definition is refused for a malformed name @@ -29,3 +66,11 @@ Feature: An operator shapes the field catalogue When the operator archives the field "birthDate" Then the catalogue does not list "birthDate" And the catalogue lists "birthDate" among archived definitions + + @wip + Scenario: Defining an archived field again answers the field it brings back + Given the field "birthDate" labelled "Birth date" of kind DATE is defined + And the operator archives the field "birthDate" + When the operator defines the field "birthDate" labelled "Date of birth" of kind DATE + Then the definition answers the id the catalogue lists for "birthDate" + And the catalogue lists "birthDate" with label "Date of birth" and kind DATE diff --git a/test/features/steps_fields_repeater_test.go b/test/features/steps_fields_repeater_test.go index 49cc39ae..f7e79cdc 100644 --- a/test/features/steps_fields_repeater_test.go +++ b/test/features/steps_fields_repeater_test.go @@ -129,22 +129,6 @@ func bindRepeaterDefinitionSteps(sc *godog.ScenarioContext) { } return fmt.Errorf("the catalogue lists no field named %q, answered %s", name, w.answered) }) - - sc.Then(`^the (?:definition|change) is refused with the reason "([^"]*)"$`, - func(ctx context.Context, reason string) error { - w := worldFrom(ctx) - var answer graphAnswer - if err := json.Unmarshal(w.answered, &answer); err != nil { - return fmt.Errorf("decoding %s: %w", w.answered, err) - } - if len(answer.Errors) == 0 { - return fmt.Errorf("the graph accepted it, answered %s", w.answered) - } - if got := answer.Errors[0].Extensions["reason"]; got != reason { - return fmt.Errorf("reason = %v, want %s, answered %s", got, reason, w.answered) - } - return nil - }) } // bindRepeaterRowSteps binds the step that writes a whole list of rows through the field values. diff --git a/test/features/steps_fields_test.go b/test/features/steps_fields_test.go index e1acec7d..b9757c0a 100644 --- a/test/features/steps_fields_test.go +++ b/test/features/steps_fields_test.go @@ -6,6 +6,7 @@ import ( "context" "encoding/json" "fmt" + "slices" "strings" "testing" @@ -26,6 +27,9 @@ const fieldsQuery = `query($includeArchived: Boolean) { fields(includeArchived: $includeArchived) { id name label kind archivedAt } }` +// reservedFieldNamesQuery reads the names a field cannot take through the graph. +const reservedFieldNamesQuery = `query { reservedFieldNames }` + // graphAnswer is the envelope every catalogue step reads. type graphAnswer struct { Data struct { @@ -43,6 +47,7 @@ type graphAnswer struct { ArchivedAt *string `json:"archivedAt"` SubFields []map[string]any `json:"subFields"` } `json:"fields"` + ReservedFieldNames []string `json:"reservedFieldNames"` } `json:"data"` Errors []struct { Message string `json:"message"` @@ -74,8 +79,9 @@ func (w *world) operationAs( return answer, nil } -// defineField declares a field, remembering the id when the graph accepts it. +// defineField declares a field, remembering the id when the graph accepts it and forgetting it otherwise. func (w *world) defineField(ctx context.Context, name, label, kind string) error { + w.lastField = uuid.Nil answer, err := w.operation(ctx, defineFieldMutation, map[string]any{"name": name, "label": label, "kind": kind}) if err != nil { @@ -200,4 +206,67 @@ func bindFieldsCatalogSteps(sc *godog.ScenarioContext) { } return nil }) + + sc.Then(`^the (?:definition|change) is refused with the reason "([^"]*)"$`, + func(ctx context.Context, reason string) error { + w := worldFrom(ctx) + var answer graphAnswer + if err := json.Unmarshal(w.answered, &answer); err != nil { + return fmt.Errorf("decoding %s: %w", w.answered, err) + } + if len(answer.Errors) == 0 { + return fmt.Errorf("the graph accepted it, answered %s", w.answered) + } + if got := answer.Errors[0].Extensions["reason"]; got != reason { + return fmt.Errorf("reason = %v, want %s, answered %s", got, reason, w.answered) + } + return nil + }) + + bindReservedNameSteps(sc) +} + +// bindReservedNameSteps binds the steps that read the names a field cannot take and the id a define answers. +func bindReservedNameSteps(sc *godog.ScenarioContext) { + sc.When(`^the operator asks which names a field cannot take$`, func(ctx context.Context) error { + _, err := worldFrom(ctx).operation(ctx, reservedFieldNamesQuery, map[string]any{}) + return err + }) + + sc.Then(`^the names a field cannot take are:$`, func(ctx context.Context, table *godog.Table) error { + w := worldFrom(ctx) + var answer graphAnswer + if err := json.Unmarshal(w.answered, &answer); err != nil { + return fmt.Errorf("decoding %s: %w", w.answered, err) + } + want := make([]string, 0, len(table.Rows)) + for _, row := range table.Rows[1:] { + want = append(want, row.Cells[0].Value) + } + if !slices.Equal(answer.Data.ReservedFieldNames, want) { + return fmt.Errorf("names = %v, want %v, answered %s", answer.Data.ReservedFieldNames, want, w.answered) + } + return nil + }) + + sc.Then(`^the definition answers the id the catalogue lists for "([^"]*)"$`, + func(ctx context.Context, name string) error { + w := worldFrom(ctx) + if w.lastField == uuid.Nil { + return fmt.Errorf("the definition answered no id, answered %s", w.answered) + } + listed, err := w.operation(ctx, fieldsQuery, map[string]any{}) + if err != nil { + return err + } + for _, held := range listed.Data.Fields { + if held.Name == name && held.ID != w.lastField.String() { + return fmt.Errorf("the definition answered %s, the catalogue lists %s", w.lastField, held.ID) + } + if held.Name == name { + return nil + } + } + return fmt.Errorf("the catalogue lists no field named %q, answered %s", name, w.answered) + }) } From 7862825ccc4c5160a70e6fd7d6cded50fb46364d Mon Sep 17 00:00:00 2001 From: SirLouen Date: Mon, 28 Sep 2026 09:04:23 +0200 Subject: [PATCH 02/25] fix(fields): refuse a field name every JavaScript object holds --- plugins/fields/field.go | 10 ++++++++-- plugins/fields/field_internal_test.go | 15 +++++++++++++++ plugins/fields/graphql_test.go | 15 ++++++++++++++- 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/plugins/fields/field.go b/plugins/fields/field.go index 002bbcfa..02a30dde 100644 --- a/plugins/fields/field.go +++ b/plugins/fields/field.go @@ -19,7 +19,7 @@ var ( errUnknownKind = errors.New("fields: unknown kind") errBlankLabel = errors.New("fields: a label carries text") errLabelTooLong = fmt.Errorf("fields: a label runs to %d characters", labelMax) - errReservedName = errors.New("fields: the name is already a field of the type") + errReservedName = errors.New("fields: the name is reserved") errSubFieldsRequired = errors.New("fields: a repeater holds at least one sub field") errSubFieldsUnexpected = errors.New("fields: only a repeater holds sub fields") @@ -64,6 +64,12 @@ var kinds = map[kind]string{ // namePattern matches the camelCase names a definition accepts. var namePattern = regexp.MustCompile(`^[a-z][a-zA-Z0-9]*$`) +// inheritedNames lists the camelCase members every JavaScript object inherits. +var inheritedNames = map[string]bool{ + "constructor": true, "hasOwnProperty": true, "isPrototypeOf": true, "propertyIsEnumerable": true, + "toLocaleString": true, "toString": true, "valueOf": true, +} + // scalar reports the GraphQL scalar the kind answers with. func (k kind) scalar() string { return kinds[k] @@ -94,7 +100,7 @@ func newDefinition( if !namePattern.MatchString(name) { return Definition{}, errMalformedName } - if reserved[name] { + if reserved[name] || inheritedNames[name] { return Definition{}, errReservedName } held := kind(declared) diff --git a/plugins/fields/field_internal_test.go b/plugins/fields/field_internal_test.go index fe42ffbe..5be9712b 100644 --- a/plugins/fields/field_internal_test.go +++ b/plugins/fields/field_internal_test.go @@ -116,6 +116,21 @@ func TestNewDefinitionRefusesAReservedName(t *testing.T) { } } +func TestNewDefinitionRefusesANameEveryObjectInherits(t *testing.T) { + t.Parallel() + + for _, name := range []string{ + "constructor", "hasOwnProperty", "isPrototypeOf", "propertyIsEnumerable", + "toLocaleString", "toString", "valueOf", + } { + _, err := newDefinition(name, "Points", "NUMBER", nil) + + if !errors.Is(err, errReservedName) { + t.Errorf("newDefinition(%q) error = %v, want errReservedName", name, err) + } + } +} + func TestNewDefinitionRefusesALabelBeyondTheCap(t *testing.T) { t.Parallel() diff --git a/plugins/fields/graphql_test.go b/plugins/fields/graphql_test.go index fbfc4a6e..fccc6b78 100644 --- a/plugins/fields/graphql_test.go +++ b/plugins/fields/graphql_test.go @@ -168,11 +168,24 @@ func TestGraphRefusesANameTheSchemaOwns(t *testing.T) { var created struct{ DefineField definition } err := client.Post(`mutation { defineField(name: "name", label: "Name", kind: TEXT) { id } }`, &created) - if err == nil || !strings.Contains(err.Error(), "already a field") { + if err == nil || !strings.Contains(err.Error(), "the name is reserved") { t.Errorf("error = %v, want the reserved name refused", err) } } +func TestGraphRefusesANameEveryObjectInherits(t *testing.T) { + t.Parallel() + + client := newFieldsClient(t) + + var created struct{ DefineField definition } + err := client.Post(`mutation { defineField(name: "constructor", label: "Points", kind: NUMBER) { id } }`, &created) + + if err == nil || !strings.Contains(err.Error(), "the name is reserved") { + t.Errorf("error = %v, want the inherited name refused", err) + } +} + func TestGraphRefusesADuplicateName(t *testing.T) { t.Parallel() From 806ac020738c715b8823d18429a67943baa1d612 Mon Sep 17 00:00:00 2001 From: SirLouen Date: Mon, 28 Sep 2026 09:04:24 +0200 Subject: [PATCH 03/25] test(features): run the scenario refusing inherited field names --- test/features/features/fields-catalog.feature | 1 - 1 file changed, 1 deletion(-) diff --git a/test/features/features/fields-catalog.feature b/test/features/features/fields-catalog.feature index 90703e7a..2076a5bf 100644 --- a/test/features/features/fields-catalog.feature +++ b/test/features/features/fields-catalog.feature @@ -21,7 +21,6 @@ Feature: An operator shapes the field catalogue When the operator defines the field "name" labelled "Name" of kind TEXT Then the definition is refused for a reserved name - @wip Scenario Outline: A name every JavaScript object holds is refused When the operator defines the field "" labelled "Points" of kind NUMBER Then the definition is refused with the reason "field_name_reserved" From c42a6822584631dae44ddf61b60d4d63fbf963cb Mon Sep 17 00:00:00 2001 From: SirLouen Date: Mon, 28 Sep 2026 09:39:06 +0200 Subject: [PATCH 04/25] fix(fields): move a stored field off a name every object holds --- plugins/fields/migration_internal_test.go | 162 ++++++++++++++++++ .../migrations/00007_move_inherited_names.sql | 33 ++++ 2 files changed, 195 insertions(+) create mode 100644 plugins/fields/migrations/00007_move_inherited_names.sql diff --git a/plugins/fields/migration_internal_test.go b/plugins/fields/migration_internal_test.go index f3cbb90a..ca6f4179 100644 --- a/plugins/fields/migration_internal_test.go +++ b/plugins/fields/migration_internal_test.go @@ -20,6 +20,9 @@ import ( // entryIDsVersion is the migration that gives every stored repeater entry an id. const entryIDsVersion = 6 +// inheritedNamesVersion is the migration that moves every definition off a name every JavaScript object inherits. +const inheritedNamesVersion = 7 + // fieldsProvider returns a goose provider over the plugin's migrations and its own version table. func fieldsProvider(t *testing.T, db *sql.DB) *goose.Provider { t.Helper() @@ -223,6 +226,165 @@ func TestEntryIDsMigrationRestoresTheStoredListsGoingDown(t *testing.T) { } } +// beforeInheritedNames returns a plugin and its database rolled back to the schema before inherited names move. +func beforeInheritedNames(t *testing.T) (*Plugin, *sql.DB, *goose.Provider) { + t.Helper() + p := newMigratedPlugin(t) + db := stdlib.OpenDBFromPool(p.pool) + t.Cleanup(func() { _ = db.Close() }) + provider := fieldsProvider(t, db) + if _, err := provider.DownTo(t.Context(), inheritedNamesVersion-1); err != nil { + t.Fatalf("rolling back to the schema before inherited names move: %v", err) + } + return p, db, provider +} + +// definitionsIn returns whether each definition a tenant holds is archived, by name. +func definitionsIn(t *testing.T, db *sql.DB, tenant uuid.UUID) map[string]bool { + t.Helper() + rows, err := db.QueryContext(t.Context(), + "SELECT name, archived_at IS NOT NULL FROM plugin_fields.definitions WHERE tenant_id = $1", tenant) + if err != nil { + t.Fatalf("reading the definitions: %v", err) + } + defer func() { _ = rows.Close() }() + held := map[string]bool{} + for rows.Next() { + var name string + var archived bool + if err := rows.Scan(&name, &archived); err != nil { + t.Fatalf("scanning a definition: %v", err) + } + held[name] = archived + } + if err := rows.Err(); err != nil { + t.Fatalf("reading the definitions: %v", err) + } + return held +} + +// moveInheritedNames applies the migration that moves definitions off inherited names. +func moveInheritedNames(t *testing.T, provider *goose.Provider) { + t.Helper() + if _, err := provider.UpTo(t.Context(), inheritedNamesVersion); err != nil { + t.Fatalf("moving inherited names: %v", err) + } +} + +func TestInheritedNamesMigrationMovesAFieldAndItsValues(t *testing.T) { + t.Parallel() + + _, db, provider := beforeInheritedNames(t) + home := sdk.TenantOrDefault(t.Context()) + storedDefinition(t, db, home, "constructor", "NUMBER", "[]", false) + storedDefinition(t, db, home, "nickname", "TEXT", "[]", false) + maria := contactHolding(t, db, home, `{"constructor": 5, "nickname": "Mari"}`) + + moveInheritedNames(t, provider) + + want := map[string]bool{"constructor2": false, "nickname": false} + if held := definitionsIn(t, db, home); !reflect.DeepEqual(held, want) { + t.Errorf("definitions = %v, want constructor moved to constructor2", held) + } + if held := heldValues(t, db, maria); !reflect.DeepEqual(held, decoded(t, `{"constructor2": 5, "nickname": "Mari"}`)) { + t.Errorf("values = %#v, want the value moved with its field", held) + } +} + +func TestInheritedNamesMigrationStepsPastANameTheWorkspaceHolds(t *testing.T) { + t.Parallel() + + _, db, provider := beforeInheritedNames(t) + home := sdk.TenantOrDefault(t.Context()) + storedDefinition(t, db, home, "constructor", "NUMBER", "[]", false) + storedDefinition(t, db, home, "constructor2", "TEXT", "[]", false) + maria := contactHolding(t, db, home, `{"constructor": 5, "constructor2": "Kept"}`) + + moveInheritedNames(t, provider) + + want := map[string]bool{"constructor2": false, "constructor3": false} + if held := definitionsIn(t, db, home); !reflect.DeepEqual(held, want) { + t.Errorf("definitions = %v, want constructor moved past the held constructor2", held) + } + moved := decoded(t, `{"constructor2": "Kept", "constructor3": 5}`) + if held := heldValues(t, db, maria); !reflect.DeepEqual(held, moved) { + t.Errorf("values = %#v, want the moved value under constructor3 and constructor2 kept", held) + } +} + +func TestInheritedNamesMigrationNumbersEachWorkspaceOnItsOwn(t *testing.T) { + t.Parallel() + + p, db, provider := beforeInheritedNames(t) + home := sdk.TenantOrDefault(t.Context()) + acme := sdk.TenantOrDefault(inTenant(t, p)) + storedDefinition(t, db, home, "valueOf", "NUMBER", "[]", false) + storedDefinition(t, db, home, "valueOf2", "NUMBER", "[]", false) + storedDefinition(t, db, acme, "valueOf", "NUMBER", "[]", false) + + moveInheritedNames(t, provider) + + want := map[string]bool{"valueOf2": false, "valueOf3": false} + if held := definitionsIn(t, db, home); !reflect.DeepEqual(held, want) { + t.Errorf("home definitions = %v, want valueOf moved to valueOf3", held) + } + if held := definitionsIn(t, db, acme); !reflect.DeepEqual(held, map[string]bool{"valueOf2": false}) { + t.Errorf("acme definitions = %v, want valueOf moved to valueOf2", held) + } +} + +func TestInheritedNamesMigrationMovesArchivedFieldsToo(t *testing.T) { + t.Parallel() + + _, db, provider := beforeInheritedNames(t) + home := sdk.TenantOrDefault(t.Context()) + storedDefinition(t, db, home, "toString", "TEXT", "[]", true) + storedDefinition(t, db, home, "hasOwnProperty", "BOOLEAN", "[]", false) + + moveInheritedNames(t, provider) + + want := map[string]bool{"toString2": true, "hasOwnProperty2": false} + if held := definitionsIn(t, db, home); !reflect.DeepEqual(held, want) { + t.Errorf("definitions = %v, want both moved, the archived one still archived", held) + } +} + +func TestInheritedNamesMigrationClearsStrayValuesUnderTheNewName(t *testing.T) { + t.Parallel() + + _, db, provider := beforeInheritedNames(t) + home := sdk.TenantOrDefault(t.Context()) + storedDefinition(t, db, home, "constructor", "NUMBER", "[]", false) + stray := contactHolding(t, db, home, `{"constructor2": "Left behind", "nickname": "Rosa"}`) + + moveInheritedNames(t, provider) + + if held := heldValues(t, db, stray); !reflect.DeepEqual(held, decoded(t, `{"nickname": "Rosa"}`)) { + t.Errorf("values = %#v, want the stray constructor2 value cleared", held) + } +} + +func TestInheritedNamesMigrationLeavesOtherFieldsAlone(t *testing.T) { + t.Parallel() + + p, db, provider := beforeInheritedNames(t) + home := sdk.TenantOrDefault(t.Context()) + acme := sdk.TenantOrDefault(inTenant(t, p)) + storedDefinition(t, db, home, "nickname", "TEXT", "[]", false) + storedDefinition(t, db, acme, "constructor", "NUMBER", "[]", false) + literal := `{"nickname": "Rosa", "constructor2": "Kept"}` + rosa := contactHolding(t, db, home, literal) + + moveInheritedNames(t, provider) + + if held := definitionsIn(t, db, home); !reflect.DeepEqual(held, map[string]bool{"nickname": false}) { + t.Errorf("definitions = %v, want the home workspace untouched", held) + } + if held := heldValues(t, db, rosa); !reflect.DeepEqual(held, decoded(t, literal)) { + t.Errorf("values = %#v, want the home values left as stored", held) + } +} + func TestEntryIDsMigrationStopsAtASubFieldNamedID(t *testing.T) { t.Parallel() diff --git a/plugins/fields/migrations/00007_move_inherited_names.sql b/plugins/fields/migrations/00007_move_inherited_names.sql new file mode 100644 index 00000000..b7ab8a46 --- /dev/null +++ b/plugins/fields/migrations/00007_move_inherited_names.sql @@ -0,0 +1,33 @@ +-- SPDX-License-Identifier: Elastic-2.0 + +-- +goose Up +CREATE TEMPORARY TABLE inherited_moves ON COMMIT DROP AS +SELECT d.tenant_id, d.name AS inherited, d.name || ( + SELECT min(n) FROM generate_series(2, ( + SELECT count(*) + 2 FROM plugin_fields.definitions AS held WHERE held.tenant_id = d.tenant_id + )) AS n + WHERE NOT EXISTS ( + SELECT 1 FROM plugin_fields.definitions AS held + WHERE held.tenant_id = d.tenant_id AND held.name = d.name || n + ) +) AS moved +FROM plugin_fields.definitions AS d +WHERE d.name IN ('constructor', 'hasOwnProperty', 'isPrototypeOf', 'propertyIsEnumerable', + 'toLocaleString', 'toString', 'valueOf'); + +UPDATE plugin_fields.contact_values AS v +SET values = v.values + - ARRAY(SELECT m.moved FROM inherited_moves AS m WHERE m.tenant_id = v.tenant_id) + - ARRAY(SELECT m.inherited FROM inherited_moves AS m WHERE m.tenant_id = v.tenant_id) + || COALESCE(( + SELECT jsonb_object_agg(m.moved, v.values -> m.inherited) FROM inherited_moves AS m + WHERE m.tenant_id = v.tenant_id AND v.values ? m.inherited + ), '{}'::jsonb) +WHERE v.tenant_id IN (SELECT m.tenant_id FROM inherited_moves AS m); + +UPDATE plugin_fields.definitions AS d +SET name = m.moved +FROM inherited_moves AS m +WHERE d.tenant_id = m.tenant_id AND d.name = m.inherited; + +-- +goose Down From 45098e56ce34cf18ca1508ebec0411dc97849477 Mon Sep 17 00:00:00 2001 From: SirLouen Date: Mon, 28 Sep 2026 09:42:46 +0200 Subject: [PATCH 05/25] fix(fields): answer the stored id when a define brings a field back --- plugins/fields/entries_internal_test.go | 2 +- plugins/fields/graphql.go | 4 +- plugins/fields/graphql_internal_test.go | 4 +- plugins/fields/graphql_test.go | 18 +++++ plugins/fields/provider_internal_test.go | 2 +- plugins/fields/seed.go | 2 +- plugins/fields/store.go | 23 +++--- plugins/fields/store_internal_test.go | 77 +++++++++++++++------ plugins/fields/tenantscope_internal_test.go | 4 +- plugins/fields/values_internal_test.go | 2 +- 10 files changed, 97 insertions(+), 41 deletions(-) diff --git a/plugins/fields/entries_internal_test.go b/plugins/fields/entries_internal_test.go index be85e19f..28998534 100644 --- a/plugins/fields/entries_internal_test.go +++ b/plugins/fields/entries_internal_test.go @@ -241,7 +241,7 @@ func TestEntryMutationsCheckTheCallersOwnFields(t *testing.T) { _, adding := resolvers.AddContactFieldEntry(acme, contactID, "history", map[string]any{"comment": "call"}) wantRefusal(t, adding, "VALIDATION", "field_unknown") - if err := p.store.define(acme, historyOf(historyColumns...)); err != nil { + if _, err := p.store.define(acme, historyOf(historyColumns...)); err != nil { t.Fatalf("define() in Acme error = %v, want nil", err) } p.catalog.forget(acme) diff --git a/plugins/fields/graphql.go b/plugins/fields/graphql.go index 6fc5b609..ae899e66 100644 --- a/plugins/fields/graphql.go +++ b/plugins/fields/graphql.go @@ -92,12 +92,14 @@ func (m MutationResolvers) DefineField( if err != nil { return nil, sdk.GraphError{Code: "VALIDATION", Reason: fieldReason(err), Err: err} } - if err := m.plugin.store.define(ctx, definition); err != nil { + stored, err := m.plugin.store.define(ctx, definition) + if err != nil { if errors.Is(err, errNameTaken) || errors.Is(err, errKindLocked) { return nil, sdk.GraphError{Code: "CONFLICT", Reason: fieldReason(err), Err: err} } return nil, err } + definition.ID = stored m.plugin.catalog.forget(ctx) return toGraphDefinition(definition), nil } diff --git a/plugins/fields/graphql_internal_test.go b/plugins/fields/graphql_internal_test.go index e048bb9f..9b795201 100644 --- a/plugins/fields/graphql_internal_test.go +++ b/plugins/fields/graphql_internal_test.go @@ -161,7 +161,7 @@ func TestStoreReportsAClosedPool(t *testing.T) { p := newClosedPlugin(t) - if err := p.store.define(t.Context(), defined(t, "birthDate", "DATE")); err == nil { + if _, err := p.store.define(t.Context(), defined(t, "birthDate", "DATE")); err == nil { t.Error("create() error = nil, want the closed pool reported") } if err := p.store.archive(t.Context(), uuid.Must(uuid.NewV7())); err == nil { @@ -269,7 +269,7 @@ func TestFieldsListsArchivedDefinitionsOnRequest(t *testing.T) { p := newMigratedPlugin(t) stored := defined(t, "birthDate", "DATE") - if err := p.store.define(t.Context(), stored); err != nil { + if _, err := p.store.define(t.Context(), stored); err != nil { t.Fatalf("create() error = %v, want nil", err) } if err := p.store.archive(t.Context(), stored.ID); err != nil { diff --git a/plugins/fields/graphql_test.go b/plugins/fields/graphql_test.go index fccc6b78..5e077af3 100644 --- a/plugins/fields/graphql_test.go +++ b/plugins/fields/graphql_test.go @@ -201,6 +201,24 @@ func TestGraphRefusesADuplicateName(t *testing.T) { } } +func TestGraphAnswersTheIDOfARevivedField(t *testing.T) { + t.Parallel() + + client := newFieldsClient(t) + const define = `mutation { defineField(name: "birthDate", label: "Birth date", kind: DATE) { id } }` + var first, revived struct{ DefineField definition } + client.MustPost(define, &first) + var archived struct{ ArchiveField bool } + client.MustPost(`mutation($id: UUID!) { archiveField(id: $id) }`, &archived, + gqlclient.Var("id", first.DefineField.ID)) + + client.MustPost(define, &revived) + + if revived.DefineField.ID != first.DefineField.ID { + t.Errorf("revived id = %s, want the id the field kept, %s", revived.DefineField.ID, first.DefineField.ID) + } +} + func TestGraphArchivesAField(t *testing.T) { t.Parallel() diff --git a/plugins/fields/provider_internal_test.go b/plugins/fields/provider_internal_test.go index 3fe027f5..15bce434 100644 --- a/plugins/fields/provider_internal_test.go +++ b/plugins/fields/provider_internal_test.go @@ -39,7 +39,7 @@ func storedValues(t *testing.T, p *Plugin, ctx context.Context, contactID uuid.U // define stores a definition and forgets the catalogue view it changes. func define(t *testing.T, p *Plugin, definition Definition) { t.Helper() - if err := p.store.define(t.Context(), definition); err != nil { + if _, err := p.store.define(t.Context(), definition); err != nil { t.Fatalf("define(%q) error = %v, want nil", definition.Name, err) } p.catalog.forget(t.Context()) diff --git a/plugins/fields/seed.go b/plugins/fields/seed.go index 6769b896..6a41eb78 100644 --- a/plugins/fields/seed.go +++ b/plugins/fields/seed.go @@ -64,7 +64,7 @@ func (p *Plugin) seedDefinitions(ctx context.Context) error { } demo.ID = uuid.Must(uuid.NewV7()) demo.CreatedAt = time.Now().UTC() - if err := p.store.define(ctx, demo); err != nil { + if _, err := p.store.define(ctx, demo); err != nil { return err } } diff --git a/plugins/fields/store.go b/plugins/fields/store.go index f40fd189..e1de477b 100644 --- a/plugins/fields/store.go +++ b/plugins/fields/store.go @@ -31,8 +31,8 @@ type store struct { pool *pgxpool.Pool } -// define stores a definition, reviving an archived one of the same shape, and clears values under a name defined anew. -func (s *store) define(ctx context.Context, definition Definition) error { +// define stores a definition, reviving an archived one of the same shape, and answers the id the stored row keeps. +func (s *store) define(ctx context.Context, definition Definition) (uuid.UUID, error) { const statement = `WITH stored AS ( INSERT INTO plugin_fields.definitions (id, name, label, kind, created_at, tenant_id, sub_fields) @@ -45,25 +45,26 @@ func (s *store) define(ctx context.Context, definition Definition) error { = jsonb_path_query_array(EXCLUDED.sub_fields, '$[*].name') AND jsonb_path_query_array(plugin_fields.definitions.sub_fields, '$[*].kind') = jsonb_path_query_array(EXCLUDED.sub_fields, '$[*].kind') - RETURNING 1 + RETURNING id ), swept AS ( UPDATE plugin_fields.contact_values SET values = values - $2::text WHERE tenant_id = $6 AND values ? $2::text AND EXISTS (SELECT 1 FROM stored) AND NOT EXISTS (SELECT 1 FROM plugin_fields.definitions WHERE tenant_id = $6 AND name = $2) ) - SELECT count(*) FROM stored` - var stored int - if err := s.pool.QueryRow(ctx, statement, + SELECT id FROM stored` + var stored uuid.UUID + err := s.pool.QueryRow(ctx, statement, definition.ID, definition.Name, definition.Label, string(definition.Kind), definition.CreatedAt, sdk.TenantOrDefault(ctx), - append([]SubField{}, definition.SubFields...)).Scan(&stored); err != nil { - return fmt.Errorf("fields: define definition: %w", err) + append([]SubField{}, definition.SubFields...)).Scan(&stored) + if errors.Is(err, pgx.ErrNoRows) { + return uuid.Nil, s.errorFor(ctx, definition) } - if stored == 0 { - return s.errorFor(ctx, definition) + if err != nil { + return uuid.Nil, fmt.Errorf("fields: define definition: %w", err) } - return nil + return stored, nil } // errorFor reports why a definition the store refused could not be written. diff --git a/plugins/fields/store_internal_test.go b/plugins/fields/store_internal_test.go index 87cc6199..786dd6be 100644 --- a/plugins/fields/store_internal_test.go +++ b/plugins/fields/store_internal_test.go @@ -46,7 +46,7 @@ func TestStoreRoundTripsADefinition(t *testing.T) { p := newMigratedPlugin(t) definition := defined(t, "birthDate", "DATE") - if err := p.store.define(t.Context(), definition); err != nil { + if _, err := p.store.define(t.Context(), definition); err != nil { t.Fatalf("define() error = %v, want nil", err) } @@ -69,11 +69,11 @@ func TestStoreRefusesADuplicateName(t *testing.T) { t.Parallel() p := newMigratedPlugin(t) - if err := p.store.define(t.Context(), defined(t, "birthDate", "DATE")); err != nil { + if _, err := p.store.define(t.Context(), defined(t, "birthDate", "DATE")); err != nil { t.Fatalf("define() error = %v, want nil", err) } - err := p.store.define(t.Context(), defined(t, "birthDate", "TEXT")) + _, err := p.store.define(t.Context(), defined(t, "birthDate", "TEXT")) if !errors.Is(err, errNameTaken) { t.Errorf("error = %v, want errNameTaken", err) @@ -85,14 +85,14 @@ func TestStoreRevivesAnArchivedDefinition(t *testing.T) { p := newMigratedPlugin(t) original := defined(t, "birthDate", "DATE") - if err := p.store.define(t.Context(), original); err != nil { + if _, err := p.store.define(t.Context(), original); err != nil { t.Fatalf("define() error = %v, want nil", err) } if err := p.store.archive(t.Context(), original.ID); err != nil { t.Fatalf("archive() error = %v, want nil", err) } - if err := p.store.define(t.Context(), defined(t, "birthDate", "DATE")); err != nil { + if _, err := p.store.define(t.Context(), defined(t, "birthDate", "DATE")); err != nil { t.Fatalf("define() error = %v, want the archived definition revived", err) } @@ -108,6 +108,38 @@ func TestStoreRevivesAnArchivedDefinition(t *testing.T) { } } +func TestStoreDefineAnswersTheIDOfARevivedDefinition(t *testing.T) { + t.Parallel() + + p := newMigratedPlugin(t) + original := defined(t, "birthDate", "DATE") + if _, err := p.store.define(t.Context(), original); err != nil { + t.Fatalf("define() error = %v, want nil", err) + } + if err := p.store.archive(t.Context(), original.ID); err != nil { + t.Fatalf("archive() error = %v, want nil", err) + } + + id, err := p.store.define(t.Context(), defined(t, "birthDate", "DATE")) + + if err != nil || id != original.ID { + t.Errorf("define() = %s, %v, want the original id %s", id, err, original.ID) + } +} + +func TestStoreDefineAnswersTheIDOfANewDefinition(t *testing.T) { + t.Parallel() + + p := newMigratedPlugin(t) + definition := defined(t, "birthDate", "DATE") + + id, err := p.store.define(t.Context(), definition) + + if err != nil || id != definition.ID { + t.Errorf("define() = %s, %v, want the new id %s", id, err, definition.ID) + } +} + func TestStoreClearsStrayValuesWhenATenantFirstDefinesANameAnotherTenantHolds(t *testing.T) { t.Parallel() @@ -123,7 +155,7 @@ func TestStoreClearsStrayValuesWhenATenantFirstDefinesANameAnotherTenantHolds(t t.Fatalf("writeValues() elsewhere error = %v, want nil", err) } - if err := p.store.define(acme, defined(t, "shoeSize", "NUMBER")); err != nil { + if _, err := p.store.define(acme, defined(t, "shoeSize", "NUMBER")); err != nil { t.Fatalf("define() error = %v, want nil", err) } @@ -141,7 +173,7 @@ func TestStoreKeepsTheValuesOfARevivedDefinition(t *testing.T) { p := newMigratedPlugin(t) contactID := seedContact(t, p, "Maria Perez") original := defined(t, "birthDate", "DATE") - if err := p.store.define(t.Context(), original); err != nil { + if _, err := p.store.define(t.Context(), original); err != nil { t.Fatalf("define() error = %v, want nil", err) } if err := p.store.writeValues(t.Context(), contactID, map[string]any{"birthDate": "1990-04-17"}); err != nil { @@ -151,7 +183,7 @@ func TestStoreKeepsTheValuesOfARevivedDefinition(t *testing.T) { t.Fatalf("archive() error = %v, want nil", err) } - if err := p.store.define(t.Context(), defined(t, "birthDate", "DATE")); err != nil { + if _, err := p.store.define(t.Context(), defined(t, "birthDate", "DATE")); err != nil { t.Fatalf("define() error = %v, want the archived definition revived", err) } @@ -203,7 +235,10 @@ func TestStoreKeepsALiveValueWhenARacingDefineIsRefused(t *testing.T) { } second := defined(t, "loyalty", "TEXT") done := make(chan error, 1) - go func() { done <- p.store.define(context.Background(), second) }() + go func() { + _, err := p.store.define(context.Background(), second) + done <- err + }() awaitLockedDefine(t, p) if err := racing.Commit(t.Context()); err != nil { @@ -223,14 +258,14 @@ func TestStoreClearsNothingWhenItRefusesADefinition(t *testing.T) { p := newMigratedPlugin(t) contactID := seedContact(t, p, "Maria Perez") - if err := p.store.define(t.Context(), defined(t, "birthDate", "DATE")); err != nil { + if _, err := p.store.define(t.Context(), defined(t, "birthDate", "DATE")); err != nil { t.Fatalf("define() error = %v, want nil", err) } if err := p.store.writeValues(t.Context(), contactID, map[string]any{"birthDate": "1990-04-17"}); err != nil { t.Fatalf("writeValues() error = %v, want nil", err) } - if err := p.store.define(t.Context(), defined(t, "birthDate", "DATE")); !errors.Is(err, errNameTaken) { + if _, err := p.store.define(t.Context(), defined(t, "birthDate", "DATE")); !errors.Is(err, errNameTaken) { t.Fatalf("define() error = %v, want errNameTaken", err) } @@ -244,14 +279,14 @@ func TestStoreRefusesRevivingUnderADifferentKind(t *testing.T) { p := newMigratedPlugin(t) original := defined(t, "birthDate", "DATE") - if err := p.store.define(t.Context(), original); err != nil { + if _, err := p.store.define(t.Context(), original); err != nil { t.Fatalf("define() error = %v, want nil", err) } if err := p.store.archive(t.Context(), original.ID); err != nil { t.Fatalf("archive() error = %v, want nil", err) } - err := p.store.define(t.Context(), defined(t, "birthDate", "TEXT")) + _, err := p.store.define(t.Context(), defined(t, "birthDate", "TEXT")) if !errors.Is(err, errKindLocked) { t.Errorf("error = %v, want errKindLocked", err) @@ -283,7 +318,7 @@ var historyColumns = []SubField{ func archivedRepeater(t *testing.T, p *Plugin) Definition { t.Helper() original := historyOf(historyColumns...) - if err := p.store.define(t.Context(), original); err != nil { + if _, err := p.store.define(t.Context(), original); err != nil { t.Fatalf("define() error = %v, want nil", err) } if err := p.store.archive(t.Context(), original.ID); err != nil { @@ -297,7 +332,7 @@ func TestStoreRoundTripsARepeatersSubFieldsInOrder(t *testing.T) { p := newMigratedPlugin(t) - if err := p.store.define(t.Context(), historyOf(historyColumns...)); err != nil { + if _, err := p.store.define(t.Context(), historyOf(historyColumns...)); err != nil { t.Fatalf("define() error = %v, want nil", err) } @@ -320,7 +355,7 @@ func TestStoreRevivesARepeaterHoldingTheSameSubFields(t *testing.T) { {Name: "comment", Label: "Note", Kind: kindLongText}, } - if err := p.store.define(t.Context(), historyOf(relabelled...)); err != nil { + if _, err := p.store.define(t.Context(), historyOf(relabelled...)); err != nil { t.Fatalf("define() error = %v, want the archived repeater revived", err) } @@ -354,7 +389,7 @@ func TestStoreRefusesRevivingARepeaterUnderOtherSubFields(t *testing.T) { p := newMigratedPlugin(t) archivedRepeater(t, p) - err := p.store.define(t.Context(), historyOf(subFields...)) + _, err := p.store.define(t.Context(), historyOf(subFields...)) if !errors.Is(err, errKindLocked) { t.Errorf("error = %v, want errKindLocked", err) @@ -378,7 +413,7 @@ func TestStoreRefusesSubFieldsOnlyARepeaterMayHold(t *testing.T) { p := newMigratedPlugin(t) - err := p.store.define(t.Context(), definition) + _, err := p.store.define(t.Context(), definition) var refused *pgconn.PgError if !errors.As(err, &refused) || refused.Code != checkViolation { @@ -393,7 +428,7 @@ func TestStoreArchiveHidesADefinitionFromTheLiveListing(t *testing.T) { p := newMigratedPlugin(t) definition := defined(t, "birthDate", "DATE") - if err := p.store.define(t.Context(), definition); err != nil { + if _, err := p.store.define(t.Context(), definition); err != nil { t.Fatalf("define() error = %v, want nil", err) } @@ -434,7 +469,7 @@ func TestStoreArchiveIsIdempotentOnALiveRow(t *testing.T) { p := newMigratedPlugin(t) definition := defined(t, "birthDate", "DATE") - if err := p.store.define(t.Context(), definition); err != nil { + if _, err := p.store.define(t.Context(), definition); err != nil { t.Fatalf("define() error = %v, want nil", err) } if err := p.store.archive(t.Context(), definition.ID); err != nil { @@ -468,7 +503,7 @@ func TestStoreListsDefinitionsByCreation(t *testing.T) { p := newMigratedPlugin(t) for _, name := range []string{"alpha", "beta", "gamma"} { - if err := p.store.define(t.Context(), defined(t, name, "TEXT")); err != nil { + if _, err := p.store.define(t.Context(), defined(t, name, "TEXT")); err != nil { t.Fatalf("define(%q) error = %v, want nil", name, err) } } diff --git a/plugins/fields/tenantscope_internal_test.go b/plugins/fields/tenantscope_internal_test.go index 1ddbfc23..e071a78e 100644 --- a/plugins/fields/tenantscope_internal_test.go +++ b/plugins/fields/tenantscope_internal_test.go @@ -29,7 +29,7 @@ func definedField(t *testing.T, p *Plugin, ctx context.Context, name string) { held := Definition{ ID: uuid.Must(uuid.NewV7()), Name: name, Label: name, Kind: "TEXT", CreatedAt: time.Now(), } - if err := p.store.define(ctx, held); err != nil { + if _, err := p.store.define(ctx, held); err != nil { t.Fatalf("defining %s: %v", name, err) } } @@ -61,7 +61,7 @@ func TestArchivingStaysInsideItsTenant(t *testing.T) { ID: uuid.Must(uuid.NewV7()), Name: "birthday", Label: "Birthday", Kind: "TEXT", CreatedAt: time.Now(), } - if err := p.store.define(acme, held); err != nil { + if _, err := p.store.define(acme, held); err != nil { t.Fatalf("define() error = %v, want nil", err) } diff --git a/plugins/fields/values_internal_test.go b/plugins/fields/values_internal_test.go index 3ff3c501..66ad7cc8 100644 --- a/plugins/fields/values_internal_test.go +++ b/plugins/fields/values_internal_test.go @@ -61,7 +61,7 @@ func TestWriteContactFieldsReportsAStoreFailure(t *testing.T) { t.Parallel() p := newMigratedPlugin(t) - if err := p.store.define(t.Context(), defined(t, "birthDate", "DATE")); err != nil { + if _, err := p.store.define(t.Context(), defined(t, "birthDate", "DATE")); err != nil { t.Fatalf("define() error = %v, want nil", err) } From 008d0ecf7faaca512003de2b4c80a0a187ab02ee Mon Sep 17 00:00:00 2001 From: SirLouen Date: Mon, 28 Sep 2026 09:42:46 +0200 Subject: [PATCH 06/25] test(features): run the scenario reviving an archived field --- test/features/features/fields-catalog.feature | 1 - 1 file changed, 1 deletion(-) diff --git a/test/features/features/fields-catalog.feature b/test/features/features/fields-catalog.feature index 2076a5bf..fedbf089 100644 --- a/test/features/features/fields-catalog.feature +++ b/test/features/features/fields-catalog.feature @@ -66,7 +66,6 @@ Feature: An operator shapes the field catalogue Then the catalogue does not list "birthDate" And the catalogue lists "birthDate" among archived definitions - @wip Scenario: Defining an archived field again answers the field it brings back Given the field "birthDate" labelled "Birth date" of kind DATE is defined And the operator archives the field "birthDate" From a652f9b745fedbfb673cdcc0441bd759f27bdeef Mon Sep 17 00:00:00 2001 From: SirLouen Date: Mon, 28 Sep 2026 09:46:11 +0200 Subject: [PATCH 07/25] feat(fields): answer the names a field cannot take --- plugins/fields/graph/schema.graphqls | 1 + plugins/fields/graphql.go | 9 +++++++++ plugins/fields/graphql_internal_test.go | 14 ++++++++++++++ 3 files changed, 24 insertions(+) diff --git a/plugins/fields/graph/schema.graphqls b/plugins/fields/graph/schema.graphqls index f40cd8b0..a87c6e1a 100644 --- a/plugins/fields/graph/schema.graphqls +++ b/plugins/fields/graph/schema.graphqls @@ -35,6 +35,7 @@ enum FieldKind { extend type Query { fields(includeArchived: Boolean): [FieldDefinition!]! @scope(area: "fields", write: false) + reservedFieldNames: [String!]! @scope(area: "fields", write: false) } extend type Mutation { diff --git a/plugins/fields/graphql.go b/plugins/fields/graphql.go index ae899e66..f8576794 100644 --- a/plugins/fields/graphql.go +++ b/plugins/fields/graphql.go @@ -5,6 +5,8 @@ package fields import ( "context" "errors" + "maps" + "slices" "github.com/google/uuid" @@ -28,6 +30,13 @@ func (p *Plugin) QueryResolvers() QueryResolvers { return QueryResolvers{plugin: p} } +// ReservedFieldNames lists the names a field cannot take, sorted. +func (q QueryResolvers) ReservedFieldNames(_ context.Context) ([]string, error) { + names := slices.AppendSeq(slices.Collect(maps.Keys(reservedNames)), maps.Keys(inheritedNames)) + slices.Sort(names) + return names, nil +} + // MutationResolvers serves the plugin's Mutation fields. type MutationResolvers struct { plugin *Plugin diff --git a/plugins/fields/graphql_internal_test.go b/plugins/fields/graphql_internal_test.go index 9b795201..024c6c3e 100644 --- a/plugins/fields/graphql_internal_test.go +++ b/plugins/fields/graphql_internal_test.go @@ -16,6 +16,20 @@ import ( "github.com/gopherium/alphone/sdk" ) +func TestReservedFieldNamesListsEveryReservedAndInheritedName(t *testing.T) { + t.Parallel() + + names, err := (QueryResolvers{}).ReservedFieldNames(t.Context()) + + want := []string{ + "constructor", "createdAt", "field", "hasOwnProperty", "id", "identities", "isPrototypeOf", + "name", "propertyIsEnumerable", "tasks", "toLocaleString", "toString", "valueOf", "whatsAppConversations", + } + if err != nil || !reflect.DeepEqual(names, want) { + t.Errorf("ReservedFieldNames() = %v, %v, want %v", names, err, want) + } +} + func TestDefineFieldNamesTheReasonItRefuses(t *testing.T) { t.Parallel() From 4d78b032f9a7595371a14aea32804e730bb11b0c Mon Sep 17 00:00:00 2001 From: SirLouen Date: Mon, 28 Sep 2026 09:46:11 +0200 Subject: [PATCH 08/25] chore(graph): regenerate the schema with the reserved field names --- graph/generated.go | 54 ++++++++++++++++++++++++++++++++++++++++++++ graph/schema.graphql | 1 + 2 files changed, 55 insertions(+) diff --git a/graph/generated.go b/graph/generated.go index d7a1546a..aee3de2f 100644 --- a/graph/generated.go +++ b/graph/generated.go @@ -228,6 +228,7 @@ type ComplexityRoot struct { Imports func(childComplexity int) int Locale func(childComplexity int) int Me func(childComplexity int) int + ReservedFieldNames func(childComplexity int) int SupportedLocales func(childComplexity int) int Task func(childComplexity int, id uuid.UUID) int Tasks func(childComplexity int, date *time.Time, dueBefore *time.Time, contactID *uuid.UUID, status *string, first *int, after *string) int @@ -385,6 +386,7 @@ type QueryResolver interface { APITokens(ctx context.Context) ([]*model.APIToken, error) Webhooks(ctx context.Context) ([]*model.Webhook, error) Fields(ctx context.Context, includeArchived *bool) ([]*model.FieldDefinition, error) + ReservedFieldNames(ctx context.Context) ([]string, error) Imports(ctx context.Context) ([]*model.ImportJob, error) ImportJob(ctx context.Context, id uuid.UUID) (*model.ImportJob, error) ImportFields(ctx context.Context) ([]*model.ImportField, error) @@ -1338,6 +1340,12 @@ func (e *executableSchema) Complexity(ctx context.Context, typeName, field strin } return e.ComplexityRoot.Query.Me(childComplexity), true + case "Query.reservedFieldNames": + if e.ComplexityRoot.Query.ReservedFieldNames == nil { + break + } + + return e.ComplexityRoot.Query.ReservedFieldNames(childComplexity), true case "Query.supportedLocales": if e.ComplexityRoot.Query.SupportedLocales == nil { break @@ -1916,6 +1924,7 @@ enum FieldKind { extend type Query { fields(includeArchived: Boolean): [FieldDefinition!]! @scope(area: "fields", write: false) + reservedFieldNames: [String!]! @scope(area: "fields", write: false) } extend type Mutation { @@ -7343,6 +7352,29 @@ func (ec *executionContext) fieldContext_Query_fields(ctx context.Context, field return fc, nil } +func (ec *executionContext) _Query_reservedFieldNames(ctx context.Context, field graphql.CollectedField) (ret graphql.Marshaler) { + return graphql.ResolveField( + ctx, + ec.OperationContext, + field, + func(ctx context.Context, field graphql.CollectedField) (*graphql.FieldContext, error) { + return ec.fieldContext_Query_reservedFieldNames(ctx, field) + }, + func(ctx context.Context) (any, error) { + return ec.Resolvers.Query().ReservedFieldNames(ctx) + }, + nil, + func(ctx context.Context, selections ast.SelectionSet, v []string) graphql.Marshaler { + return ec.marshalNString2ᚕstringᚄ(ctx, selections, v) + }, + true, + true, + ) +} +func (ec *executionContext) fieldContext_Query_reservedFieldNames(_ context.Context, field graphql.CollectedField) (fc *graphql.FieldContext, err error) { + return graphql.NewScalarFieldContext("Query", field, true, true, errors.New("field of type String does not have child fields")) +} + func (ec *executionContext) _Query_imports(ctx context.Context, field graphql.CollectedField) (ret graphql.Marshaler) { return graphql.ResolveField( ctx, @@ -12101,6 +12133,28 @@ func (ec *executionContext) _Query(ctx context.Context, sel ast.SelectionSet) gr func(ctx context.Context) graphql.Marshaler { return innerFunc(ctx, out) }) } + out.Concurrently(i, func(ctx context.Context) graphql.Marshaler { return rrm(innerCtx) }) + case "reservedFieldNames": + field := field + + innerFunc := func(ctx context.Context, fs *graphql.FieldSet) (res graphql.Marshaler) { + defer func() { + if r := recover(); r != nil { + ec.Error(ctx, ec.Recover(ctx, r)) + } + }() + res = ec._Query_reservedFieldNames(ctx, field) + if res == graphql.Null { + atomic.AddUint32(&fs.Invalids, 1) + } + return res + } + + rrm := func(ctx context.Context) graphql.Marshaler { + return ec.OperationContext.RootResolverMiddleware(ctx, + func(ctx context.Context) graphql.Marshaler { return innerFunc(ctx, out) }) + } + out.Concurrently(i, func(ctx context.Context) graphql.Marshaler { return rrm(innerCtx) }) case "imports": field := field diff --git a/graph/schema.graphql b/graph/schema.graphql index 61f14f61..b6708771 100644 --- a/graph/schema.graphql +++ b/graph/schema.graphql @@ -199,6 +199,7 @@ type Query { apiTokens: [ApiToken!]! @scope(area: "tokens", write: false) webhooks: [Webhook!]! @scope(area: "webhooks", write: false) fields(includeArchived: Boolean): [FieldDefinition!]! @scope(area: "fields", write: false) + reservedFieldNames: [String!]! @scope(area: "fields", write: false) imports: [ImportJob!]! @scope(area: "imports", write: false) importJob(id: UUID!): ImportJob @scope(area: "imports", write: false) importFields: [ImportField!]! @scope(area: "imports", write: false) From 00352492a5d085534d06582a34d23643ee5e8930 Mon Sep 17 00:00:00 2001 From: SirLouen Date: Mon, 28 Sep 2026 09:46:12 +0200 Subject: [PATCH 09/25] test(features): run the reserved names scenario the graph now serves --- test/features/features/fields-catalog.feature | 1 - 1 file changed, 1 deletion(-) diff --git a/test/features/features/fields-catalog.feature b/test/features/features/fields-catalog.feature index fedbf089..7c762576 100644 --- a/test/features/features/fields-catalog.feature +++ b/test/features/features/fields-catalog.feature @@ -36,7 +36,6 @@ Feature: An operator shapes the field catalogue | toString | | valueOf | - @wip Scenario: The catalogue lists every name a field cannot take When the operator asks which names a field cannot take Then the names a field cannot take are: From bf92da5bbb790d3d7b9123895f33e3715a623171 Mon Sep 17 00:00:00 2001 From: SirLouen Date: Mon, 28 Sep 2026 09:49:58 +0200 Subject: [PATCH 10/25] test(fields): reserve every field the Contact type holds --- plugins/fields/graphql_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/plugins/fields/graphql_test.go b/plugins/fields/graphql_test.go index 5e077af3..0aa21266 100644 --- a/plugins/fields/graphql_test.go +++ b/plugins/fields/graphql_test.go @@ -5,6 +5,7 @@ package fields_test import ( "context" "net/http" + "slices" "strings" "testing" @@ -201,6 +202,24 @@ func TestGraphRefusesADuplicateName(t *testing.T) { } } +func TestGraphReservesEveryFieldTheContactTypeHolds(t *testing.T) { + t.Parallel() + + client := newFieldsClient(t) + var answer struct{ ReservedFieldNames []string } + client.MustPost(`{ reservedFieldNames }`, &answer) + + contact := graphres.ExecutableSchema(nil).Schema().Types["Contact"] + if contact == nil || len(contact.Fields) == 0 { + t.Fatal("the compiled schema holds no Contact fields") + } + for _, field := range contact.Fields { + if !strings.HasPrefix(field.Name, "__") && !slices.Contains(answer.ReservedFieldNames, field.Name) { + t.Errorf("reservedFieldNames lacks %q, a field the Contact type holds", field.Name) + } + } +} + func TestGraphAnswersTheIDOfARevivedField(t *testing.T) { t.Parallel() From 75a4802f38bb9ea0a4eafd11a4ac6b5a785132e8 Mon Sep 17 00:00:00 2001 From: SirLouen Date: Mon, 28 Sep 2026 10:04:09 +0200 Subject: [PATCH 11/25] test(e2e): let the Fields screen name new fields --- test/e2e/tests/fields-names.spec.ts | 42 ++++++++++++++++++++++++++ test/e2e/tests/fields-repeater.spec.ts | 2 +- test/e2e/tests/importer.spec.ts | 1 - 3 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 test/e2e/tests/fields-names.spec.ts diff --git a/test/e2e/tests/fields-names.spec.ts b/test/e2e/tests/fields-names.spec.ts new file mode 100644 index 00000000..3948933d --- /dev/null +++ b/test/e2e/tests/fields-names.spec.ts @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +import { expect, test } from '@playwright/test' +import type { Page } from '@playwright/test' + +/** + * Adds one Text field from the Fields screen and waits for its row. + * @param page - The page showing the Fields screen. + * @param label - The field's label. + */ +async function addTextField(page: Page, label: string) { + await page.getByLabel('Label', { exact: true }).fill(label) + await page.getByRole('combobox', { name: 'Kind' }).click() + const listbox = page.getByRole('listbox') + await listbox.getByRole('option', { name: 'Text', exact: true }).click() + await expect(listbox).toBeHidden() + await page.getByRole('button', { name: 'Add field' }).click() + await expect(fieldRow(page, label)).toBeVisible() +} + +/** + * Returns the row of the Fields list that holds a label. + * @param page - The page showing the Fields screen. + * @param label - The field's label. + * @returns The row. + */ +function fieldRow(page: Page, label: string) { + return page.getByRole('region', { name: 'Fields' }).getByRole('row').filter({ hasText: label }) +} + +test('names each new field from its label and numbers a name already taken', async ({ page }) => { + const stamp = Date.now() + + await page.goto('/') + await page.getByRole('link', { name: 'Fields' }).click() + await expect(page.getByRole('heading', { name: 'Fields', level: 1 })).toBeVisible() + await addTextField(page, `Visit ${stamp} notes`) + await addTextField(page, `Visit ${stamp}: notes`) + + await expect(fieldRow(page, `Visit ${stamp} notes`)).toContainText(`visit${stamp}Notes`) + await expect(fieldRow(page, `Visit ${stamp}: notes`)).toContainText(`visit${stamp}Notes2`) +}) diff --git a/test/e2e/tests/fields-repeater.spec.ts b/test/e2e/tests/fields-repeater.spec.ts index 6b437f36..2366aa76 100644 --- a/test/e2e/tests/fields-repeater.spec.ts +++ b/test/e2e/tests/fields-repeater.spec.ts @@ -40,7 +40,6 @@ test('defines a repeater and keeps a contact history one entry at a time', async await page.getByRole('link', { name: 'Fields' }).click() await expect(page.getByRole('heading', { name: 'Fields', level: 1 })).toBeVisible() await page.getByLabel('Label', { exact: true }).fill(label) - await page.getByLabel('Name', { exact: true }).fill(`history${stamp}`) await chooseKind(page, page.getByRole('combobox', { name: 'Kind' }), 'Repeater') await addSubField(page, 1, 'Date', 'Date') await addSubField(page, 2, 'Comment', 'Long text') @@ -48,6 +47,7 @@ test('defines a repeater and keeps a contact history one entry at a time', async const defined = page.getByRole('region', { name: 'Fields' }).getByRole('row').filter({ hasText: label }) await expect(defined).toContainText('Repeater') await expect(defined).toContainText('Date, Comment') + await expect(defined).toContainText(`history${stamp}`) await page.getByRole('link', { name: 'Contacts' }).click() await page.getByRole('link', { name: 'New contact' }).click() diff --git a/test/e2e/tests/importer.spec.ts b/test/e2e/tests/importer.spec.ts index 1736d0be..59aeb504 100644 --- a/test/e2e/tests/importer.spec.ts +++ b/test/e2e/tests/importer.spec.ts @@ -86,7 +86,6 @@ test('maps a spreadsheet column onto a field an operator defined', async ({ page await page.getByRole('link', { name: 'Fields' }).click() await expect(page.getByRole('heading', { name: 'Fields', level: 1 })).toBeVisible() await page.getByLabel('Label').fill(fieldLabel) - await page.getByLabel('Name').fill(field) await page.getByRole('combobox', { name: 'Kind' }).click() await page.getByRole('listbox').getByRole('option', { name: 'Date', exact: true }).click() await page.getByRole('button', { name: 'Add field' }).click() From db3db510eb525455008a56d22c46e8f0e02ed7c8 Mon Sep 17 00:00:00 2001 From: SirLouen Date: Mon, 28 Sep 2026 10:33:39 +0200 Subject: [PATCH 12/25] feat(fields): read archived fields and reserved names for the Fields screen --- plugins/fields/frontend/gql/gql.ts | 6 ++++ plugins/fields/frontend/gql/graphql.ts | 6 ++++ plugins/fields/frontend/operations.ts | 31 +++++++++++++++++++ .../fields/frontend/test/operations.test.ts | 8 ++++- 4 files changed, 50 insertions(+), 1 deletion(-) diff --git a/plugins/fields/frontend/gql/gql.ts b/plugins/fields/frontend/gql/gql.ts index a96c1adf..ad38b0a8 100644 --- a/plugins/fields/frontend/gql/gql.ts +++ b/plugins/fields/frontend/gql/gql.ts @@ -15,6 +15,7 @@ import type { TypedDocumentNode as DocumentNode } from '@graphql-typed-document- */ type Documents = { "\n\tquery Fields {\n\t\tfields {\n\t\t\tid\n\t\t\tname\n\t\t\tlabel\n\t\t\tkind\n\t\t\tsubFields {\n\t\t\t\tname\n\t\t\t\tlabel\n\t\t\t\tkind\n\t\t\t}\n\t\t}\n\t}\n": typeof types.FieldsDocument, + "\n\tquery FieldCatalogue {\n\t\tfields {\n\t\t\tid\n\t\t\tname\n\t\t\tlabel\n\t\t\tkind\n\t\t\tsubFields {\n\t\t\t\tname\n\t\t\t\tlabel\n\t\t\t\tkind\n\t\t\t}\n\t\t}\n\t\tevery: fields(includeArchived: true) {\n\t\t\tid\n\t\t\tname\n\t\t\tlabel\n\t\t\tkind\n\t\t\tsubFields {\n\t\t\t\tname\n\t\t\t\tlabel\n\t\t\t\tkind\n\t\t\t}\n\t\t}\n\t\treservedFieldNames\n\t}\n": typeof types.FieldCatalogueDocument, "\n\tmutation DefineField(\n\t\t$name: String!\n\t\t$label: String!\n\t\t$kind: FieldKind!\n\t\t$subFields: [FieldSubFieldInput!]\n\t) {\n\t\tdefineField(name: $name, label: $label, kind: $kind, subFields: $subFields) {\n\t\t\tid\n\t\t\tname\n\t\t\tlabel\n\t\t\tkind\n\t\t\tsubFields {\n\t\t\t\tname\n\t\t\t\tlabel\n\t\t\t\tkind\n\t\t\t}\n\t\t}\n\t}\n": typeof types.DefineFieldDocument, "\n\tmutation ArchiveField($id: UUID!) {\n\t\tarchiveField(id: $id)\n\t}\n": typeof types.ArchiveFieldDocument, "\n\tmutation WriteContactFields($contactId: UUID!, $values: JSON!) {\n\t\twriteContactFields(contactId: $contactId, values: $values)\n\t}\n": typeof types.WriteContactFieldsDocument, @@ -24,6 +25,7 @@ type Documents = { }; const documents: Documents = { "\n\tquery Fields {\n\t\tfields {\n\t\t\tid\n\t\t\tname\n\t\t\tlabel\n\t\t\tkind\n\t\t\tsubFields {\n\t\t\t\tname\n\t\t\t\tlabel\n\t\t\t\tkind\n\t\t\t}\n\t\t}\n\t}\n": types.FieldsDocument, + "\n\tquery FieldCatalogue {\n\t\tfields {\n\t\t\tid\n\t\t\tname\n\t\t\tlabel\n\t\t\tkind\n\t\t\tsubFields {\n\t\t\t\tname\n\t\t\t\tlabel\n\t\t\t\tkind\n\t\t\t}\n\t\t}\n\t\tevery: fields(includeArchived: true) {\n\t\t\tid\n\t\t\tname\n\t\t\tlabel\n\t\t\tkind\n\t\t\tsubFields {\n\t\t\t\tname\n\t\t\t\tlabel\n\t\t\t\tkind\n\t\t\t}\n\t\t}\n\t\treservedFieldNames\n\t}\n": types.FieldCatalogueDocument, "\n\tmutation DefineField(\n\t\t$name: String!\n\t\t$label: String!\n\t\t$kind: FieldKind!\n\t\t$subFields: [FieldSubFieldInput!]\n\t) {\n\t\tdefineField(name: $name, label: $label, kind: $kind, subFields: $subFields) {\n\t\t\tid\n\t\t\tname\n\t\t\tlabel\n\t\t\tkind\n\t\t\tsubFields {\n\t\t\t\tname\n\t\t\t\tlabel\n\t\t\t\tkind\n\t\t\t}\n\t\t}\n\t}\n": types.DefineFieldDocument, "\n\tmutation ArchiveField($id: UUID!) {\n\t\tarchiveField(id: $id)\n\t}\n": types.ArchiveFieldDocument, "\n\tmutation WriteContactFields($contactId: UUID!, $values: JSON!) {\n\t\twriteContactFields(contactId: $contactId, values: $values)\n\t}\n": types.WriteContactFieldsDocument, @@ -50,6 +52,10 @@ export function graphql(source: string): unknown; * The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. */ export function graphql(source: "\n\tquery Fields {\n\t\tfields {\n\t\t\tid\n\t\t\tname\n\t\t\tlabel\n\t\t\tkind\n\t\t\tsubFields {\n\t\t\t\tname\n\t\t\t\tlabel\n\t\t\t\tkind\n\t\t\t}\n\t\t}\n\t}\n"): (typeof documents)["\n\tquery Fields {\n\t\tfields {\n\t\t\tid\n\t\t\tname\n\t\t\tlabel\n\t\t\tkind\n\t\t\tsubFields {\n\t\t\t\tname\n\t\t\t\tlabel\n\t\t\t\tkind\n\t\t\t}\n\t\t}\n\t}\n"]; +/** + * The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. + */ +export function graphql(source: "\n\tquery FieldCatalogue {\n\t\tfields {\n\t\t\tid\n\t\t\tname\n\t\t\tlabel\n\t\t\tkind\n\t\t\tsubFields {\n\t\t\t\tname\n\t\t\t\tlabel\n\t\t\t\tkind\n\t\t\t}\n\t\t}\n\t\tevery: fields(includeArchived: true) {\n\t\t\tid\n\t\t\tname\n\t\t\tlabel\n\t\t\tkind\n\t\t\tsubFields {\n\t\t\t\tname\n\t\t\t\tlabel\n\t\t\t\tkind\n\t\t\t}\n\t\t}\n\t\treservedFieldNames\n\t}\n"): (typeof documents)["\n\tquery FieldCatalogue {\n\t\tfields {\n\t\t\tid\n\t\t\tname\n\t\t\tlabel\n\t\t\tkind\n\t\t\tsubFields {\n\t\t\t\tname\n\t\t\t\tlabel\n\t\t\t\tkind\n\t\t\t}\n\t\t}\n\t\tevery: fields(includeArchived: true) {\n\t\t\tid\n\t\t\tname\n\t\t\tlabel\n\t\t\tkind\n\t\t\tsubFields {\n\t\t\t\tname\n\t\t\t\tlabel\n\t\t\t\tkind\n\t\t\t}\n\t\t}\n\t\treservedFieldNames\n\t}\n"]; /** * The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. */ diff --git a/plugins/fields/frontend/gql/graphql.ts b/plugins/fields/frontend/gql/graphql.ts index df6d69d6..09cdacc8 100644 --- a/plugins/fields/frontend/gql/graphql.ts +++ b/plugins/fields/frontend/gql/graphql.ts @@ -24,6 +24,11 @@ export type FieldsQueryVariables = Exact<{ [key: string]: never; }>; export type FieldsQuery = { fields: Array<{ id: string, name: string, label: string, kind: FieldKind, subFields: Array<{ name: string, label: string, kind: FieldKind }> }> }; +export type FieldCatalogueQueryVariables = Exact<{ [key: string]: never; }>; + + +export type FieldCatalogueQuery = { reservedFieldNames: Array, fields: Array<{ id: string, name: string, label: string, kind: FieldKind, subFields: Array<{ name: string, label: string, kind: FieldKind }> }>, every: Array<{ id: string, name: string, label: string, kind: FieldKind, subFields: Array<{ name: string, label: string, kind: FieldKind }> }> }; + export type DefineFieldMutationVariables = Exact<{ name: string; label: string; @@ -79,6 +84,7 @@ export type DeleteContactFieldEntryMutation = { deleteContactFieldEntry: boolean export const FieldsDocument = {"kind":"Document","definitions":[{"kind":"OperationDefinition","operation":"query","name":{"kind":"Name","value":"Fields"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"fields"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"id"}},{"kind":"Field","name":{"kind":"Name","value":"name"}},{"kind":"Field","name":{"kind":"Name","value":"label"}},{"kind":"Field","name":{"kind":"Name","value":"kind"}},{"kind":"Field","name":{"kind":"Name","value":"subFields"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"name"}},{"kind":"Field","name":{"kind":"Name","value":"label"}},{"kind":"Field","name":{"kind":"Name","value":"kind"}}]}}]}}]}}]} as unknown as DocumentNode; +export const FieldCatalogueDocument = {"kind":"Document","definitions":[{"kind":"OperationDefinition","operation":"query","name":{"kind":"Name","value":"FieldCatalogue"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"fields"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"id"}},{"kind":"Field","name":{"kind":"Name","value":"name"}},{"kind":"Field","name":{"kind":"Name","value":"label"}},{"kind":"Field","name":{"kind":"Name","value":"kind"}},{"kind":"Field","name":{"kind":"Name","value":"subFields"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"name"}},{"kind":"Field","name":{"kind":"Name","value":"label"}},{"kind":"Field","name":{"kind":"Name","value":"kind"}}]}}]}},{"kind":"Field","alias":{"kind":"Name","value":"every"},"name":{"kind":"Name","value":"fields"},"arguments":[{"kind":"Argument","name":{"kind":"Name","value":"includeArchived"},"value":{"kind":"BooleanValue","value":true}}],"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"id"}},{"kind":"Field","name":{"kind":"Name","value":"name"}},{"kind":"Field","name":{"kind":"Name","value":"label"}},{"kind":"Field","name":{"kind":"Name","value":"kind"}},{"kind":"Field","name":{"kind":"Name","value":"subFields"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"name"}},{"kind":"Field","name":{"kind":"Name","value":"label"}},{"kind":"Field","name":{"kind":"Name","value":"kind"}}]}}]}},{"kind":"Field","name":{"kind":"Name","value":"reservedFieldNames"}}]}}]} as unknown as DocumentNode; export const DefineFieldDocument = {"kind":"Document","definitions":[{"kind":"OperationDefinition","operation":"mutation","name":{"kind":"Name","value":"DefineField"},"variableDefinitions":[{"kind":"VariableDefinition","variable":{"kind":"Variable","name":{"kind":"Name","value":"name"}},"type":{"kind":"NonNullType","type":{"kind":"NamedType","name":{"kind":"Name","value":"String"}}}},{"kind":"VariableDefinition","variable":{"kind":"Variable","name":{"kind":"Name","value":"label"}},"type":{"kind":"NonNullType","type":{"kind":"NamedType","name":{"kind":"Name","value":"String"}}}},{"kind":"VariableDefinition","variable":{"kind":"Variable","name":{"kind":"Name","value":"kind"}},"type":{"kind":"NonNullType","type":{"kind":"NamedType","name":{"kind":"Name","value":"FieldKind"}}}},{"kind":"VariableDefinition","variable":{"kind":"Variable","name":{"kind":"Name","value":"subFields"}},"type":{"kind":"ListType","type":{"kind":"NonNullType","type":{"kind":"NamedType","name":{"kind":"Name","value":"FieldSubFieldInput"}}}}}],"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"defineField"},"arguments":[{"kind":"Argument","name":{"kind":"Name","value":"name"},"value":{"kind":"Variable","name":{"kind":"Name","value":"name"}}},{"kind":"Argument","name":{"kind":"Name","value":"label"},"value":{"kind":"Variable","name":{"kind":"Name","value":"label"}}},{"kind":"Argument","name":{"kind":"Name","value":"kind"},"value":{"kind":"Variable","name":{"kind":"Name","value":"kind"}}},{"kind":"Argument","name":{"kind":"Name","value":"subFields"},"value":{"kind":"Variable","name":{"kind":"Name","value":"subFields"}}}],"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"id"}},{"kind":"Field","name":{"kind":"Name","value":"name"}},{"kind":"Field","name":{"kind":"Name","value":"label"}},{"kind":"Field","name":{"kind":"Name","value":"kind"}},{"kind":"Field","name":{"kind":"Name","value":"subFields"},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"name"}},{"kind":"Field","name":{"kind":"Name","value":"label"}},{"kind":"Field","name":{"kind":"Name","value":"kind"}}]}}]}}]}}]} as unknown as DocumentNode; export const ArchiveFieldDocument = {"kind":"Document","definitions":[{"kind":"OperationDefinition","operation":"mutation","name":{"kind":"Name","value":"ArchiveField"},"variableDefinitions":[{"kind":"VariableDefinition","variable":{"kind":"Variable","name":{"kind":"Name","value":"id"}},"type":{"kind":"NonNullType","type":{"kind":"NamedType","name":{"kind":"Name","value":"UUID"}}}}],"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"archiveField"},"arguments":[{"kind":"Argument","name":{"kind":"Name","value":"id"},"value":{"kind":"Variable","name":{"kind":"Name","value":"id"}}}]}]}}]} as unknown as DocumentNode; export const WriteContactFieldsDocument = {"kind":"Document","definitions":[{"kind":"OperationDefinition","operation":"mutation","name":{"kind":"Name","value":"WriteContactFields"},"variableDefinitions":[{"kind":"VariableDefinition","variable":{"kind":"Variable","name":{"kind":"Name","value":"contactId"}},"type":{"kind":"NonNullType","type":{"kind":"NamedType","name":{"kind":"Name","value":"UUID"}}}},{"kind":"VariableDefinition","variable":{"kind":"Variable","name":{"kind":"Name","value":"values"}},"type":{"kind":"NonNullType","type":{"kind":"NamedType","name":{"kind":"Name","value":"JSON"}}}}],"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"writeContactFields"},"arguments":[{"kind":"Argument","name":{"kind":"Name","value":"contactId"},"value":{"kind":"Variable","name":{"kind":"Name","value":"contactId"}}},{"kind":"Argument","name":{"kind":"Name","value":"values"},"value":{"kind":"Variable","name":{"kind":"Name","value":"values"}}}]}]}}]} as unknown as DocumentNode; diff --git a/plugins/fields/frontend/operations.ts b/plugins/fields/frontend/operations.ts index a72b3ebb..c1aabd54 100644 --- a/plugins/fields/frontend/operations.ts +++ b/plugins/fields/frontend/operations.ts @@ -21,6 +21,37 @@ export const fieldsQuery = graphql(` } `) +/** fieldCatalogueOperation names the query the Fields screen reads its catalogue with. */ +export const fieldCatalogueOperation = 'FieldCatalogue' + +export const fieldCatalogueQuery = graphql(` + query FieldCatalogue { + fields { + id + name + label + kind + subFields { + name + label + kind + } + } + every: fields(includeArchived: true) { + id + name + label + kind + subFields { + name + label + kind + } + } + reservedFieldNames + } +`) + export const defineFieldMutation = graphql(` mutation DefineField( $name: String! diff --git a/plugins/fields/frontend/test/operations.test.ts b/plugins/fields/frontend/test/operations.test.ts index b3b7c7a1..aabebde7 100644 --- a/plugins/fields/frontend/test/operations.test.ts +++ b/plugins/fields/frontend/test/operations.test.ts @@ -2,10 +2,16 @@ import { expect, test } from 'vitest' -import { catalogueOperation, fieldsQuery } from '../operations' +import { catalogueOperation, fieldCatalogueOperation, fieldCatalogueQuery, fieldsQuery } from '../operations' test('names the catalogue query by the operation it declares', () => { const [query] = fieldsQuery.definitions expect(query.kind === 'OperationDefinition' ? query.name?.value : undefined).toBe(catalogueOperation) }) + +test('names the Fields screen query by the operation it declares', () => { + const [query] = fieldCatalogueQuery.definitions + + expect(query.kind === 'OperationDefinition' ? query.name?.value : undefined).toBe(fieldCatalogueOperation) +}) From d0f6b5db20da3c29e138fff213fdc7792c3b2016 Mon Sep 17 00:00:00 2001 From: SirLouen Date: Mon, 28 Sep 2026 10:33:40 +0200 Subject: [PATCH 13/25] feat(fields): make a new field's name from its label --- plugins/fields/frontend/FieldsScreen.tsx | 64 ++++-- plugins/fields/frontend/test/fields.test.tsx | 230 +++++++++++++------ 2 files changed, 210 insertions(+), 84 deletions(-) diff --git a/plugins/fields/frontend/FieldsScreen.tsx b/plugins/fields/frontend/FieldsScreen.tsx index 49aabce1..d54527f4 100644 --- a/plugins/fields/frontend/FieldsScreen.tsx +++ b/plugins/fields/frontend/FieldsScreen.tsx @@ -25,9 +25,14 @@ import { import { useState } from 'react' import { fieldsIcon } from './icon' -import type { FieldKind } from './gql/graphql' +import type { FieldCatalogueQuery, FieldKind } from './gql/graphql' import { ENTRY_ID_KEY, kindItems, kindOf, subKindItems } from './kind' -import { archiveFieldMutation, catalogueOperation, defineFieldMutation, fieldsQuery } from './operations' +import { + archiveFieldMutation, + defineFieldMutation, + fieldCatalogueOperation, + fieldCatalogueQuery, +} from './operations' /** FieldRow is one catalogue entry as the screen renders it. */ interface FieldRow { @@ -51,12 +56,18 @@ interface DraftSubField { kind: FieldKind } +/** KnownNames are every stored field and the names the server refuses, which a new field's name steps past. */ +interface KnownNames { + every: FieldRow[] + reserved: string[] +} + /** * Renders the catalogue of contact fields an operator defines. * @returns The fields screen. */ export function FieldsScreen() { - const [catalogue] = useGraphQuery({ query: fieldsQuery }) + const [catalogue] = useGraphQuery({ query: fieldCatalogueQuery, requestPolicy: 'cache-and-network' }) const reload = useCatalogueRefresh() if (catalogue.fetching && !catalogue.data) { @@ -73,22 +84,35 @@ export function FieldsScreen() { ) } - const fields = (catalogue.data?.fields ?? []) as FieldRow[] + const rows = catalogueRows(catalogue.data) return ( - + }> {__('Add a field', 'alphone-fields')} - + ) } +/** + * Returns the live fields, every stored field and the reserved names a catalogue answer holds. + * @param data - The catalogue answer, absent while none has arrived. + * @returns The rows, each list empty when the answer lacks it. + */ +function catalogueRows(data: FieldCatalogueQuery | undefined): KnownNames & { live: FieldRow[] } { + return { + live: (data?.fields ?? []) as FieldRow[], + every: (data?.every ?? []) as FieldRow[], + reserved: data?.reservedFieldNames ?? [], + } +} + /** * Returns the refresh rerunning the catalogue query against the network. * @returns The refresh callback. @@ -96,7 +120,7 @@ export function FieldsScreen() { function useCatalogueRefresh() { const graph = useGraph() return () => { - graph.refetch([catalogueOperation]) + graph.refetch([fieldCatalogueOperation]) } } @@ -225,13 +249,22 @@ function namedSubFields(drafts: DraftSubField[]) { } /** - * Renders the form defining one new field. - * @param props - The reload run after a definition lands. + * Returns the name a new field takes from its label. + * @param label - The label of the new field. + * @param known - Every stored field and the names the server refuses. + * @returns The name, numbered past every name already taken. + */ +function fieldName(label: string, known: KnownNames) { + return keyFromLabel(label, { style: 'camel', taken: [...known.reserved, ...known.every.map((row) => row.name)] }) +} + +/** + * Renders the form defining one new field, naming it from its label. + * @param props - The names a new field steps past and the reload run after a definition lands. * @returns The add field form. */ -function AddFieldForm({ onAdded }: { onAdded: () => void }) { +function AddFieldForm({ known, onAdded }: { known: KnownNames; onAdded: () => void }) { const [label, setLabel] = useState('') - const [name, setName] = useState('') const [kind, setKind] = useState('TEXT') const [subFields, setSubFields] = useState([]) const kinds = kindItems() @@ -244,10 +277,9 @@ function AddFieldForm({ onAdded }: { onAdded: () => void }) { onSubmit={(event) => { event.preventDefault() const sent = repeater ? namedSubFields(subFields) : undefined - void define({ name, label, kind, subFields: sent }).then((result) => { + void define({ name: fieldName(label, known), label, kind, subFields: sent }).then((result) => { if (!result.error) { setLabel('') - setName('') setSubFields([]) onAdded() } @@ -265,12 +297,6 @@ function AddFieldForm({ onAdded }: { onAdded: () => void }) { value={label} onChange={(event) => setLabel(event.target.value)} /> - setName(event.target.value)} - /> @@ -35,14 +39,37 @@ function renderScreen() { ) } -function serveFields(fields: unknown[]) { +function serveFieldCatalogue(live: unknown[], archived: unknown[] = [], reserved: string[] = RESERVED) { + server.use( + graphql.query('FieldCatalogue', () => + HttpResponse.json({ data: { fields: live, every: [...live, ...archived], reservedFieldNames: reserved } }), + ), + ) +} + +function captureDefine(answer: unknown = birthDate) { + const defined = vi.fn() server.use( - graphql.query('Fields', () => HttpResponse.json({ data: { fields } })), + graphql.mutation('DefineField', async ({ variables }) => { + defined(variables) + return HttpResponse.json({ data: { defineField: answer } }) + }), ) + return defined +} + +async function defineLabelled(label: string) { + await userEvent.type(await screen.findByLabelText('Label'), label) + await userEvent.click(screen.getByRole('button', { name: 'Add field' })) +} + +async function sentName(defined: ReturnType) { + await waitFor(() => expect(defined).toHaveBeenCalledTimes(1)) + return defined.mock.calls[0][0].name } test('the catalogue lists every defined field in a table', async () => { - serveFields([birthDate]) + serveFieldCatalogue([birthDate]) renderScreen() @@ -58,7 +85,7 @@ test('the catalogue lists every defined field in a table', async () => { }) test('an empty catalogue invites the first field', async () => { - serveFields([]) + serveFieldCatalogue([]) renderScreen() @@ -67,7 +94,7 @@ test('an empty catalogue invites the first field', async () => { test('a failed read is reported', async () => { server.use( - graphql.query('Fields', () => + graphql.query('FieldCatalogue', () => HttpResponse.json({ errors: [{ message: 'boom' }] }), ), ) @@ -77,21 +104,22 @@ test('a failed read is reported', async () => { expect(await screen.findByRole('alert')).toBeInTheDocument() }) -test('defining a field sends its name, label and kind', async () => { - serveFields([]) - const defined = vi.fn() - server.use( - graphql.mutation('DefineField', async ({ variables }) => { - defined(variables) - return HttpResponse.json({ data: { defineField: birthDate } }) - }), - ) +test('the add form asks for no name', async () => { + serveFieldCatalogue([]) renderScreen() await screen.findByText(/No fields yet/i) - await userEvent.type(await screen.findByLabelText('Label'), 'Birth date') - await userEvent.type(screen.getByLabelText('Name'), 'birthDate') - await userEvent.click(screen.getByRole('button', { name: 'Add field' })) + + expect(screen.getByLabelText('Label')).toBeInTheDocument() + expect(screen.queryByLabelText('Name')).not.toBeInTheDocument() +}) + +test('defining a field sends the name its label makes', async () => { + serveFieldCatalogue([]) + const defined = captureDefine() + + renderScreen() + await defineLabelled('Birth date') await waitFor(() => expect(defined).toHaveBeenCalledWith({ @@ -102,15 +130,101 @@ test('defining a field sends its name, label and kind', async () => { ) }) +test('numbers a name a live field holds', async () => { + serveFieldCatalogue([{ ...birthDate, label: 'Date of birth' }]) + const defined = captureDefine() + + renderScreen() + await defineLabelled('Birth date') + + expect(await sentName(defined)).toBe('birthDate2') +}) + +test('numbers a name an archived field holds', async () => { + serveFieldCatalogue([], [birthDate]) + const defined = captureDefine() + + renderScreen() + await defineLabelled('Birth date') + + expect(await sentName(defined)).toBe('birthDate2') +}) + +test('steps past a name the contact already has', async () => { + serveFieldCatalogue([]) + const defined = captureDefine() + + renderScreen() + await defineLabelled('Name') + + expect(await sentName(defined)).toBe('name2') +}) + +test('steps past a name every object has', async () => { + serveFieldCatalogue([]) + const defined = captureDefine() + + renderScreen() + await defineLabelled('Constructor') + + expect(await sentName(defined)).toBe('constructor2') +}) + +test('steps past the names the server lists', async () => { + serveFieldCatalogue([], [], ['birthDate']) + const defined = captureDefine() + + renderScreen() + await defineLabelled('Birth date') + + expect(await sentName(defined)).toBe('birthDate2') +}) + +test('names a label with no letters or digits field2', async () => { + serveFieldCatalogue([]) + const defined = captureDefine() + + renderScreen() + await defineLabelled('???') + + expect(await sentName(defined)).toBe('field2') +}) + +test('numbers a label ending in a digit straight after it', async () => { + serveFieldCatalogue([{ ...birthDate, name: 'address2', label: 'Second address', kind: 'TEXT' }]) + const defined = captureDefine() + + renderScreen() + await defineLabelled('Address 2') + + expect(await sentName(defined)).toBe('address22') +}) + +test('a second visit reads the catalogue from the server again', async () => { + let reads = 0 + server.use( + graphql.query('FieldCatalogue', () => { + reads += 1 + return HttpResponse.json({ data: { fields: [], every: [], reservedFieldNames: RESERVED } }) + }), + ) + const { graph } = fakeGraphClient() + + const first = renderScreen(graph) + await screen.findByText(/No fields yet/i) + first.unmount() + renderScreen(graph) + + await waitFor(() => expect(reads).toBe(2)) +}) + async function submitField() { await screen.findByText(/No fields yet/i) - await userEvent.type(await screen.findByLabelText('Label'), 'Birth date') - await userEvent.type(screen.getByLabelText('Name'), 'birthDate') - await userEvent.click(screen.getByRole('button', { name: 'Add field' })) + await defineLabelled('Birth date') } test('a taken name is reported word for word', async () => { - serveFields([]) + serveFieldCatalogue([]) server.use( graphql.mutation('DefineField', () => HttpResponse.json({ @@ -131,19 +245,12 @@ test('a taken name is reported word for word', async () => { }) test('the chosen kind is sent with the definition', async () => { - serveFields([]) - const defined = vi.fn() - server.use( - graphql.mutation('DefineField', async ({ variables }) => { - defined(variables) - return HttpResponse.json({ data: { defineField: birthDate } }) - }), - ) + serveFieldCatalogue([]) + const defined = captureDefine() renderScreen() await screen.findByText(/No fields yet/i) await userEvent.type(await screen.findByLabelText('Label'), 'Birth date') - await userEvent.type(screen.getByLabelText('Name'), 'birthDate') await userEvent.click(screen.getByRole('combobox', { name: 'Kind' })) await userEvent.click(await screen.findByRole('option', { name: 'Date' })) await userEvent.click(screen.getByRole('button', { name: 'Add field' })) @@ -158,7 +265,7 @@ test('the chosen kind is sent with the definition', async () => { }) test('marks the kind the reader chose as the selected option', async () => { - serveFields([]) + serveFieldCatalogue([]) renderScreen() await screen.findByText(/No fields yet/i) @@ -173,7 +280,9 @@ test('marks the kind the reader chose as the selected option', async () => { test('a defined field appears in the catalogue without a reload', async () => { let served: unknown[] = [] server.use( - graphql.query('Fields', () => HttpResponse.json({ data: { fields: served } })), + graphql.query('FieldCatalogue', () => + HttpResponse.json({ data: { fields: served, every: served, reservedFieldNames: RESERVED } }), + ), graphql.mutation('DefineField', () => { served = [birthDate] return HttpResponse.json({ data: { defineField: birthDate } }) @@ -188,15 +297,18 @@ test('a defined field appears in the catalogue without a reload', async () => { }) test('an answer carrying no catalogue reads as empty', async () => { - server.use(graphql.query('Fields', () => HttpResponse.json({ data: {} }))) + server.use(graphql.query('FieldCatalogue', () => HttpResponse.json({ data: {} }))) + const defined = captureDefine() renderScreen() - expect(await screen.findByText(/No fields yet/i)).toBeInTheDocument() + await defineLabelled('Birth date') + + expect(await sentName(defined)).toBe('birthDate') }) test('a failed archive is reported', async () => { - serveFields([birthDate]) + serveFieldCatalogue([birthDate]) server.use( graphql.mutation('ArchiveField', () => HttpResponse.json({ @@ -216,7 +328,7 @@ test('a failed archive is reported', async () => { }) test('a validation error is reported word for word', async () => { - serveFields([]) + serveFieldCatalogue([]) server.use( graphql.mutation('DefineField', () => HttpResponse.json({ @@ -248,21 +360,9 @@ const history = { ], } -function captureDefine() { - const defined = vi.fn() - server.use( - graphql.mutation('DefineField', async ({ variables }) => { - defined(variables) - return HttpResponse.json({ data: { defineField: history } }) - }), - ) - return defined -} - async function startRepeater() { await screen.findByText(/No fields yet/i) await userEvent.type(await screen.findByLabelText('Label'), 'History') - await userEvent.type(screen.getByLabelText('Name'), 'history') await userEvent.click(screen.getByRole('combobox', { name: 'Kind' })) await userEvent.click(await screen.findByRole('option', { name: 'Repeater' })) } @@ -277,7 +377,7 @@ async function addSubField(label: string, kind: string) { } test('the catalogue shows a repeater beside the labels of its sub fields', async () => { - serveFields([history]) + serveFieldCatalogue([history]) renderScreen() @@ -288,7 +388,7 @@ test('the catalogue shows a repeater beside the labels of its sub fields', async }) test('a field holding no sub fields shows its kind alone', async () => { - serveFields([birthDate]) + serveFieldCatalogue([birthDate]) renderScreen() @@ -298,7 +398,7 @@ test('a field holding no sub fields shows its kind alone', async () => { }) test('only the repeater kind asks for sub fields', async () => { - serveFields([]) + serveFieldCatalogue([]) renderScreen() await screen.findByText(/No fields yet/i) @@ -311,8 +411,8 @@ test('only the repeater kind asks for sub fields', async () => { }) test('defining a repeater sends sub fields named from their labels', async () => { - serveFields([]) - const defined = captureDefine() + serveFieldCatalogue([]) + const defined = captureDefine(history) renderScreen() await startRepeater() @@ -335,8 +435,8 @@ test('defining a repeater sends sub fields named from their labels', async () => }) test('two sub fields sharing a label are sent under distinct names', async () => { - serveFields([]) - const defined = captureDefine() + serveFieldCatalogue([]) + const defined = captureDefine(history) renderScreen() await startRepeater() @@ -352,8 +452,8 @@ test('two sub fields sharing a label are sent under distinct names', async () => }) test('a sub field labelled ID is sent under a name other than the one entries keep their id under', async () => { - serveFields([]) - const defined = captureDefine() + serveFieldCatalogue([]) + const defined = captureDefine(history) renderScreen() await startRepeater() @@ -365,7 +465,7 @@ test('a sub field labelled ID is sent under a name other than the one entries ke }) test('the sub field kind menu leaves the repeater out', async () => { - serveFields([]) + serveFieldCatalogue([]) renderScreen() await startRepeater() @@ -378,8 +478,8 @@ test('the sub field kind menu leaves the repeater out', async () => { }) test('a field moved off the repeater kind sends no sub fields', async () => { - serveFields([]) - const defined = captureDefine() + serveFieldCatalogue([]) + const defined = captureDefine(history) renderScreen() await startRepeater() @@ -394,8 +494,8 @@ test('a field moved off the repeater kind sends no sub fields', async () => { }) test('a defined repeater clears its sub fields for the next one', async () => { - serveFields([]) - captureDefine() + serveFieldCatalogue([]) + captureDefine(history) renderScreen() await startRepeater() @@ -407,7 +507,7 @@ test('a defined repeater clears its sub fields for the next one', async () => { }) test('archiving a field sends its id', async () => { - serveFields([birthDate]) + serveFieldCatalogue([birthDate]) const archived = vi.fn() server.use( graphql.mutation('ArchiveField', async ({ variables }) => { From 4309ac4eb9d039de6f4df744357f1c7aa969dab1 Mon Sep 17 00:00:00 2001 From: SirLouen Date: Mon, 28 Sep 2026 10:36:49 +0200 Subject: [PATCH 14/25] feat(fields): head the name column API name --- plugins/fields/frontend/FieldsScreen.tsx | 2 +- plugins/fields/frontend/languages/es-ES.json | 2 +- plugins/fields/frontend/test/fields.test.tsx | 2 +- plugins/fields/languages/alphone-fields.pot | 6 +++--- plugins/fields/languages/es-ES.po | 8 ++++---- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/plugins/fields/frontend/FieldsScreen.tsx b/plugins/fields/frontend/FieldsScreen.tsx index d54527f4..bf73551c 100644 --- a/plugins/fields/frontend/FieldsScreen.tsx +++ b/plugins/fields/frontend/FieldsScreen.tsx @@ -160,7 +160,7 @@ function FieldList({ fields, onChanged }: { fields: FieldRow[]; onChanged: () => {__('Label', 'alphone-fields')} - {__('Name', 'alphone-fields')} + {__('API name', 'alphone-fields')} {__('Kind', 'alphone-fields')} diff --git a/plugins/fields/frontend/languages/es-ES.json b/plugins/fields/frontend/languages/es-ES.json index 6e4ae86e..eb228c65 100644 --- a/plugins/fields/frontend/languages/es-ES.json +++ b/plugins/fields/frontend/languages/es-ES.json @@ -1 +1 @@ -{"":{"lang":"es","plural-forms":""},"%(label)s: %(value)s":["%(label)s: %(value)s"],"A field name starts lowercase and runs together, like birthDate.":["El nombre de un campo empieza por minúscula y va todo junto, como birthDate."],"A field needs a label.":["El campo necesita una etiqueta."],"A repeater cannot hold another repeater.":["Un repetidor no puede contener otro repetidor."],"A repeater needs at least one sub field.":["Un repetidor necesita al menos un subcampo."],"A repeater takes its entries one at a time.":["Un repetidor recibe sus entradas de una en una."],"A sub field cannot be named id. Each entry keeps that name for itself.":["Un subcampo no puede llamarse id. Cada entrada guarda ese nombre para sí misma."],"A sub field name starts lowercase and runs together, like followUp.":["El nombre de un subcampo empieza por minúscula y va todo junto, como followUp."],"Add a field":["Añadir un campo"],"Add a field to store more about every contact.":["Añade un campo para guardar más información de cada contacto."],"Add an entry to %(label)s":["Añadir una entrada a %(label)s"],"Add field":["Añadir campo"],"Add sub field":["Añadir subcampo"],"An archived field of that name holds another kind or other sub fields.":["Un campo archivado con ese nombre tiene otro tipo u otros subcampos."],"Another field already holds that name.":["Ya hay otro campo con ese nombre."],"Archive":["Archivar"],"Archive %(label)s":["Archivar %(label)s"],"Blank entry":["Entrada en blanco"],"Cancel":["Cancelar"],"Edit entry":["Editar la entrada"],"Edit entry: %(name)s":["Editar la entrada: %(name)s"],"Fields":["Campos"],"Fill in at least one part of the entry.":["Rellena al menos una parte de la entrada."],"Keep":["Conservar"],"Kind":["Tipo"],"Label":["Etiqueta"],"Loading fields…":["Cargando los campos…"],"Move sub field down":["Bajar el subcampo"],"Move sub field up":["Subir el subcampo"],"Name":["Nombre"],"No entries yet.":["Todavía no hay ninguna entrada."],"No fields yet.":["Todavía no hay ningún campo."],"No sub fields yet.":["Todavía no hay ningún subcampo."],"Only a repeater holds sub fields.":["Solo un repetidor tiene subcampos."],"Remove":["Quitar"],"Remove entry":["Quitar la entrada"],"Remove entry: %(name)s":["Quitar la entrada: %(name)s"],"Remove sub field":["Quitar el subcampo"],"Remove this entry?":["¿Quitar esta entrada?"],"Save entry":["Guardar la entrada"],"Save fields":["Guardar los campos"],"Send the values as field names to values.":["Envía los valores como pares de nombre de campo y valor."],"Sub field %(number)d":["Subcampo %(number)d"],"Sub fields":["Subcampos"],"That entry no longer exists.":["Esa entrada ya no existe."],"That field does not keep a list of entries.":["Ese campo no guarda una lista de entradas."],"That field is not one this contact holds.":["Ese campo no es uno de los que tiene este contacto."],"That field no longer exists.":["Ese campo ya no existe."],"That kind is not one AlphOne offers.":["Ese tipo no es uno de los que ofrece AlphOne."],"That label is too long. Try a shorter one.":["Esa etiqueta es demasiado larga. Prueba con una más corta."],"That value does not match the kind the field declares.":["Ese valor no encaja con el tipo que declara el campo."],"The contact already holds a detail by that name.":["El contacto ya tiene un dato con ese nombre."],"The entry could not be added.":["No se ha podido añadir la entrada."],"The entry could not be removed.":["No se ha podido quitar la entrada."],"The entry could not be saved.":["No se ha podido guardar la entrada."],"The field could not be archived.":["No se ha podido archivar el campo."],"The field could not be defined.":["No se ha podido definir el campo."],"The fields could not be loaded.":["No se han podido cargar los campos."],"The fields could not be saved.":["No se han podido guardar los campos."],"This list is full. It holds %(max)d entries at most.":["Esta lista está llena. Guarda como mucho %(max)d entradas."],"Two sub fields of this repeater hold the same name.":["Dos subcampos de este repetidor tienen el mismo nombre."],"admin section\u0004Fields":["Campos"],"entry cell\u0004No":["No"],"entry cell\u0004Yes":["Sí"],"entry name\u0004%(day)s, %(text)s":["%(day)s, %(text)s"],"field kind\u0004Choice":["Elección"],"field kind\u0004Date":["Fecha"],"field kind\u0004Long text":["Texto largo"],"field kind\u0004Number":["Número"],"field kind\u0004Repeater":["Repetidor"],"field kind\u0004Text":["Texto"],"field kind\u0004Yes or no":["Sí o no"]} +{"":{"lang":"es","plural-forms":""},"%(label)s: %(value)s":["%(label)s: %(value)s"],"A field name starts lowercase and runs together, like birthDate.":["El nombre de un campo empieza por minúscula y va todo junto, como birthDate."],"A field needs a label.":["El campo necesita una etiqueta."],"A repeater cannot hold another repeater.":["Un repetidor no puede contener otro repetidor."],"A repeater needs at least one sub field.":["Un repetidor necesita al menos un subcampo."],"A repeater takes its entries one at a time.":["Un repetidor recibe sus entradas de una en una."],"A sub field cannot be named id. Each entry keeps that name for itself.":["Un subcampo no puede llamarse id. Cada entrada guarda ese nombre para sí misma."],"A sub field name starts lowercase and runs together, like followUp.":["El nombre de un subcampo empieza por minúscula y va todo junto, como followUp."],"API name":["Nombre en la API"],"Add a field":["Añadir un campo"],"Add a field to store more about every contact.":["Añade un campo para guardar más información de cada contacto."],"Add an entry to %(label)s":["Añadir una entrada a %(label)s"],"Add field":["Añadir campo"],"Add sub field":["Añadir subcampo"],"An archived field of that name holds another kind or other sub fields.":["Un campo archivado con ese nombre tiene otro tipo u otros subcampos."],"Another field already holds that name.":["Ya hay otro campo con ese nombre."],"Archive":["Archivar"],"Archive %(label)s":["Archivar %(label)s"],"Blank entry":["Entrada en blanco"],"Cancel":["Cancelar"],"Edit entry":["Editar la entrada"],"Edit entry: %(name)s":["Editar la entrada: %(name)s"],"Fields":["Campos"],"Fill in at least one part of the entry.":["Rellena al menos una parte de la entrada."],"Keep":["Conservar"],"Kind":["Tipo"],"Label":["Etiqueta"],"Loading fields…":["Cargando los campos…"],"Move sub field down":["Bajar el subcampo"],"Move sub field up":["Subir el subcampo"],"No entries yet.":["Todavía no hay ninguna entrada."],"No fields yet.":["Todavía no hay ningún campo."],"No sub fields yet.":["Todavía no hay ningún subcampo."],"Only a repeater holds sub fields.":["Solo un repetidor tiene subcampos."],"Remove":["Quitar"],"Remove entry":["Quitar la entrada"],"Remove entry: %(name)s":["Quitar la entrada: %(name)s"],"Remove sub field":["Quitar el subcampo"],"Remove this entry?":["¿Quitar esta entrada?"],"Save entry":["Guardar la entrada"],"Save fields":["Guardar los campos"],"Send the values as field names to values.":["Envía los valores como pares de nombre de campo y valor."],"Sub field %(number)d":["Subcampo %(number)d"],"Sub fields":["Subcampos"],"That entry no longer exists.":["Esa entrada ya no existe."],"That field does not keep a list of entries.":["Ese campo no guarda una lista de entradas."],"That field is not one this contact holds.":["Ese campo no es uno de los que tiene este contacto."],"That field no longer exists.":["Ese campo ya no existe."],"That kind is not one AlphOne offers.":["Ese tipo no es uno de los que ofrece AlphOne."],"That label is too long. Try a shorter one.":["Esa etiqueta es demasiado larga. Prueba con una más corta."],"That value does not match the kind the field declares.":["Ese valor no encaja con el tipo que declara el campo."],"The contact already holds a detail by that name.":["El contacto ya tiene un dato con ese nombre."],"The entry could not be added.":["No se ha podido añadir la entrada."],"The entry could not be removed.":["No se ha podido quitar la entrada."],"The entry could not be saved.":["No se ha podido guardar la entrada."],"The field could not be archived.":["No se ha podido archivar el campo."],"The field could not be defined.":["No se ha podido definir el campo."],"The fields could not be loaded.":["No se han podido cargar los campos."],"The fields could not be saved.":["No se han podido guardar los campos."],"This list is full. It holds %(max)d entries at most.":["Esta lista está llena. Guarda como mucho %(max)d entradas."],"Two sub fields of this repeater hold the same name.":["Dos subcampos de este repetidor tienen el mismo nombre."],"admin section\u0004Fields":["Campos"],"entry cell\u0004No":["No"],"entry cell\u0004Yes":["Sí"],"entry name\u0004%(day)s, %(text)s":["%(day)s, %(text)s"],"field kind\u0004Choice":["Elección"],"field kind\u0004Date":["Fecha"],"field kind\u0004Long text":["Texto largo"],"field kind\u0004Number":["Número"],"field kind\u0004Repeater":["Repetidor"],"field kind\u0004Text":["Texto"],"field kind\u0004Yes or no":["Sí o no"]} diff --git a/plugins/fields/frontend/test/fields.test.tsx b/plugins/fields/frontend/test/fields.test.tsx index 6dd5c4ed..e3f0ca09 100644 --- a/plugins/fields/frontend/test/fields.test.tsx +++ b/plugins/fields/frontend/test/fields.test.tsx @@ -80,7 +80,7 @@ test('the catalogue lists every defined field in a table', async () => { expect(cells[1]).toHaveTextContent('birthDate') expect(cells[2]).toHaveTextContent('Date') expect(within(table).getByRole('columnheader', { name: 'Label' })).toBeInTheDocument() - expect(within(table).getByRole('columnheader', { name: 'Name' })).toBeInTheDocument() + expect(within(table).getByRole('columnheader', { name: 'API name' })).toBeInTheDocument() expect(within(table).getByRole('columnheader', { name: 'Kind' })).toBeInTheDocument() }) diff --git a/plugins/fields/languages/alphone-fields.pot b/plugins/fields/languages/alphone-fields.pot index 7d80adcf..6b3cdf8d 100644 --- a/plugins/fields/languages/alphone-fields.pot +++ b/plugins/fields/languages/alphone-fields.pot @@ -31,6 +31,9 @@ msgstr "" msgid "A sub field name starts lowercase and runs together, like followUp." msgstr "" +msgid "API name" +msgstr "" + msgid "Add a field" msgstr "" @@ -94,9 +97,6 @@ msgstr "" msgid "Move sub field up" msgstr "" -msgid "Name" -msgstr "" - msgid "No entries yet." msgstr "" diff --git a/plugins/fields/languages/es-ES.po b/plugins/fields/languages/es-ES.po index 01402dbd..dd5cafcc 100644 --- a/plugins/fields/languages/es-ES.po +++ b/plugins/fields/languages/es-ES.po @@ -40,6 +40,10 @@ msgstr "Un subcampo no puede llamarse id. Cada entrada guarda ese nombre para s msgid "A sub field name starts lowercase and runs together, like followUp." msgstr "El nombre de un subcampo empieza por minúscula y va todo junto, como followUp." +#, fuzzy +msgid "API name" +msgstr "Nombre en la API" + #, fuzzy msgid "Add a field" msgstr "Añadir un campo" @@ -124,10 +128,6 @@ msgstr "Bajar el subcampo" msgid "Move sub field up" msgstr "Subir el subcampo" -#, fuzzy -msgid "Name" -msgstr "Nombre" - #, fuzzy msgid "No entries yet." msgstr "Todavía no hay ninguna entrada." From ab0e766f5460f73ffe8847ccee158cf5277c59f7 Mon Sep 17 00:00:00 2001 From: SirLouen Date: Mon, 28 Sep 2026 10:40:10 +0200 Subject: [PATCH 15/25] feat(fields): fetch the fields again when another one takes the name --- plugins/fields/frontend/FieldsScreen.tsx | 24 +++++-- plugins/fields/frontend/entryOutcome.ts | 4 +- plugins/fields/frontend/errorTemplates.ts | 4 +- plugins/fields/frontend/languages/es-ES.json | 2 +- plugins/fields/frontend/test/fields.test.tsx | 67 +++++++++++++++++++- plugins/fields/languages/alphone-fields.pot | 9 +-- plugins/fields/languages/es-ES.po | 12 ++-- 7 files changed, 94 insertions(+), 28 deletions(-) diff --git a/plugins/fields/frontend/FieldsScreen.tsx b/plugins/fields/frontend/FieldsScreen.tsx index bf73551c..0337f6c2 100644 --- a/plugins/fields/frontend/FieldsScreen.tsx +++ b/plugins/fields/frontend/FieldsScreen.tsx @@ -24,6 +24,7 @@ import { } from '@alphone/frontend-sdk' import { useState } from 'react' +import { reasonOf } from './entryOutcome' import { fieldsIcon } from './icon' import type { FieldCatalogueQuery, FieldKind } from './gql/graphql' import { ENTRY_ID_KEY, kindItems, kindOf, subKindItems } from './kind' @@ -62,6 +63,9 @@ interface KnownNames { reserved: string[] } +/** RACED are the reasons a define answers when another field took its name first. */ +const RACED = new Set(['field_name_taken', 'field_kind_locked']) + /** * Renders the catalogue of contact fields an operator defines. * @returns The fields screen. @@ -93,7 +97,7 @@ export function FieldsScreen() { }> {__('Add a field', 'alphone-fields')} - + @@ -252,21 +256,24 @@ function namedSubFields(drafts: DraftSubField[]) { * Returns the name a new field takes from its label. * @param label - The label of the new field. * @param known - Every stored field and the names the server refuses. + * @param refused - The names the server refused as taken on this visit. * @returns The name, numbered past every name already taken. */ -function fieldName(label: string, known: KnownNames) { - return keyFromLabel(label, { style: 'camel', taken: [...known.reserved, ...known.every.map((row) => row.name)] }) +function fieldName(label: string, known: KnownNames, refused: readonly string[]) { + const stored = known.every.map((row) => row.name) + return keyFromLabel(label, { style: 'camel', taken: [...known.reserved, ...refused, ...stored] }) } /** * Renders the form defining one new field, naming it from its label. - * @param props - The names a new field steps past and the reload run after a definition lands. + * @param props - The names a new field steps past and the reload run after every answer. * @returns The add field form. */ -function AddFieldForm({ known, onAdded }: { known: KnownNames; onAdded: () => void }) { +function AddFieldForm({ known, onAnswered }: { known: KnownNames; onAnswered: () => void }) { const [label, setLabel] = useState('') const [kind, setKind] = useState('TEXT') const [subFields, setSubFields] = useState([]) + const [refused, setRefused] = useState([]) const kinds = kindItems() const [defined, define] = useGraphMutation(defineFieldMutation) const repeater = kind === 'REPEATER' @@ -276,12 +283,15 @@ function AddFieldForm({ known, onAdded }: { known: KnownNames; onAdded: () => vo className="godmin-form" onSubmit={(event) => { event.preventDefault() + const name = fieldName(label, known, refused) const sent = repeater ? namedSubFields(subFields) : undefined - void define({ name: fieldName(label, known), label, kind, subFields: sent }).then((result) => { + void define({ name, label, kind, subFields: sent }).then((result) => { + onAnswered() if (!result.error) { setLabel('') setSubFields([]) - onAdded() + } else if (RACED.has(reasonOf(result.error))) { + setRefused((held) => [...held, name]) } }) }} diff --git a/plugins/fields/frontend/entryOutcome.ts b/plugins/fields/frontend/entryOutcome.ts index e1de63c4..44bbe831 100644 --- a/plugins/fields/frontend/entryOutcome.ts +++ b/plugins/fields/frontend/entryOutcome.ts @@ -26,11 +26,11 @@ const REFETCHES = new Map([ ]) /** - * Returns the reason a refused entry call answered with. + * Returns the reason a refused call answered with. * @param error - The failure the call answered with. * @returns The reason, or an empty string. */ -function reasonOf(error: GraphFailure): string { +export function reasonOf(error: GraphFailure): string { const reason = graphExtensions(error).reason return typeof reason === 'string' ? reason : '' } diff --git a/plugins/fields/frontend/errorTemplates.ts b/plugins/fields/frontend/errorTemplates.ts index b8122b19..87163698 100644 --- a/plugins/fields/frontend/errorTemplates.ts +++ b/plugins/fields/frontend/errorTemplates.ts @@ -23,8 +23,8 @@ export function errorTemplates(): Record { field_sub_field_name_invalid: __('A sub field name starts lowercase and runs together, like followUp.', DOMAIN), field_sub_field_name_taken: __('Two sub fields of this repeater hold the same name.', DOMAIN), field_sub_field_name_reserved: __('A sub field cannot be named id. Each entry keeps that name for itself.', DOMAIN), - field_name_taken: __('Another field already holds that name.', DOMAIN), - field_kind_locked: __('An archived field of that name holds another kind or other sub fields.', DOMAIN), + field_name_taken: __('The field list just changed. Press Add field again.', DOMAIN), + field_kind_locked: __('The field list just changed. Press Add field again.', DOMAIN), field_not_found: __('That field no longer exists.', DOMAIN), field_unknown: __('That field is not one this contact holds.', DOMAIN), value_kind_mismatch: __('That value does not match the kind the field declares.', DOMAIN), diff --git a/plugins/fields/frontend/languages/es-ES.json b/plugins/fields/frontend/languages/es-ES.json index eb228c65..8d9fe4f4 100644 --- a/plugins/fields/frontend/languages/es-ES.json +++ b/plugins/fields/frontend/languages/es-ES.json @@ -1 +1 @@ -{"":{"lang":"es","plural-forms":""},"%(label)s: %(value)s":["%(label)s: %(value)s"],"A field name starts lowercase and runs together, like birthDate.":["El nombre de un campo empieza por minúscula y va todo junto, como birthDate."],"A field needs a label.":["El campo necesita una etiqueta."],"A repeater cannot hold another repeater.":["Un repetidor no puede contener otro repetidor."],"A repeater needs at least one sub field.":["Un repetidor necesita al menos un subcampo."],"A repeater takes its entries one at a time.":["Un repetidor recibe sus entradas de una en una."],"A sub field cannot be named id. Each entry keeps that name for itself.":["Un subcampo no puede llamarse id. Cada entrada guarda ese nombre para sí misma."],"A sub field name starts lowercase and runs together, like followUp.":["El nombre de un subcampo empieza por minúscula y va todo junto, como followUp."],"API name":["Nombre en la API"],"Add a field":["Añadir un campo"],"Add a field to store more about every contact.":["Añade un campo para guardar más información de cada contacto."],"Add an entry to %(label)s":["Añadir una entrada a %(label)s"],"Add field":["Añadir campo"],"Add sub field":["Añadir subcampo"],"An archived field of that name holds another kind or other sub fields.":["Un campo archivado con ese nombre tiene otro tipo u otros subcampos."],"Another field already holds that name.":["Ya hay otro campo con ese nombre."],"Archive":["Archivar"],"Archive %(label)s":["Archivar %(label)s"],"Blank entry":["Entrada en blanco"],"Cancel":["Cancelar"],"Edit entry":["Editar la entrada"],"Edit entry: %(name)s":["Editar la entrada: %(name)s"],"Fields":["Campos"],"Fill in at least one part of the entry.":["Rellena al menos una parte de la entrada."],"Keep":["Conservar"],"Kind":["Tipo"],"Label":["Etiqueta"],"Loading fields…":["Cargando los campos…"],"Move sub field down":["Bajar el subcampo"],"Move sub field up":["Subir el subcampo"],"No entries yet.":["Todavía no hay ninguna entrada."],"No fields yet.":["Todavía no hay ningún campo."],"No sub fields yet.":["Todavía no hay ningún subcampo."],"Only a repeater holds sub fields.":["Solo un repetidor tiene subcampos."],"Remove":["Quitar"],"Remove entry":["Quitar la entrada"],"Remove entry: %(name)s":["Quitar la entrada: %(name)s"],"Remove sub field":["Quitar el subcampo"],"Remove this entry?":["¿Quitar esta entrada?"],"Save entry":["Guardar la entrada"],"Save fields":["Guardar los campos"],"Send the values as field names to values.":["Envía los valores como pares de nombre de campo y valor."],"Sub field %(number)d":["Subcampo %(number)d"],"Sub fields":["Subcampos"],"That entry no longer exists.":["Esa entrada ya no existe."],"That field does not keep a list of entries.":["Ese campo no guarda una lista de entradas."],"That field is not one this contact holds.":["Ese campo no es uno de los que tiene este contacto."],"That field no longer exists.":["Ese campo ya no existe."],"That kind is not one AlphOne offers.":["Ese tipo no es uno de los que ofrece AlphOne."],"That label is too long. Try a shorter one.":["Esa etiqueta es demasiado larga. Prueba con una más corta."],"That value does not match the kind the field declares.":["Ese valor no encaja con el tipo que declara el campo."],"The contact already holds a detail by that name.":["El contacto ya tiene un dato con ese nombre."],"The entry could not be added.":["No se ha podido añadir la entrada."],"The entry could not be removed.":["No se ha podido quitar la entrada."],"The entry could not be saved.":["No se ha podido guardar la entrada."],"The field could not be archived.":["No se ha podido archivar el campo."],"The field could not be defined.":["No se ha podido definir el campo."],"The fields could not be loaded.":["No se han podido cargar los campos."],"The fields could not be saved.":["No se han podido guardar los campos."],"This list is full. It holds %(max)d entries at most.":["Esta lista está llena. Guarda como mucho %(max)d entradas."],"Two sub fields of this repeater hold the same name.":["Dos subcampos de este repetidor tienen el mismo nombre."],"admin section\u0004Fields":["Campos"],"entry cell\u0004No":["No"],"entry cell\u0004Yes":["Sí"],"entry name\u0004%(day)s, %(text)s":["%(day)s, %(text)s"],"field kind\u0004Choice":["Elección"],"field kind\u0004Date":["Fecha"],"field kind\u0004Long text":["Texto largo"],"field kind\u0004Number":["Número"],"field kind\u0004Repeater":["Repetidor"],"field kind\u0004Text":["Texto"],"field kind\u0004Yes or no":["Sí o no"]} +{"":{"lang":"es","plural-forms":""},"%(label)s: %(value)s":["%(label)s: %(value)s"],"A field name starts lowercase and runs together, like birthDate.":["El nombre de un campo empieza por minúscula y va todo junto, como birthDate."],"A field needs a label.":["El campo necesita una etiqueta."],"A repeater cannot hold another repeater.":["Un repetidor no puede contener otro repetidor."],"A repeater needs at least one sub field.":["Un repetidor necesita al menos un subcampo."],"A repeater takes its entries one at a time.":["Un repetidor recibe sus entradas de una en una."],"A sub field cannot be named id. Each entry keeps that name for itself.":["Un subcampo no puede llamarse id. Cada entrada guarda ese nombre para sí misma."],"A sub field name starts lowercase and runs together, like followUp.":["El nombre de un subcampo empieza por minúscula y va todo junto, como followUp."],"API name":["Nombre en la API"],"Add a field":["Añadir un campo"],"Add a field to store more about every contact.":["Añade un campo para guardar más información de cada contacto."],"Add an entry to %(label)s":["Añadir una entrada a %(label)s"],"Add field":["Añadir campo"],"Add sub field":["Añadir subcampo"],"Archive":["Archivar"],"Archive %(label)s":["Archivar %(label)s"],"Blank entry":["Entrada en blanco"],"Cancel":["Cancelar"],"Edit entry":["Editar la entrada"],"Edit entry: %(name)s":["Editar la entrada: %(name)s"],"Fields":["Campos"],"Fill in at least one part of the entry.":["Rellena al menos una parte de la entrada."],"Keep":["Conservar"],"Kind":["Tipo"],"Label":["Etiqueta"],"Loading fields…":["Cargando los campos…"],"Move sub field down":["Bajar el subcampo"],"Move sub field up":["Subir el subcampo"],"No entries yet.":["Todavía no hay ninguna entrada."],"No fields yet.":["Todavía no hay ningún campo."],"No sub fields yet.":["Todavía no hay ningún subcampo."],"Only a repeater holds sub fields.":["Solo un repetidor tiene subcampos."],"Remove":["Quitar"],"Remove entry":["Quitar la entrada"],"Remove entry: %(name)s":["Quitar la entrada: %(name)s"],"Remove sub field":["Quitar el subcampo"],"Remove this entry?":["¿Quitar esta entrada?"],"Save entry":["Guardar la entrada"],"Save fields":["Guardar los campos"],"Send the values as field names to values.":["Envía los valores como pares de nombre de campo y valor."],"Sub field %(number)d":["Subcampo %(number)d"],"Sub fields":["Subcampos"],"That entry no longer exists.":["Esa entrada ya no existe."],"That field does not keep a list of entries.":["Ese campo no guarda una lista de entradas."],"That field is not one this contact holds.":["Ese campo no es uno de los que tiene este contacto."],"That field no longer exists.":["Ese campo ya no existe."],"That kind is not one AlphOne offers.":["Ese tipo no es uno de los que ofrece AlphOne."],"That label is too long. Try a shorter one.":["Esa etiqueta es demasiado larga. Prueba con una más corta."],"That value does not match the kind the field declares.":["Ese valor no encaja con el tipo que declara el campo."],"The contact already holds a detail by that name.":["El contacto ya tiene un dato con ese nombre."],"The entry could not be added.":["No se ha podido añadir la entrada."],"The entry could not be removed.":["No se ha podido quitar la entrada."],"The entry could not be saved.":["No se ha podido guardar la entrada."],"The field could not be archived.":["No se ha podido archivar el campo."],"The field could not be defined.":["No se ha podido definir el campo."],"The field list just changed. Press Add field again.":["La lista de campos acaba de cambiar. Pulsa Añadir campo otra vez."],"The fields could not be loaded.":["No se han podido cargar los campos."],"The fields could not be saved.":["No se han podido guardar los campos."],"This list is full. It holds %(max)d entries at most.":["Esta lista está llena. Guarda como mucho %(max)d entradas."],"Two sub fields of this repeater hold the same name.":["Dos subcampos de este repetidor tienen el mismo nombre."],"admin section\u0004Fields":["Campos"],"entry cell\u0004No":["No"],"entry cell\u0004Yes":["Sí"],"entry name\u0004%(day)s, %(text)s":["%(day)s, %(text)s"],"field kind\u0004Choice":["Elección"],"field kind\u0004Date":["Fecha"],"field kind\u0004Long text":["Texto largo"],"field kind\u0004Number":["Número"],"field kind\u0004Repeater":["Repetidor"],"field kind\u0004Text":["Texto"],"field kind\u0004Yes or no":["Sí o no"]} diff --git a/plugins/fields/frontend/test/fields.test.tsx b/plugins/fields/frontend/test/fields.test.tsx index e3f0ca09..37129d61 100644 --- a/plugins/fields/frontend/test/fields.test.tsx +++ b/plugins/fields/frontend/test/fields.test.tsx @@ -1,6 +1,6 @@ // SPDX-License-Identifier: AGPL-3.0-or-later -import { GraphProvider } from '@alphone/frontend-sdk' +import { GraphProvider, configureErrorText } from '@alphone/frontend-sdk' import { HttpResponse, fakeGraphClient, @@ -10,9 +10,14 @@ import { import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { render, screen, waitFor, within } from '@testing-library/react' import userEvent from '@testing-library/user-event' -import { expect, test, vi } from 'vitest' +import { afterEach, expect, test, vi } from 'vitest' import { FieldsScreen } from '../FieldsScreen' +import { refusal, speakTemplates } from './harness' + +afterEach(() => { + configureErrorText({ templates: () => ({}), fallback: () => '' }) +}) const RESERVED = [ 'constructor', 'createdAt', 'field', 'hasOwnProperty', 'id', 'identities', 'isPrototypeOf', @@ -244,6 +249,64 @@ test('a taken name is reported word for word', async () => { expect(await screen.findByRole('alert')).toHaveTextContent(/holds that name/) }) +test('a name taken meanwhile asks to press Add field again', async () => { + speakTemplates() + serveFieldCatalogue([]) + server.use(graphql.mutation('DefineField', () => HttpResponse.json(refusal('CONFLICT', 'field_name_taken')))) + + renderScreen() + await submitField() + + expect(await screen.findByRole('alert')).toHaveTextContent('The field list just changed. Press Add field again.') +}) + +test('an archived field holding the name asks to press Add field again', async () => { + speakTemplates() + serveFieldCatalogue([]) + server.use(graphql.mutation('DefineField', () => HttpResponse.json(refusal('CONFLICT', 'field_kind_locked')))) + + renderScreen() + await submitField() + + expect(await screen.findByRole('alert')).toHaveTextContent('The field list just changed. Press Add field again.') +}) + +test('the catalogue is read again after a refused define', async () => { + let reads = 0 + server.use( + graphql.query('FieldCatalogue', () => { + reads += 1 + return HttpResponse.json({ data: { fields: [], every: [], reservedFieldNames: RESERVED } }) + }), + graphql.mutation('DefineField', () => HttpResponse.json(refusal('VALIDATION', 'field_label_too_long'))), + ) + + renderScreen() + await submitField() + await screen.findByRole('alert') + + await waitFor(() => expect(reads).toBe(2)) +}) + +test('a refused name is numbered on the next press', async () => { + serveFieldCatalogue([]) + const defined = vi.fn() + server.use( + graphql.mutation('DefineField', async ({ variables }) => { + defined(variables) + return HttpResponse.json(refusal('CONFLICT', 'field_name_taken')) + }), + ) + + renderScreen() + await submitField() + await waitFor(() => expect(defined).toHaveBeenCalledTimes(1)) + await userEvent.click(screen.getByRole('button', { name: 'Add field' })) + + await waitFor(() => expect(defined).toHaveBeenCalledTimes(2)) + expect(defined.mock.calls.map((call) => call[0].name)).toEqual(['birthDate', 'birthDate2']) +}) + test('the chosen kind is sent with the definition', async () => { serveFieldCatalogue([]) const defined = captureDefine() diff --git a/plugins/fields/languages/alphone-fields.pot b/plugins/fields/languages/alphone-fields.pot index 6b3cdf8d..0dcab01f 100644 --- a/plugins/fields/languages/alphone-fields.pot +++ b/plugins/fields/languages/alphone-fields.pot @@ -49,12 +49,6 @@ msgstr "" msgid "Add sub field" msgstr "" -msgid "An archived field of that name holds another kind or other sub fields." -msgstr "" - -msgid "Another field already holds that name." -msgstr "" - msgid "Archive" msgstr "" @@ -178,6 +172,9 @@ msgstr "" msgid "The field could not be defined." msgstr "" +msgid "The field list just changed. Press Add field again." +msgstr "" + msgid "The fields could not be loaded." msgstr "" diff --git a/plugins/fields/languages/es-ES.po b/plugins/fields/languages/es-ES.po index dd5cafcc..f35b9d3f 100644 --- a/plugins/fields/languages/es-ES.po +++ b/plugins/fields/languages/es-ES.po @@ -64,14 +64,6 @@ msgstr "Añadir campo" msgid "Add sub field" msgstr "Añadir subcampo" -#, fuzzy -msgid "An archived field of that name holds another kind or other sub fields." -msgstr "Un campo archivado con ese nombre tiene otro tipo u otros subcampos." - -#, fuzzy -msgid "Another field already holds that name." -msgstr "Ya hay otro campo con ese nombre." - #, fuzzy msgid "Archive" msgstr "Archivar" @@ -236,6 +228,10 @@ msgstr "No se ha podido archivar el campo." msgid "The field could not be defined." msgstr "No se ha podido definir el campo." +#, fuzzy +msgid "The field list just changed. Press Add field again." +msgstr "La lista de campos acaba de cambiar. Pulsa Añadir campo otra vez." + #, fuzzy msgid "The fields could not be loaded." msgstr "No se han podido cargar los campos." From 4ade4cf167e96ca84df73cadff82663901166791 Mon Sep 17 00:00:00 2001 From: SirLouen Date: Mon, 28 Sep 2026 10:40:11 +0200 Subject: [PATCH 16/25] test(frontend): expect the field list message in the merged templates --- frontend/src/test/locale-boot.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/test/locale-boot.test.ts b/frontend/src/test/locale-boot.test.ts index 8509e0d3..118d6d1e 100644 --- a/frontend/src/test/locale-boot.test.ts +++ b/frontend/src/test/locale-boot.test.ts @@ -91,7 +91,7 @@ test('renders core and plugin reasons from one merged map', () => { const merged = appErrorTemplates() expect(merged.contact_name_required).toBe('A contact needs a name.') - expect(merged.field_name_taken).toBe('Another field already holds that name.') + expect(merged.field_name_taken).toBe('The field list just changed. Press Add field again.') expect(merged.import_not_found).toBe('That import no longer exists.') expect(merged.message_content_required).toBe('Write something to send.') }) From de26f490e0f7e1c133191777012c2a5c8debe8b4 Mon Sep 17 00:00:00 2001 From: SirLouen Date: Mon, 28 Sep 2026 10:44:26 +0200 Subject: [PATCH 17/25] feat(fields): refuse a label a live field already has --- plugins/fields/frontend/FieldsScreen.tsx | 31 ++++++++++++++--- plugins/fields/frontend/languages/es-ES.json | 2 +- plugins/fields/frontend/test/fields.test.tsx | 36 ++++++++++++++++++++ plugins/fields/languages/alphone-fields.pot | 3 ++ plugins/fields/languages/es-ES.po | 4 +++ 5 files changed, 71 insertions(+), 5 deletions(-) diff --git a/plugins/fields/frontend/FieldsScreen.tsx b/plugins/fields/frontend/FieldsScreen.tsx index 0337f6c2..fd21c01a 100644 --- a/plugins/fields/frontend/FieldsScreen.tsx +++ b/plugins/fields/frontend/FieldsScreen.tsx @@ -57,8 +57,9 @@ interface DraftSubField { kind: FieldKind } -/** KnownNames are every stored field and the names the server refuses, which a new field's name steps past. */ +/** KnownNames are the live fields, every stored field and the names the server refuses. */ interface KnownNames { + live: FieldRow[] every: FieldRow[] reserved: string[] } @@ -109,7 +110,7 @@ export function FieldsScreen() { * @param data - The catalogue answer, absent while none has arrived. * @returns The rows, each list empty when the answer lacks it. */ -function catalogueRows(data: FieldCatalogueQuery | undefined): KnownNames & { live: FieldRow[] } { +function catalogueRows(data: FieldCatalogueQuery | undefined): KnownNames { return { live: (data?.fields ?? []) as FieldRow[], every: (data?.every ?? []) as FieldRow[], @@ -264,6 +265,17 @@ function fieldName(label: string, known: KnownNames, refused: readonly string[]) return keyFromLabel(label, { style: 'camel', taken: [...known.reserved, ...refused, ...stored] }) } +/** + * Reports whether a live field already carries a label, ignoring case and outer spaces. + * @param label - The label typed. + * @param live - The live fields. + * @returns True when a live field carries it. + */ +function labelHeld(label: string, live: FieldRow[]) { + const typed = label.trim().toLowerCase() + return live.some((row) => row.label.trim().toLowerCase() === typed) +} + /** * Renders the form defining one new field, naming it from its label. * @param props - The names a new field steps past and the reload run after every answer. @@ -271,6 +283,7 @@ function fieldName(label: string, known: KnownNames, refused: readonly string[]) */ function AddFieldForm({ known, onAnswered }: { known: KnownNames; onAnswered: () => void }) { const [label, setLabel] = useState('') + const [labelTaken, setLabelTaken] = useState(false) const [kind, setKind] = useState('TEXT') const [subFields, setSubFields] = useState([]) const [refused, setRefused] = useState([]) @@ -283,6 +296,10 @@ function AddFieldForm({ known, onAnswered }: { known: KnownNames; onAnswered: () className="godmin-form" onSubmit={(event) => { event.preventDefault() + if (labelHeld(label, known.live)) { + setLabelTaken(true) + return + } const name = fieldName(label, known, refused) const sent = repeater ? namedSubFields(subFields) : undefined void define({ name, label, kind, subFields: sent }).then((result) => { @@ -296,7 +313,10 @@ function AddFieldForm({ known, onAnswered }: { known: KnownNames; onAnswered: () }) }} > - {defined.error ? ( + {labelTaken ? ( + {__('A field with that label already exists.', 'alphone-fields')} + ) : null} + {defined.error && !labelTaken ? ( {validationMessage(graphError(defined.error), __('The field could not be defined.', 'alphone-fields'))} @@ -305,7 +325,10 @@ function AddFieldForm({ known, onAnswered }: { known: KnownNames; onAnswered: () label={__('Label', 'alphone-fields')} autoComplete="off" value={label} - onChange={(event) => setLabel(event.target.value)} + onChange={(event) => { + setLabel(event.target.value) + setLabelTaken(false) + }} /> { expect(await sentName(defined)).toBe('birthDate2') }) +test('refuses a label a live field already has', async () => { + serveFieldCatalogue([birthDate]) + const defined = captureDefine() + + renderScreen() + await defineLabelled(' birth DATE ') + + expect(await screen.findByRole('alert')).toHaveTextContent('A field with that label already exists.') + expect(defined).not.toHaveBeenCalled() +}) + +test('a refused label replaces the notice of an earlier failure', async () => { + serveFieldCatalogue([birthDate]) + server.use(graphql.mutation('DefineField', () => HttpResponse.json(refusal('VALIDATION', 'field_label_too_long')))) + + renderScreen() + await defineLabelled('Anniversary') + await screen.findByRole('alert') + await userEvent.clear(screen.getByLabelText('Label')) + await defineLabelled('Birth date') + + expect(await screen.findByText('A field with that label already exists.')).toBeInTheDocument() + expect(screen.getAllByRole('alert')).toHaveLength(1) +}) + +test('typing another label clears the label notice', async () => { + serveFieldCatalogue([birthDate]) + + renderScreen() + await defineLabelled('Birth date') + await screen.findByText('A field with that label already exists.') + await userEvent.type(screen.getByLabelText('Label'), 's') + + expect(screen.queryByRole('alert')).not.toBeInTheDocument() +}) + test('steps past a name the contact already has', async () => { serveFieldCatalogue([]) const defined = captureDefine() diff --git a/plugins/fields/languages/alphone-fields.pot b/plugins/fields/languages/alphone-fields.pot index 0dcab01f..081144a9 100644 --- a/plugins/fields/languages/alphone-fields.pot +++ b/plugins/fields/languages/alphone-fields.pot @@ -16,6 +16,9 @@ msgstr "" msgid "A field needs a label." msgstr "" +msgid "A field with that label already exists." +msgstr "" + msgid "A repeater cannot hold another repeater." msgstr "" diff --git a/plugins/fields/languages/es-ES.po b/plugins/fields/languages/es-ES.po index f35b9d3f..36e7041d 100644 --- a/plugins/fields/languages/es-ES.po +++ b/plugins/fields/languages/es-ES.po @@ -20,6 +20,10 @@ msgstr "El nombre de un campo empieza por minúscula y va todo junto, como birth msgid "A field needs a label." msgstr "El campo necesita una etiqueta." +#, fuzzy +msgid "A field with that label already exists." +msgstr "Ya existe un campo con esa etiqueta." + #, fuzzy msgid "A repeater cannot hold another repeater." msgstr "Un repetidor no puede contener otro repetidor." From efb6ab1c5fc637873e4ff075639c81afe44431c5 Mon Sep 17 00:00:00 2001 From: SirLouen Date: Mon, 28 Sep 2026 11:23:08 +0200 Subject: [PATCH 18/25] docs(fields): explain field names and revival to API callers --- docs/src/content/docs/guides/fields.md | 22 ++++++++++++++++--- .../src/content/docs/reference/graphql-api.md | 4 ++-- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/docs/src/content/docs/guides/fields.md b/docs/src/content/docs/guides/fields.md index 9114742a..30509a65 100644 --- a/docs/src/content/docs/guides/fields.md +++ b/docs/src/content/docs/guides/fields.md @@ -188,9 +188,25 @@ mutation { } ``` -The API does not make sub field names for you. Send a camelCase name for each -one, unique inside the repeater. `id` is refused, because each entry keeps its -own id under that name. +The API does not make names for you. `defineField` takes a `name` such as +`birthDate`. It starts with a lowercase letter and uses only a to z, A to Z +and 0 to 9. Each sub field needs such a name too, unique inside its repeater. +`id` is refused for a sub field, because each entry keeps its own id under +that name. + +`reservedFieldNames` lists the names no field can take. They are the fields +`Contact` is built with, such as `name` and `tasks`, and the names every +JavaScript object has, such as `constructor` and `toString`. Such a name is +refused with `field_name_reserved`. Upgrading AlphOne moves a field stored +under one of the JavaScript names to the next free name, such as +`constructor2`, with its values. + +`fields(includeArchived: true)` also lists archived fields, each with its +`archivedAt`, so you can find the name to send. Calling `defineField` with an +archived field's name brings that field back with its values and the label +you send. The kind must match, and for a repeater so must the sub field names +and kinds, in the same order. Otherwise the call is refused with +`field_kind_locked`. The answer carries the field's old id. A repeater reads as a list of entries typed `JSON`, the last one added first. Each entry is an object keyed by sub field name, with the `id` AlphOne gave diff --git a/docs/src/content/docs/reference/graphql-api.md b/docs/src/content/docs/reference/graphql-api.md index 41f053d8..088324f0 100644 --- a/docs/src/content/docs/reference/graphql-api.md +++ b/docs/src/content/docs/reference/graphql-api.md @@ -413,7 +413,7 @@ The stock plugins add their own: | `field_label_required` | | a field needs a label | | `field_label_too_long` | | the label is past the cap | | `field_kind_unknown` | | the kind is not one the plugin knows | -| `field_name_reserved` | | the name is already a column of the type | +| `field_name_reserved` | | the name is one `reservedFieldNames` lists | | `field_sub_fields_required` | | a repeater needs at least one sub field | | `field_sub_fields_unexpected` | | only a repeater holds sub fields | | `field_sub_field_nested` | | a sub field cannot be a repeater | @@ -520,7 +520,7 @@ cannot drift. Point a client at the endpoint, or read | Contacts | `contacts`, `contact` | `createContact`, `renameContact`, `addContactIdentity`, `deleteContactIdentity` | | Tasks | `tasks`, `task` | `createTask`, `updateTask` | | Webhooks | `webhooks` | `createWebhook`, `deleteWebhook` | -| Fields | `fields`, `Contact.field` | `defineField`, `archiveField`, `writeContactFields`, `addContactFieldEntry`, `updateContactFieldEntry`, `deleteContactFieldEntry` | +| Fields | `fields`, `reservedFieldNames`, `Contact.field` | `defineField`, `archiveField`, `writeContactFields`, `addContactFieldEntry`, `updateContactFieldEntry`, `deleteContactFieldEntry` | | Imports | `imports`, `importJob`, `importFields` | `importUpload`, `importSetMapping`, `importCommit` | | WhatsApp | `whatsAppConversations`, `whatsAppConversation` | `whatsAppSendMessage` | | Version | `version` | | From 5d1f513b8e8bda8c5a4164df2ba752f3687cad6b Mon Sep 17 00:00:00 2001 From: SirLouen Date: Mon, 28 Sep 2026 11:23:08 +0200 Subject: [PATCH 19/25] docs(fields): say AlphOne makes a field's name from its label --- docs/src/content/docs/guides/fields.md | 53 +++++++++++++++++--------- 1 file changed, 35 insertions(+), 18 deletions(-) diff --git a/docs/src/content/docs/guides/fields.md b/docs/src/content/docs/guides/fields.md index 30509a65..659a730a 100644 --- a/docs/src/content/docs/guides/fields.md +++ b/docs/src/content/docs/guides/fields.md @@ -10,14 +10,9 @@ running AlphOne, without a restart or a new version. ## Add a field -Open **Fields** and fill in three things. +Open **Fields** and fill in two things. -**Label** is the text people see on screen, such as `Birth date`. Change it -whenever you like. - -**Name** is what the API calls the field, such as `birthDate`. It starts with -a lowercase letter and holds only letters and digits. Pick it carefully, -because it cannot be changed later. +**Label** is the text people see on screen, such as `Birth date`. **Kind** says what the field holds. Seven kinds are available. @@ -31,8 +26,25 @@ because it cannot be changed later. | Choice | A short line, kept apart from Text so a later release can add a fixed option list | | Repeater | A list of entries that share the same parts, such as a contact history | -Save, and the field exists. Open any contact and it is there, waiting to be -filled in. +Press **Add field**, and the field exists. Open any contact and it is there, +waiting to be filled in. + +AlphOne makes the field's name from its label, so `Birth date` becomes +`birthDate`. The API uses that name, and the field list shows it under +**API name**. If another field already has that name, even an archived one, +the new name gets the next free number, such as `birthDate2`. The number goes +straight after the name, so `Address 2` becomes `address22` when `address2` +is taken. The name cannot be changed later. + +If a field in the list already has the label, the screen says so and adds +nothing. Case and spaces at either end do not matter, so `birth date` counts +as `Birth date`. + +A few names are reserved, such as `name`, `tasks` and `constructor`. A label +that makes one of them gets a number too, so `Name` becomes `name2`. A label +with no Latin letters or digits, such as `???` or one written in Cyrillic, is +named after the word `field`. That name is reserved too, so it becomes +`field2`. ## Fill a field in @@ -57,10 +69,10 @@ a label and a kind. For a history, that is `Date` as a Date and `Comment` as a Long text. A sub field can be any kind except Repeater, so a repeater never holds another -repeater. You do not type a name for a sub field. AlphOne makes one from its -label, so `Follow-up comment` becomes `followUpComment`. Two sub fields with -the same label get two names, such as `note` and `note2`. A sub field labelled -`ID` becomes `id2`, because each entry keeps its own id under `id`. +repeater. AlphOne names a sub field from its label too, so +`Follow-up comment` becomes `followUpComment`. Two sub fields with the same +label get two names, such as `note` and `note2`. A sub field labelled `ID` +becomes `id2`, because each entry keeps its own id under `id`. The sub fields cannot be changed once the repeater exists. The reason is the same as for the kind: old entries would no longer fit. @@ -91,6 +103,10 @@ You do not have to type every value in by hand. When you import a CSV or an Excel file, your fields sit in the mapping dropdown beside Name, Email and Phone. Point a column at one and the values arrive with the contacts. +A field named `email` or `phone` stays out of the dropdown, because Email and +Phone already use those names. The labels `Email` and `Phone` make exactly +those names, so choose a longer label, such as `Email consent`. + The kind is checked before anything is stored. A row whose cell does not fit its field fails, the reason names the field and its kind, and no contact is created for that row. Fix the spreadsheet and import it again. @@ -120,15 +136,16 @@ leave old values that no longer fit. Press **Archive** beside a field. It disappears from the contact screen and from the API straight away. -Archiving does not delete anything. The values stay in the database. If you -create the field again later, with the same name and the same kind, the old -values come back. A repeater also needs the same sub fields, in the same -order and with the same names and kinds, or AlphOne refuses it. +Archiving does not delete anything. The values stay in the database, and the +archived field keeps its name. So a new field with the same label gets a +numbered name and starts empty. Only the API can bring the old field back with +its values. See [Using your fields from the API](#using-your-fields-from-the-api). ## Using your fields from the API A field you create becomes a real field on `Contact` in the GraphQL API, under -the name you chose. So after adding `birthDate` you can ask for it directly: +its API name. So after adding `Birth date` you can ask for `birthDate` +directly: ```graphql query { From 5f74dcb45f8eb685e47841d53fe196273048685e Mon Sep 17 00:00:00 2001 From: SirLouen Date: Mon, 28 Sep 2026 11:23:09 +0200 Subject: [PATCH 20/25] fix(fields): rewrite only the contact rows that hold an inherited name --- plugins/fields/migration_internal_test.go | 27 +++++++++++++++++++ .../migrations/00007_move_inherited_names.sql | 7 ++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/plugins/fields/migration_internal_test.go b/plugins/fields/migration_internal_test.go index ca6f4179..2f788a3b 100644 --- a/plugins/fields/migration_internal_test.go +++ b/plugins/fields/migration_internal_test.go @@ -349,6 +349,33 @@ func TestInheritedNamesMigrationMovesArchivedFieldsToo(t *testing.T) { } } +// valuesVersion returns the row version of one contact's values row. +func valuesVersion(t *testing.T, db *sql.DB, contactID uuid.UUID) string { + t.Helper() + var version string + if err := db.QueryRowContext(t.Context(), + "SELECT xmin::text FROM plugin_fields.contact_values WHERE contact_id = $1", contactID).Scan(&version); err != nil { + t.Fatalf("reading the row version: %v", err) + } + return version +} + +func TestInheritedNamesMigrationLeavesRowsWithoutTheNamesUnwritten(t *testing.T) { + t.Parallel() + + _, db, provider := beforeInheritedNames(t) + home := sdk.TenantOrDefault(t.Context()) + storedDefinition(t, db, home, "valueOf", "NUMBER", "[]", false) + rosa := contactHolding(t, db, home, `{"nickname": "Rosa"}`) + before := valuesVersion(t, db, rosa) + + moveInheritedNames(t, provider) + + if after := valuesVersion(t, db, rosa); after != before { + t.Errorf("row version = %s, want %s, the row holds no moved name", after, before) + } +} + func TestInheritedNamesMigrationClearsStrayValuesUnderTheNewName(t *testing.T) { t.Parallel() diff --git a/plugins/fields/migrations/00007_move_inherited_names.sql b/plugins/fields/migrations/00007_move_inherited_names.sql index b7ab8a46..01e54d7f 100644 --- a/plugins/fields/migrations/00007_move_inherited_names.sql +++ b/plugins/fields/migrations/00007_move_inherited_names.sql @@ -23,7 +23,12 @@ SET values = v.values SELECT jsonb_object_agg(m.moved, v.values -> m.inherited) FROM inherited_moves AS m WHERE m.tenant_id = v.tenant_id AND v.values ? m.inherited ), '{}'::jsonb) -WHERE v.tenant_id IN (SELECT m.tenant_id FROM inherited_moves AS m); +WHERE v.tenant_id IN (SELECT m.tenant_id FROM inherited_moves AS m) + AND v.values ?| ARRAY( + SELECT m.moved FROM inherited_moves AS m WHERE m.tenant_id = v.tenant_id + UNION ALL + SELECT m.inherited FROM inherited_moves AS m WHERE m.tenant_id = v.tenant_id + ); UPDATE plugin_fields.definitions AS d SET name = m.moved From 645500c3b0f0b243c00d5f0d1d7d64ac3fba9e80 Mon Sep 17 00:00:00 2001 From: SirLouen Date: Mon, 28 Sep 2026 11:23:09 +0200 Subject: [PATCH 21/25] test(fields): move two values of one contact off inherited names --- plugins/fields/migration_internal_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plugins/fields/migration_internal_test.go b/plugins/fields/migration_internal_test.go index 2f788a3b..6e248edf 100644 --- a/plugins/fields/migration_internal_test.go +++ b/plugins/fields/migration_internal_test.go @@ -340,6 +340,7 @@ func TestInheritedNamesMigrationMovesArchivedFieldsToo(t *testing.T) { home := sdk.TenantOrDefault(t.Context()) storedDefinition(t, db, home, "toString", "TEXT", "[]", true) storedDefinition(t, db, home, "hasOwnProperty", "BOOLEAN", "[]", false) + maria := contactHolding(t, db, home, `{"toString": "a", "hasOwnProperty": true}`) moveInheritedNames(t, provider) @@ -347,6 +348,10 @@ func TestInheritedNamesMigrationMovesArchivedFieldsToo(t *testing.T) { if held := definitionsIn(t, db, home); !reflect.DeepEqual(held, want) { t.Errorf("definitions = %v, want both moved, the archived one still archived", held) } + moved := decoded(t, `{"toString2": "a", "hasOwnProperty2": true}`) + if held := heldValues(t, db, maria); !reflect.DeepEqual(held, moved) { + t.Errorf("values = %#v, want both values moved with their fields", held) + } } // valuesVersion returns the row version of one contact's values row. From 61aa33db87e3cc6564a8c5a2c7a67c9f7a0c49bf Mon Sep 17 00:00:00 2001 From: SirLouen Date: Mon, 28 Sep 2026 11:23:10 +0200 Subject: [PATCH 22/25] fix(fields): keep the add form when a define never reaches the server --- plugins/fields/frontend/FieldsScreen.tsx | 6 ++++-- plugins/fields/frontend/test/fields.test.tsx | 13 +++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/plugins/fields/frontend/FieldsScreen.tsx b/plugins/fields/frontend/FieldsScreen.tsx index fd21c01a..4c426054 100644 --- a/plugins/fields/frontend/FieldsScreen.tsx +++ b/plugins/fields/frontend/FieldsScreen.tsx @@ -278,7 +278,7 @@ function labelHeld(label: string, live: FieldRow[]) { /** * Renders the form defining one new field, naming it from its label. - * @param props - The names a new field steps past and the reload run after every answer. + * @param props - The names a new field steps past and the reload run after every answer the server gives. * @returns The add field form. */ function AddFieldForm({ known, onAnswered }: { known: KnownNames; onAnswered: () => void }) { @@ -303,7 +303,9 @@ function AddFieldForm({ known, onAnswered }: { known: KnownNames; onAnswered: () const name = fieldName(label, known, refused) const sent = repeater ? namedSubFields(subFields) : undefined void define({ name, label, kind, subFields: sent }).then((result) => { - onAnswered() + if (!result.error?.networkError) { + onAnswered() + } if (!result.error) { setLabel('') setSubFields([]) diff --git a/plugins/fields/frontend/test/fields.test.tsx b/plugins/fields/frontend/test/fields.test.tsx index c3438b9e..53e95be7 100644 --- a/plugins/fields/frontend/test/fields.test.tsx +++ b/plugins/fields/frontend/test/fields.test.tsx @@ -343,6 +343,19 @@ test('a refused name is numbered on the next press', async () => { expect(defined.mock.calls.map((call) => call[0].name)).toEqual(['birthDate', 'birthDate2']) }) +test('a define lost on the way keeps the form and its draft', async () => { + serveFieldCatalogue([]) + server.use(graphql.mutation('DefineField', () => HttpResponse.error())) + const { graph } = fakeGraphClient() + + renderScreen(graph) + await defineLabelled('Anniversary') + + expect(await screen.findByRole('alert')).toHaveTextContent('The field could not be defined.') + expect(graph.refetch).not.toHaveBeenCalled() + expect(screen.getByLabelText('Label')).toHaveValue('Anniversary') +}) + test('the chosen kind is sent with the definition', async () => { serveFieldCatalogue([]) const defined = captureDefine() From 566e7b54d4f38d4b53742a3a8c5723e652647eac Mon Sep 17 00:00:00 2001 From: SirLouen Date: Mon, 28 Sep 2026 11:23:10 +0200 Subject: [PATCH 23/25] fix(fields): keep an earlier define failure hidden once a label is refused --- plugins/fields/frontend/FieldsScreen.tsx | 14 +++++++++----- plugins/fields/frontend/test/fields.test.tsx | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/plugins/fields/frontend/FieldsScreen.tsx b/plugins/fields/frontend/FieldsScreen.tsx index 4c426054..deb421ba 100644 --- a/plugins/fields/frontend/FieldsScreen.tsx +++ b/plugins/fields/frontend/FieldsScreen.tsx @@ -64,6 +64,9 @@ interface KnownNames { reserved: string[] } +/** FormNotice is the notice the add form shows: a label a live field holds, the last define refusal, or none. */ +type FormNotice = 'label' | 'define' | null + /** RACED are the reasons a define answers when another field took its name first. */ const RACED = new Set(['field_name_taken', 'field_kind_locked']) @@ -283,7 +286,7 @@ function labelHeld(label: string, live: FieldRow[]) { */ function AddFieldForm({ known, onAnswered }: { known: KnownNames; onAnswered: () => void }) { const [label, setLabel] = useState('') - const [labelTaken, setLabelTaken] = useState(false) + const [notice, setNotice] = useState(null) const [kind, setKind] = useState('TEXT') const [subFields, setSubFields] = useState([]) const [refused, setRefused] = useState([]) @@ -297,7 +300,7 @@ function AddFieldForm({ known, onAnswered }: { known: KnownNames; onAnswered: () onSubmit={(event) => { event.preventDefault() if (labelHeld(label, known.live)) { - setLabelTaken(true) + setNotice('label') return } const name = fieldName(label, known, refused) @@ -306,6 +309,7 @@ function AddFieldForm({ known, onAnswered }: { known: KnownNames; onAnswered: () if (!result.error?.networkError) { onAnswered() } + setNotice(result.error ? 'define' : null) if (!result.error) { setLabel('') setSubFields([]) @@ -315,10 +319,10 @@ function AddFieldForm({ known, onAnswered }: { known: KnownNames; onAnswered: () }) }} > - {labelTaken ? ( + {notice === 'label' ? ( {__('A field with that label already exists.', 'alphone-fields')} ) : null} - {defined.error && !labelTaken ? ( + {notice === 'define' && defined.error ? ( {validationMessage(graphError(defined.error), __('The field could not be defined.', 'alphone-fields'))} @@ -329,7 +333,7 @@ function AddFieldForm({ known, onAnswered }: { known: KnownNames; onAnswered: () value={label} onChange={(event) => { setLabel(event.target.value) - setLabelTaken(false) + setNotice((held) => (held === 'label' ? null : held)) }} /> { expect(screen.getByLabelText('Label')).toHaveValue('Anniversary') }) +test('a refused label drops an earlier failure for good', async () => { + serveFieldCatalogue([birthDate]) + server.use(graphql.mutation('DefineField', () => HttpResponse.json(refusal('VALIDATION', 'field_label_too_long')))) + + renderScreen() + await defineLabelled('Anniversary') + await screen.findByRole('alert') + await userEvent.clear(screen.getByLabelText('Label')) + await defineLabelled('Birth date') + await screen.findByText('A field with that label already exists.') + await userEvent.type(screen.getByLabelText('Label'), 's') + + expect(screen.queryByRole('alert')).not.toBeInTheDocument() +}) + test('the chosen kind is sent with the definition', async () => { serveFieldCatalogue([]) const defined = captureDefine() From 6bbd71171848e8dd2bd219cd2d7934a5dbc38dc3 Mon Sep 17 00:00:00 2001 From: SirLouen Date: Mon, 28 Sep 2026 11:23:11 +0200 Subject: [PATCH 24/25] test(fields): pin which refused names the add form numbers --- plugins/fields/frontend/test/fields.test.tsx | 34 +++++++++++++++++--- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/plugins/fields/frontend/test/fields.test.tsx b/plugins/fields/frontend/test/fields.test.tsx index 2d1a1004..3461c99e 100644 --- a/plugins/fields/frontend/test/fields.test.tsx +++ b/plugins/fields/frontend/test/fields.test.tsx @@ -324,23 +324,49 @@ test('the catalogue is read again after a refused define', async () => { await waitFor(() => expect(reads).toBe(2)) }) -test('a refused name is numbered on the next press', async () => { +test.each(['field_name_taken', 'field_kind_locked'])( + 'a name refused with %s is numbered on the next press', + async (reason) => { + serveFieldCatalogue([]) + const defined = vi.fn() + server.use( + graphql.mutation('DefineField', async ({ variables }) => { + defined(variables) + return HttpResponse.json(refusal('CONFLICT', reason)) + }), + ) + + renderScreen() + await submitField() + await screen.findByRole('alert') + await userEvent.click(screen.getByRole('button', { name: 'Add field' })) + + await waitFor(() => expect(defined).toHaveBeenCalledTimes(2)) + expect(defined.mock.calls.map((call) => call[0].name)).toEqual(['birthDate', 'birthDate2']) + }, +) + +test('a refusal that is not a race sends the same name again', async () => { serveFieldCatalogue([]) const defined = vi.fn() server.use( graphql.mutation('DefineField', async ({ variables }) => { defined(variables) - return HttpResponse.json(refusal('CONFLICT', 'field_name_taken')) + return HttpResponse.json( + defined.mock.calls.length === 1 + ? refusal('VALIDATION', 'field_label_too_long') + : { data: { defineField: birthDate } }, + ) }), ) renderScreen() await submitField() - await waitFor(() => expect(defined).toHaveBeenCalledTimes(1)) + await screen.findByRole('alert') await userEvent.click(screen.getByRole('button', { name: 'Add field' })) await waitFor(() => expect(defined).toHaveBeenCalledTimes(2)) - expect(defined.mock.calls.map((call) => call[0].name)).toEqual(['birthDate', 'birthDate2']) + expect(defined.mock.calls.map((call) => call[0].name)).toEqual(['birthDate', 'birthDate']) }) test('a define lost on the way keeps the form and its draft', async () => { From 0656df2b2789cd3adafac53a5c7488a1d0b70aa9 Mon Sep 17 00:00:00 2001 From: SirLouen Date: Mon, 28 Sep 2026 15:59:33 +0200 Subject: [PATCH 25/25] test(fields): document the Fields screen test helpers --- plugins/fields/frontend/test/fields.test.tsx | 32 ++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/plugins/fields/frontend/test/fields.test.tsx b/plugins/fields/frontend/test/fields.test.tsx index 3461c99e..e5f41bb5 100644 --- a/plugins/fields/frontend/test/fields.test.tsx +++ b/plugins/fields/frontend/test/fields.test.tsx @@ -33,6 +33,11 @@ const birthDate = { subFields: [], } +/** + * Renders the Fields screen inside its graph and query providers. + * @param graph - The graph client the screen reads through. + * @returns The render result. + */ function renderScreen(graph = fakeGraphClient().graph) { const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) return render( @@ -44,6 +49,12 @@ function renderScreen(graph = fakeGraphClient().graph) { ) } +/** + * Answers the Fields screen catalogue with the given fields and reserved names. + * @param live - The live fields. + * @param archived - The archived fields. + * @param reserved - The names the server refuses. + */ function serveFieldCatalogue(live: unknown[], archived: unknown[] = [], reserved: string[] = RESERVED) { server.use( graphql.query('FieldCatalogue', () => @@ -52,6 +63,11 @@ function serveFieldCatalogue(live: unknown[], archived: unknown[] = [], reserved ) } +/** + * Answers every define with the given field and records the variables it was sent. + * @param answer - The field the define answers. + * @returns The recorder of the variables. + */ function captureDefine(answer: unknown = birthDate) { const defined = vi.fn() server.use( @@ -63,11 +79,20 @@ function captureDefine(answer: unknown = birthDate) { return defined } +/** + * Types a label into the add form and presses Add field. + * @param label - The label to type. + */ async function defineLabelled(label: string) { await userEvent.type(await screen.findByLabelText('Label'), label) await userEvent.click(screen.getByRole('button', { name: 'Add field' })) } +/** + * Returns the name the only define sent. + * @param defined - The recorder of the define variables. + * @returns The name, once the define was sent. + */ async function sentName(defined: ReturnType) { await waitFor(() => expect(defined).toHaveBeenCalledTimes(1)) return defined.mock.calls[0][0].name @@ -259,6 +284,7 @@ test('a second visit reads the catalogue from the server again', async () => { await waitFor(() => expect(reads).toBe(2)) }) +/** Defines Birth date from the add form of an empty catalogue. */ async function submitField() { await screen.findByText(/No fields yet/i) await defineLabelled('Birth date') @@ -513,6 +539,7 @@ const history = { ], } +/** Starts a repeater labelled History in the add form of an empty catalogue. */ async function startRepeater() { await screen.findByText(/No fields yet/i) await userEvent.type(await screen.findByLabelText('Label'), 'History') @@ -520,6 +547,11 @@ async function startRepeater() { await userEvent.click(await screen.findByRole('option', { name: 'Repeater' })) } +/** + * Adds one sub field to the repeater the add form holds. + * @param label - The sub field label. + * @param kind - The kind option to choose. + */ async function addSubField(label: string, kind: string) { await userEvent.click(screen.getByRole('button', { name: 'Add sub field' })) const rows = screen.getAllByRole('group', { name: /^Sub field \d+$/ })