From bd970960c4953624cb6dba107334cec5ad0d8a8a Mon Sep 17 00:00:00 2001 From: Phillip Cloud <417981+cpcloud@users.noreply.github.com> Date: Tue, 21 Apr 2026 11:25:14 -0400 Subject: [PATCH 1/4] fix(ux): make magic-add work in document drill-downs Shift+A (magic-add) dispatch checked tab.Kind == tabDocuments, but drill-down tabs inherit the parent tab's Kind (e.g. tabAppliances for Appliances > Documents), so the keypress fell through silently. The "Press o to open" hint had the same anti-pattern. Route both through Tab.isDocumentTab(), which already consults the handler's FormKind() and covers both top-level and entity-scoped document tabs. When invoked inside a drill-down, the quick form is pre-populated with the parent entity so the created document is correctly attached, and acceptDeferredExtraction no longer lets the LLM override a user-chosen scope. Reproduction 1. Open a house with at least one Appliance and one document. 2. Navigate to Appliances, enter, and drill into Documents for any appliance. 3. Press i to enter edit mode, then Shift+A. 4. Before: nothing happens. After: the quick-add form opens, pre-scoped to the appliance. --- internal/app/extraction.go | 14 +- internal/app/forms.go | 9 ++ internal/app/handlers.go | 8 ++ internal/app/magic_add_drilldown_test.go | 165 +++++++++++++++++++++++ internal/app/model_keys.go | 11 +- 5 files changed, 200 insertions(+), 7 deletions(-) create mode 100644 internal/app/magic_add_drilldown_test.go diff --git a/internal/app/extraction.go b/internal/app/extraction.go index a1dbcedd..24a4964a 100644 --- a/internal/app/extraction.go +++ b/internal/app/extraction.go @@ -874,14 +874,20 @@ func (m *Model) acceptDeferredExtraction() error { doc := ex.pendingDoc // Apply fields from "create documents" operations to the pending doc. + // When the pending doc already has an entity scope (set by magic-add + // in an entity drill-down), preserve it: the user's explicit scope + // wins over any LLM guess. + preScoped := doc.EntityKind != "" for _, op := range ex.operations { if op.Table == tableDocuments { applyStringField(op.Data, "title", &doc.Title) applyStringField(op.Data, "notes", &doc.Notes) - applyStringField(op.Data, "entity_kind", &doc.EntityKind) - if v, ok := op.Data["entity_id"]; ok { - if n := extract.ParseStringID(v); n != "" { - doc.EntityID = n + if !preScoped { + applyStringField(op.Data, "entity_kind", &doc.EntityKind) + if v, ok := op.Data["entity_id"]; ok { + if n := extract.ParseStringID(v); n != "" { + doc.EntityID = n + } } } } diff --git a/internal/app/forms.go b/internal/app/forms.go index 970ed9f6..e99e947a 100644 --- a/internal/app/forms.go +++ b/internal/app/forms.go @@ -2395,8 +2395,17 @@ func (m *Model) startDocumentForm(entityKind string) error { // startQuickDocumentForm opens a minimal document form that only asks for a // file path. Title and notes are auto-filled by the extraction pipeline on // submit, making this the fast path for ingesting files. +// +// When invoked from an entity-scoped document drill-down, the parent +// entity is pre-populated so the created document is attached to that +// entity rather than left unscoped. func (m *Model) startQuickDocumentForm() { values := &documentFormData{DeferCreate: true} + if tab := m.effectiveTab(); tab != nil { + if sh, ok := tab.Handler.(scopedHandler); ok && sh.entityKind != "" { + values.EntityRef = entityRef{Kind: sh.entityKind, ID: sh.entityID} + } + } form := huh.NewForm( huh.NewGroup( m.newDocumentFilePicker("File to attach"). diff --git a/internal/app/handlers.go b/internal/app/handlers.go index 3f3d964c..9e643cc3 100644 --- a/internal/app/handlers.go +++ b/internal/app/handlers.go @@ -353,6 +353,12 @@ type scopedHandler struct { inlineEditFn func(*Model, string, int) error // nil = TabHandler.InlineEdit startAddFn func(*Model) error // nil = TabHandler.StartAddForm submitFn func(*Model) error // nil = TabHandler.SubmitForm + + // Document scope (set only by newEntityDocumentHandler). Exposed so + // flows like magic-add can pre-populate the entity on a quick form + // when the active handler represents a scoped document view. + entityKind string + entityID string } func (s scopedHandler) Load( @@ -633,5 +639,7 @@ func newEntityDocumentHandler(entityKind string, entityID string) scopedHandler submitFn: func(m *Model) error { return m.submitScopedDocumentForm(entityKind, entityID) }, + entityKind: entityKind, + entityID: entityID, } } diff --git a/internal/app/magic_add_drilldown_test.go b/internal/app/magic_add_drilldown_test.go new file mode 100644 index 00000000..4a57ac8d --- /dev/null +++ b/internal/app/magic_add_drilldown_test.go @@ -0,0 +1,165 @@ +// Copyright 2026 Phillip Cloud +// Licensed under the Apache License, Version 2.0 + +package app + +import ( + "testing" + + "github.com/micasa-dev/micasa/internal/data" + "github.com/micasa-dev/micasa/internal/extract" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestMagicAddWorksInDocumentDrilldowns verifies that pressing Shift+A +// (magic-add / QuickAdd) inside any document drill-down opens the +// deferred-create document form, matching behavior on the top-level +// Documents tab. Without the fix, the dispatch falls through because the +// drill-down's Tab.Kind is the parent tab's kind (e.g. tabAppliances), +// not tabDocuments. +func TestMagicAddWorksInDocumentDrilldowns(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + entityKind string + openDetail func(t *testing.T, m *Model) string // returns the scoped entity ID + }{ + { + name: "project", + entityKind: data.DocumentEntityProject, + openDetail: func(t *testing.T, m *Model) string { + t.Helper() + types, err := m.store.ProjectTypes() + require.NoError(t, err) + require.NoError(t, m.store.CreateProject(&data.Project{ + Title: "Magic Drill Proj", + ProjectTypeID: types[0].ID, + Status: data.ProjectStatusPlanned, + })) + projects, err := m.store.ListProjects(false) + require.NoError(t, err) + require.NotEmpty(t, projects) + require.NoError(t, m.openProjectDocumentDetail(projects[0].ID, "Magic Drill Proj")) + return projects[0].ID + }, + }, + { + name: "appliance", + entityKind: data.DocumentEntityAppliance, + openDetail: func(t *testing.T, m *Model) string { + t.Helper() + require.NoError( + t, + m.store.CreateAppliance(&data.Appliance{Name: "Magic Dishwasher"}), + ) + appls, err := m.store.ListAppliances(false) + require.NoError(t, err) + require.NotEmpty(t, appls) + require.NoError(t, m.openApplianceDocumentDetail(appls[0].ID, "Magic Dishwasher")) + return appls[0].ID + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + m := newTestModelWithStore(t) + entityID := tc.openDetail(t, m) + require.True(t, m.inDetail()) + require.True( + t, + m.effectiveTab().isDocumentTab(), + "drill-down tab should be detected as a document tab", + ) + + m.enterEditMode() + require.Equal(t, modeEdit, m.mode) + + sendKey(m, keyShiftA) + assert.Equal(t, modeForm, m.mode, + "Shift+A in docs drill-down should open the quick-add form") + + fd, ok := m.fs.formData.(*documentFormData) + require.True(t, ok, "form data should be documentFormData") + assert.True(t, fd.DeferCreate, "quick-add form must set DeferCreate") + assert.Equal(t, tc.entityKind, fd.EntityRef.Kind, + "quick-add in drill-down must pre-scope to the parent entity kind") + assert.Equal(t, entityID, fd.EntityRef.ID, + "quick-add in drill-down must pre-scope to the parent entity ID") + }) + } +} + +// TestMagicAddOnTopLevelDocumentsTabLeavesScopeEmpty ensures the fix for +// drill-down parity does not regress the top-level Documents tab, where +// the entity is chosen by the user (or by LLM extraction) rather than +// pre-scoped. +func TestMagicAddOnTopLevelDocumentsTabLeavesScopeEmpty(t *testing.T) { + t.Parallel() + m := newTestModelWithStore(t) + m.active = tabIndex(tabDocuments) + require.False(t, m.inDetail()) + + m.enterEditMode() + sendKey(m, keyShiftA) + require.Equal(t, modeForm, m.mode) + + fd, ok := m.fs.formData.(*documentFormData) + require.True(t, ok) + assert.True(t, fd.DeferCreate) + assert.Empty(t, fd.EntityRef.Kind, "top-level quick-add must not pre-scope") + assert.Empty(t, fd.EntityRef.ID, "top-level quick-add must not pre-scope") +} + +// TestAcceptDeferredExtraction_PreservesPreScopedEntity ensures that +// when magic-add is triggered in a drill-down (so the pending doc +// already has EntityKind/EntityID set), the subsequent extraction +// accept does NOT let the LLM override the user-chosen scope. +func TestAcceptDeferredExtraction_PreservesPreScopedEntity(t *testing.T) { + t.Parallel() + m := newTestModelWithStore(t) + require.NoError(t, m.store.CreateAppliance(&data.Appliance{Name: "Scoped Appl"})) + appls, err := m.store.ListAppliances(false) + require.NoError(t, err) + require.NotEmpty(t, appls) + applID := appls[0].ID + + ex := &extractionLogState{ + ID: 42, + Visible: true, + Done: true, + toolCursor: -1, + expanded: make(map[extractionStep]bool), + pendingDoc: &data.Document{ + FileName: "manual.pdf", + MIMEType: "application/pdf", + Data: []byte("pdf-bytes"), + EntityKind: data.DocumentEntityAppliance, + EntityID: applID, + }, + } + // LLM proposes a different entity; the drill-down scope must win. + ex.operations = []extract.Operation{ + {Action: "create", Table: data.TableDocuments, Data: map[string]any{ + "title": "Dishwasher Manual", + "notes": "warranty info", + "entity_kind": data.DocumentEntityVendor, + "entity_id": "01JNOTEXIST0000000VENDOR", + }}, + } + m.ex.extraction = ex + + require.NoError(t, m.acceptDeferredExtraction()) + + docs, err := m.store.ListDocuments(false) + require.NoError(t, err) + require.Len(t, docs, 1) + assert.Equal(t, "Dishwasher Manual", docs[0].Title, "LLM title should still apply") + assert.Equal(t, data.DocumentEntityAppliance, docs[0].EntityKind, + "pre-scoped entity kind must survive LLM override") + assert.Equal(t, applID, docs[0].EntityID, + "pre-scoped entity ID must survive LLM override") +} diff --git a/internal/app/model_keys.go b/internal/app/model_keys.go index 9310aa75..73a7ac56 100644 --- a/internal/app/model_keys.go +++ b/internal/app/model_keys.go @@ -447,8 +447,10 @@ func (m *Model) handleNormalEnter() error { return nil } - // On the Documents tab, hint at the open-file shortcut. - if tab.Kind == tabDocuments { + // On any document tab (top-level or drill-down), hint at the open-file + // shortcut. Drill-down tabs inherit the parent's Kind, so rely on the + // semantic helper rather than a literal Kind comparison. + if tab.isDocumentTab() { m.setStatusInfo("Press o to open.") return nil } @@ -464,7 +466,10 @@ func (m *Model) handleEditKeys(msg tea.KeyPressMsg) (tea.Cmd, bool) { m.startAddForm() return m.formInitCmd(), true case key.Matches(msg, m.keys.QuickAdd): - if tab := m.effectiveTab(); tab != nil && tab.Kind == tabDocuments { + // Magic-add works on any document tab, including entity-scoped + // drill-downs (Appliances > Docs, Projects > Docs, etc.), which + // inherit the parent tab's Kind. + if tab := m.effectiveTab(); tab != nil && tab.isDocumentTab() { m.startQuickDocumentForm() return m.formInitCmd(), true } From be0e65fe868c3e03de0ab90e087590a24fa76c98 Mon Sep 17 00:00:00 2001 From: Phillip Cloud <417981+cpcloud@users.noreply.github.com> Date: Tue, 21 Apr 2026 11:25:25 -0400 Subject: [PATCH 2/4] docs(codebase-map): document drill-down Tab.Kind inheritance Drill-down tabs inherit the parent's Kind rather than the semantic entity Kind. Checks like tab.Kind == tabDocuments silently break parity; go through Tab.isDocumentTab() or the handler's FormKind() instead. Refresh the verified date. --- .claude/codebase/patterns.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.claude/codebase/patterns.md b/.claude/codebase/patterns.md index e483d317..02d291d8 100644 --- a/.claude/codebase/patterns.md +++ b/.claude/codebase/patterns.md @@ -1,6 +1,6 @@ - + # Code Patterns & Conventions @@ -18,6 +18,14 @@ Each entity tab implements TabHandler interface. All entity-specific logic (Load, Delete, StartAddForm, SubmitForm, etc.) lives in the handler. No scattered FormKind/TabKind switches outside the handler. +### Drill-down Tab Kind Inheritance +Detail (drill-down) tabs inherit the parent tab's Kind, not the semantic +entity Kind (see `openDetailFromDef` in `model_tabs.go`). E.g. an +Appliance > Documents drill-down has `Tab.Kind = tabAppliances`, not +`tabDocuments`. Always identify entity semantics via the Handler's +`FormKind()` (or a helper like `Tab.isDocumentTab()`) -- never +`tab.Kind == tabX`, which silently breaks drill-down parity. + ### Rendering Pipeline ``` View() -> buildView() From d91778e20caf9f06ffcba7f48a795386494f49f6 Mon Sep 17 00:00:00 2001 From: Phillip Cloud <417981+cpcloud@users.noreply.github.com> Date: Tue, 21 Apr 2026 11:37:01 -0400 Subject: [PATCH 3/4] fix(ux): dispatch drill-down hard-delete and settled-filter by FormKind promptHardDelete and the confirm-prompt dispatch keyed on tab.Kind, so Shift+D did nothing in the Appliances > Maintenance drill-down (whose Kind is tabAppliances, inherited from the parent). The same bug affected the "Permanently delete this item/incident?" label and the soft-delete status messages. Route all entity-semantic checks through handlerFormKind(tab), a new helper that reads tab.Handler.FormKind() and maps cleanly onto formIncident, formMaintenance, etc. This keeps drill-downs functionally identical to their parent tabs. toggleSettledFilter gets the same treatment: drop the blanket m.inDetail() early-return and gate on FormKind == formProject via m.effectiveTab() so any future project-scoped drill-down inherits the filter automatically. Reproduction 1. Open a house with at least one appliance. 2. Add a maintenance item scoped to that appliance. 3. Drill into Appliances > Maintenance, press i, then d to soft-delete, then Shift+D and y. 4. Before: prompt said "incident", and Yes hard-deleted an incident (no-op because no IDs matched), leaving the maintenance item intact. After: prompt says "item", and Yes permanently deletes the item. --- internal/app/detail_openers_test.go | 5 +- internal/app/drilldown_parity_test.go | 104 ++++++++++++++++++++++++++ internal/app/model.go | 10 +-- internal/app/model_status.go | 30 ++++++-- internal/app/view.go | 2 +- 5 files changed, 133 insertions(+), 18 deletions(-) create mode 100644 internal/app/drilldown_parity_test.go diff --git a/internal/app/detail_openers_test.go b/internal/app/detail_openers_test.go index a608b00e..04ddc73a 100644 --- a/internal/app/detail_openers_test.go +++ b/internal/app/detail_openers_test.go @@ -20,10 +20,7 @@ func (m *Model) openServiceLogDetail(maintID string, maintName string) error { return m.openDetailFromDef(serviceLogDef, maintID, maintName) } -func (m *Model) openApplianceMaintenanceDetail( - applianceID string, //nolint:unparam // signature mirrors the other openXxx helpers; tests always pass the same fixture ID - applianceName string, //nolint:unparam // signature mirrors the other openXxx helpers; tests always pass the same fixture name -) error { +func (m *Model) openApplianceMaintenanceDetail(applianceID string, applianceName string) error { return m.openDetailFromDef(applianceMaintenanceDef, applianceID, applianceName) } diff --git a/internal/app/drilldown_parity_test.go b/internal/app/drilldown_parity_test.go new file mode 100644 index 00000000..aa0d3163 --- /dev/null +++ b/internal/app/drilldown_parity_test.go @@ -0,0 +1,104 @@ +// Copyright 2026 Phillip Cloud +// Licensed under the Apache License, Version 2.0 + +package app + +import ( + "testing" + + "github.com/micasa-dev/micasa/internal/data" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestHardDeleteWorksInMaintenanceDrilldown verifies that Shift+D +// hard-deletes a soft-deleted maintenance item from the +// Appliances > Maintenance drill-down the same way it does from the +// top-level Maintenance tab. Without the fix, both the promptHardDelete +// gate and the confirmHardDelete dispatch keyed on tab.Kind, which in +// the drill-down is tabAppliances -- silently routing or blocking. +func TestHardDeleteWorksInMaintenanceDrilldown(t *testing.T) { + t.Parallel() + m := newTestModelWithStore(t) + + // Set up appliance + one maintenance item scoped to it. + require.NoError(t, m.store.CreateAppliance(&data.Appliance{Name: "Furnace"})) + appls, err := m.store.ListAppliances(false) + require.NoError(t, err) + require.NotEmpty(t, appls) + applID := appls[0].ID + + cats, err := m.store.MaintenanceCategories() + require.NoError(t, err) + require.NotEmpty(t, cats) + require.NoError(t, m.store.CreateMaintenance(&data.MaintenanceItem{ + Name: "Replace filter", + CategoryID: cats[0].ID, + ApplianceID: &applID, + })) + items, err := m.store.ListMaintenanceByAppliance(applID, false) + require.NoError(t, err) + require.Len(t, items, 1) + itemID := items[0].ID + + // Open Appliance > Maintenance drill-down. + require.NoError(t, m.openApplianceMaintenanceDetail(applID, "Furnace")) + require.True(t, m.inDetail()) + tab := m.effectiveTab() + require.NotNil(t, tab) + assert.Equal(t, formMaintenance, tab.Handler.FormKind(), + "drill-down handler must identify as maintenance via FormKind") + + // Select the row, enter edit mode. + require.NotEmpty(t, tab.Rows) + sendKey(m, "i") + require.Equal(t, modeEdit, m.mode) + + // Shift+D on a live row must surface the maintenance-specific message. + sendKey(m, "D") + assert.NotEqual(t, confirmHardDelete, m.confirm, + "Shift+D on live row should not prompt hard-delete") + assert.Contains(t, m.statusView(), "Delete the item first", + "message must use 'item' (maintenance), not 'incident'") + + // Soft-delete, then hard-delete. + sendKey(m, "d") + require.NoError(t, m.reloadEffectiveTab()) + + sendKey(m, "D") + assert.Equal(t, confirmHardDelete, m.confirm, + "Shift+D on soft-deleted row should prompt hard-delete in drill-down") + assert.Contains(t, m.statusView(), "Permanently delete this item?", + "prompt label must say 'item' in a maintenance drill-down") + + sendKey(m, "y") + assert.Equal(t, confirmNone, m.confirm) + assert.Contains(t, m.statusView(), "Permanently deleted") + + // Row must be gone from the store. + _, err = m.store.GetMaintenance(itemID) + assert.Error(t, err, "maintenance item must be hard-deleted from the store") +} + +// TestToggleSettledFilterIdentifiesProjectTabByFormKind ensures the +// settled-filter no longer key-cases on tab.Kind, so future project +// drill-downs (none exists today) would inherit the feature and +// non-project drill-downs continue to be no-ops. +func TestToggleSettledFilterIdentifiesProjectTabByFormKind(t *testing.T) { + t.Parallel() + m := newTestModelWithStore(t) + + // Top-level Projects tab: toggle should succeed. + m.active = tabIndex(tabProjects) + require.NoError(t, m.reloadActiveTab()) + assert.True(t, m.toggleSettledFilter(), + "top-level Projects must still respond to settled filter") + + // Non-project drill-down: toggle must be a no-op. + require.NoError(t, m.store.CreateAppliance(&data.Appliance{Name: "Toggle Test"})) + appls, err := m.store.ListAppliances(false) + require.NoError(t, err) + require.NoError(t, m.openApplianceMaintenanceDetail(appls[0].ID, "Toggle Test")) + assert.False(t, m.toggleSettledFilter(), + "settled filter must not fire in a non-project drill-down") +} diff --git a/internal/app/model.go b/internal/app/model.go index 9f54afe3..07829d71 100644 --- a/internal/app/model.go +++ b/internal/app/model.go @@ -566,11 +566,11 @@ var activeProjectStatuses = []string{ } func (m *Model) toggleSettledFilter() bool { - if m.inDetail() { - return false - } - tab := m.activeTab() - if tab == nil || tab.Kind != tabProjects { + // Identify project tabs by handler FormKind so any future + // project-scoped drill-down inherits the filter automatically and + // non-project drill-downs continue to no-op. + tab := m.effectiveTab() + if handlerFormKind(tab) != formProject { return false } col := statusColumnIndex(tab.Specs) diff --git a/internal/app/model_status.go b/internal/app/model_status.go index 5d204f5b..8674a83a 100644 --- a/internal/app/model_status.go +++ b/internal/app/model_status.go @@ -108,7 +108,7 @@ func (m *Model) toggleDeleteSelected() { if tab.LastDeleted != nil && *tab.LastDeleted == meta.ID { tab.LastDeleted = nil } - if tab.Kind == tabIncidents { + if handlerFormKind(tab) == formIncident { m.setStatusInfo("Reopened.") } else { m.setStatusInfo("Restored.") @@ -124,7 +124,7 @@ func (m *Model) toggleDeleteSelected() { if !tab.showDeletedExplicit { tab.ShowDeleted = true } - if tab.Kind == tabIncidents { + if handlerFormKind(tab) == formIncident { m.setStatusInfo("Resolved. Press d to reopen.") } else { m.setStatusInfo("Deleted. Press d to restore.") @@ -132,17 +132,31 @@ func (m *Model) toggleDeleteSelected() { m.surfaceError(m.reloadEffectiveTab()) } +// handlerFormKind returns the FormKind of a tab's handler, or formNone +// when the tab or its handler is nil. Prefer this over tab.Kind when +// identifying entity semantics, because drill-down tabs inherit the +// parent tab's Kind rather than the entity's kind. +func handlerFormKind(tab *Tab) FormKind { + if tab == nil || tab.Handler == nil { + return formNone + } + return tab.Handler.FormKind() +} + func (m *Model) promptHardDelete() { tab := m.effectiveTab() if tab == nil { return } - switch tab.Kind { - case tabIncidents, tabMaintenance: - case tabProjects, tabQuotes, tabAppliances, tabVendors, tabDocuments: + kind := handlerFormKind(tab) + switch kind { + case formIncident, formMaintenance: + // hard-deletable: continue + case formNone, formHouse, formProject, formQuote, formAppliance, + formServiceLog, formVendor, formDocument: return default: - panic(fmt.Sprintf("unhandled TabKind: %d", tab.Kind)) + panic(fmt.Sprintf("unhandled FormKind: %d", kind)) } meta, ok := m.selectedRowMeta() if !ok { @@ -150,7 +164,7 @@ func (m *Model) promptHardDelete() { return } if !meta.Deleted { - if tab.Kind == tabIncidents { + if kind == formIncident { m.setStatusError("Resolve the incident first (d), then permanently delete (D).") } else { m.setStatusError("Delete the item first (d), then permanently delete (D).") @@ -167,7 +181,7 @@ func (m *Model) handleConfirmHardDelete(msg tea.KeyPressMsg) { m.confirm = confirmNone tab := m.effectiveTab() var err error - if tab != nil && tab.Kind == tabMaintenance { + if handlerFormKind(tab) == formMaintenance { err = m.store.HardDeleteMaintenance(m.hardDeleteID) } else { err = m.store.HardDeleteIncident(m.hardDeleteID) diff --git a/internal/app/view.go b/internal/app/view.go index 2104eea0..686415d0 100644 --- a/internal/app/view.go +++ b/internal/app/view.go @@ -283,7 +283,7 @@ func (m *Model) statusView() string { } if m.confirm == confirmHardDelete { entity := "incident" - if tab := m.effectiveTab(); tab != nil && tab.Kind == tabMaintenance { + if handlerFormKind(m.effectiveTab()) == formMaintenance { entity = "item" } prompt := m.styles.FormDirty().Render("Permanently delete this " + entity + "?") From f43be28bc0da7d0f3e1beebcc19a4f2d305347a2 Mon Sep 17 00:00:00 2001 From: Phillip Cloud <417981+cpcloud@users.noreply.github.com> Date: Wed, 22 Apr 2026 20:22:14 -0400 Subject: [PATCH 4/4] fix(ux): pre-scope add forms in scoped drill-downs Pressing "a" in Appliance > Maintenance, Project > Quotes, and Vendor > Quotes used to open the generic add form and default the parent field to whichever row happened to sort first in its list. Users had to re-select the entity they just drilled into. Carry the drill-down context into the add form via a scoped startAddFn on each handler, backed by new startMaintenanceFormScoped and startQuoteFormScoped helpers that pre-populate the known parent when called with a non-empty ID/name. Reproduction 1. Create two appliances, Decoy and Parent. 2. Drill into Parent > Maintenance. 3. Press i, then a. 4. Before: form's Appliance select defaults to Decoy. After: it defaults to Parent. --- internal/app/drilldown_parity_test.go | 103 ++++++++++++++++++++++++++ internal/app/forms.go | 32 +++++++- internal/app/handlers.go | 14 ++++ 3 files changed, 148 insertions(+), 1 deletion(-) diff --git a/internal/app/drilldown_parity_test.go b/internal/app/drilldown_parity_test.go index aa0d3163..7678368e 100644 --- a/internal/app/drilldown_parity_test.go +++ b/internal/app/drilldown_parity_test.go @@ -102,3 +102,106 @@ func TestToggleSettledFilterIdentifiesProjectTabByFormKind(t *testing.T) { assert.False(t, m.toggleSettledFilter(), "settled filter must not fire in a non-project drill-down") } + +// TestApplianceMaintenanceDrilldownAddPrescopesAppliance ensures that +// pressing "a" in an Appliance > Maintenance drill-down pre-populates +// the parent appliance instead of defaulting to the first appliance in +// the store. Without the fix, the drill-down context is lost as soon +// as the add form opens. +func TestApplianceMaintenanceDrilldownAddPrescopesAppliance(t *testing.T) { + t.Parallel() + m := newTestModelWithStore(t) + require.NoError(t, m.store.CreateAppliance(&data.Appliance{Name: "Decoy First"})) + require.NoError(t, m.store.CreateAppliance(&data.Appliance{Name: "Parent Appliance"})) + require.NoError(t, m.loadLookups()) + + appls, err := m.store.ListAppliances(false) + require.NoError(t, err) + require.Len(t, appls, 2) + var parentID string + for _, a := range appls { + if a.Name == "Parent Appliance" { + parentID = a.ID + break + } + } + require.NotEmpty(t, parentID) + + require.NoError(t, m.openApplianceMaintenanceDetail(parentID, "Parent Appliance")) + m.enterEditMode() + sendKey(m, "a") + require.Equal(t, modeForm, m.mode) + + fd, ok := m.fs.formData.(*maintenanceFormData) + require.True(t, ok, "expected maintenance form data") + assert.Equal(t, parentID, fd.ApplianceID, + "add form in Appliance > Maintenance must pre-scope to the drilled-into appliance") +} + +// TestProjectQuoteDrilldownAddPrescopesProject ensures Project > Quotes +// drill-down pre-selects the parent project on the quote add form. +// Two projects are created so the parent is NOT the default (most +// recently updated) project returned by ListProjects. +func TestProjectQuoteDrilldownAddPrescopesProject(t *testing.T) { + t.Parallel() + m := newTestModelWithStore(t) + types, err := m.store.ProjectTypes() + require.NoError(t, err) + require.NoError(t, m.store.CreateProject(&data.Project{ + Title: "Parent Project", ProjectTypeID: types[0].ID, Status: data.ProjectStatusPlanned, + })) + // Create the decoy AFTER the parent so it sorts ahead in + // ListProjects (order: updated_at desc), ensuring the default + // options[0] is the decoy and the test must rely on pre-scoping. + require.NoError(t, m.store.CreateProject(&data.Project{ + Title: "Decoy Project", ProjectTypeID: types[0].ID, Status: data.ProjectStatusPlanned, + })) + projects, err := m.store.ListProjects(false) + require.NoError(t, err) + require.Len(t, projects, 2) + var parentID string + for _, p := range projects { + if p.Title == "Parent Project" { + parentID = p.ID + break + } + } + require.NotEmpty(t, parentID) + + require.NoError(t, m.openProjectQuoteDetail(parentID, "Parent Project")) + m.enterEditMode() + sendKey(m, "a") + require.Equal(t, modeForm, m.mode) + + fd, ok := m.fs.formData.(*quoteFormData) + require.True(t, ok, "expected quote form data") + assert.Equal(t, parentID, fd.ProjectID, + "add form in Project > Quotes must pre-scope to the drilled-into project") +} + +// TestVendorQuoteDrilldownAddPrescopesVendor ensures Vendor > Quotes +// drill-down pre-fills the vendor name on the quote add form. +func TestVendorQuoteDrilldownAddPrescopesVendor(t *testing.T) { + t.Parallel() + m := newTestModelWithStore(t) + types, err := m.store.ProjectTypes() + require.NoError(t, err) + require.NoError(t, m.store.CreateProject(&data.Project{ + Title: "Any Project", ProjectTypeID: types[0].ID, Status: data.ProjectStatusPlanned, + })) + require.NoError(t, m.store.CreateVendor(&data.Vendor{Name: "Parent Vendor"})) + vendors, err := m.store.ListVendors(false) + require.NoError(t, err) + require.NotEmpty(t, vendors) + parentID := vendors[0].ID + + require.NoError(t, m.openVendorQuoteDetail(parentID, "Parent Vendor")) + m.enterEditMode() + sendKey(m, "a") + require.Equal(t, modeForm, m.mode) + + fd, ok := m.fs.formData.(*quoteFormData) + require.True(t, ok, "expected quote form data") + assert.Equal(t, "Parent Vendor", fd.VendorName, + "add form in Vendor > Quotes must pre-fill the drilled-into vendor name") +} diff --git a/internal/app/forms.go b/internal/app/forms.go index e99e947a..1f977ed3 100644 --- a/internal/app/forms.go +++ b/internal/app/forms.go @@ -316,6 +316,13 @@ func (m *Model) openProjectForm(values *projectFormData, options []huh.Option[st } func (m *Model) startQuoteForm() error { + return m.startQuoteFormScoped("", "") +} + +// startQuoteFormScoped opens the add-quote form with optional pre-scope +// for drill-down contexts. Empty strings mean "no pre-scope" and the +// form defaults to the first option (project) / blank (vendor name). +func (m *Model) startQuoteFormScoped(projectID, vendorName string) error { projects, err := m.store.ListProjects(false) if err != nil { return err @@ -323,9 +330,17 @@ func (m *Model) startQuoteForm() error { if len(projects) == 0 { return errors.New("add a project before adding quotes") } - values := "eFormData{} + values := "eFormData{VendorName: vendorName} options := projectOptions(projects) values.ProjectID = options[0].Value + if projectID != "" { + for _, opt := range options { + if opt.Value == projectID { + values.ProjectID = projectID + break + } + } + } form := huh.NewForm( huh.NewGroup( huh.NewSelect[string](). @@ -422,6 +437,13 @@ func scheduleTypeOptions() []huh.Option[scheduleType] { } func (m *Model) startMaintenanceForm() error { + return m.startMaintenanceFormScoped("") +} + +// startMaintenanceFormScoped opens the add-maintenance form with an +// optional appliance pre-selected. An empty applianceID means "no +// pre-scope" and the form defaults to no appliance. +func (m *Model) startMaintenanceFormScoped(applianceID string) error { values := &maintenanceFormData{ScheduleType: schedNone} catOptions := maintenanceOptions(m.maintenanceCategories) if len(catOptions) > 0 { @@ -432,6 +454,14 @@ func (m *Model) startMaintenanceForm() error { return fmt.Errorf("list appliances: %w", err) } appOpts := applianceOptions(appliances) + if applianceID != "" { + for _, opt := range appOpts { + if opt.Value == applianceID { + values.ApplianceID = applianceID + break + } + } + } form := huh.NewForm( huh.NewGroup( huh.NewInput(). diff --git a/internal/app/handlers.go b/internal/app/handlers.go index 9e643cc3..bed5f6a3 100644 --- a/internal/app/handlers.go +++ b/internal/app/handlers.go @@ -5,6 +5,7 @@ package app import ( "errors" + "fmt" "strings" "time" @@ -421,6 +422,9 @@ func newApplianceMaintenanceHandler(applianceID string) scopedHandler { return rows, meta, cellRows, nil }, inlineEditFn: skipColEdit(parent, int(maintenanceColAppliance)), // skip Appliance column + startAddFn: func(m *Model) error { + return m.startMaintenanceFormScoped(applianceID) + }, } } @@ -526,6 +530,13 @@ func newVendorQuoteHandler(vendorID string) scopedHandler { return rows, meta, cellRows, nil }, inlineEditFn: skipColEdit(parent, 2), // skip Vendor column + startAddFn: func(m *Model) error { + v, err := m.store.GetVendor(vendorID) + if err != nil { + return fmt.Errorf("load vendor: %w", err) + } + return m.startQuoteFormScoped("", v.Name) + }, } } @@ -578,6 +589,9 @@ func newProjectQuoteHandler(projectID string) scopedHandler { return rows, meta, cellRows, nil }, inlineEditFn: skipColEdit(parent, 1), // skip Project column + startAddFn: func(m *Model) error { + return m.startQuoteFormScoped(projectID, "") + }, } }