Skip to content

refactor: unify TUI tabs behind an entry interface; fix convention violations - #24

Open
lvlcn-t wants to merge 1 commit into
feat/tui-crud-credentialsfrom
refactor/tui-tabs-entry
Open

refactor: unify TUI tabs behind an entry interface; fix convention violations#24
lvlcn-t wants to merge 1 commit into
feat/tui-crud-credentialsfrom
refactor/tui-tabs-entry

Conversation

@lvlcn-t

@lvlcn-t lvlcn-t commented Jul 21, 2026

Copy link
Copy Markdown
Owner

Why

The CRUD series left a type-tag dispatch smell: four near-identical tab
types (browseTab, TenantsTab, ContextsTab, CredentialsTab) fed by a
12-method tabs.Manager interface no test ever mocked, with Tabs
re-discriminating the selected item's concrete kind in four separate
type-switches
. A generic details.Item routed through a switch on its
kind is neither polymorphism nor concrete typing.

What

Real polymorphism, driven by the item types:

  • entry interface (embeds list.Item + details.Item, adds
    name/blank/form/save/remove) — each item (TenantItem,
    ContextItem, CredentialItem) carries its own CRUD behaviour as
    methods, with compile-time var _ entry assertions. save switches on
    the submission intent internally, on the item.
  • activatable — the context-only "use" side-effect, type-asserted
    once (like io.WriterTo), replacing handleInteractiveSelect.
  • One Tab struct replaces the four tab types. Tabs holds a concrete
    *contexts.Manager (tabs.Manager interface deleted) and dispatches
    through entry methods with zero type-switches.

Net -219 lines.

Convention fixes (from the audit)

  • contexts: RenameContext/DeleteContext and the tenant/credential
    delete path wrap the existing ErrContext*/ErrTenant*/ErrCredential*
    sentinels with %w; renameAndRetarget takes the renameTarget struct
    instead of 5 params; entryTarget.deleteXremove.
  • form.Model stores the last validation error (not its text), exposes
    Err(); form tests assert errors.Is.
  • tui/tabs tests drop every assert.Contains on View()/status/
    err.Error(). Pure validators are unit-tested with errors.Is/
    require.Error; duplicate-create rejection asserts
    contexts.ErrTenantExists via a lastErr accessor.
  • Domain not-found error tests folded into a table using errors.Is.

Verification

  • go test -race ./..., go vet, full pre-commit — green.
  • Invariants grep-verified: no tabs.Manager interface; no concrete-item
    type-switch outside item files; no message-substring assertions;
    contexts still a leaf; tui never imports cmd.
  • CLI smoke test unaffected; behaviour preserved by the existing CRUD flow
    tests.

Based on feat/tui-crud-credentials (#23); reviewing the stack #20#23
first is recommended.

…olations

The tabs were four near-identical types (browseTab, TenantsTab,
ContextsTab, CredentialsTab) fed by a 12-method tabs.Manager interface no
test ever mocked, with Tabs re-discriminating the selected item's concrete
kind in four separate type-switches. That is a type-tag dispatch smell: a
generic item routed through a switch on its kind.

Replace it with real polymorphism:

- entry interface (embeds list.Item + details.Item, adds name/blank/form/
  save/remove): each item type (TenantItem, ContextItem, CredentialItem)
  carries its own CRUD behaviour as methods, with compile-time assertions.
  save switches on the submission intent internally, on the item.
- activatable: the optional context-only "use" side-effect, type-asserted
  once (like io.WriterTo), replacing handleInteractiveSelect.
- One Tab struct replaces the four tab types; Tabs holds a concrete
  *contexts.Manager (the tabs.Manager interface is deleted) and dispatches
  through entry methods with zero type-switches.

Also fixes the convention violations found in the CRUD series:

- contexts: RenameContext/DeleteContext and the tenant/credential delete
  path now wrap the existing ErrContext*/ErrTenant*/ErrCredential*
  sentinels with %w; renameAndRetarget takes the renameTarget struct
  instead of five params; entryTarget.deleteX renamed to remove.
- form.Model stores the last validation error (not its text) and exposes
  Err(); tests assert errors.Is instead of message substrings.
- tui/tabs tests drop every assert.Contains on View()/status/err.Error();
  the pure validators are unit-tested with errors.Is/require.Error, and
  the duplicate-create rejection asserts contexts.ErrTenantExists via a
  lastErr accessor.
- Domain error tests fold not-found singles into a table and use errors.Is.

Net -219 lines; behaviour unchanged (verified by the CRUD flow tests and a
CLI smoke test).

Signed-off-by: lvlcn-t <75443136+lvlcn-t@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown

File Coverage
All files 62%
az/azconfig.go 66%
az/client.go 0%
az/client.go 0%
az/commands.go 0%
az/secrets.go 0%
az/secrets.go 15%
cmd/context_view.go 85%
cmd/current.go 80%
cmd/delete_context.go 71%
cmd/get.go 56%
cmd/list.go 46%
cmd/output_flags.go 85%
cmd/rename_context.go 69%
cmd/set_context.go 69%
cmd/set_credential.go 97%
cmd/set_credential.go 80%
cmd/set_tenant.go 95%
cmd/set_tenant.go 57%
cmd/use.go 47%
cmd/view.go 95%
cmd/view.go 48%
config/config.go 91%
config/credential.go 0%
config/credential.go 64%
config/credential.go 70%
config/keyvault.go 14%
config/loader.go 82%
config/loader.go 79%
config/store.go 69%
config/writer.go 69%
contexts/contexts.go 92%
contexts/contexts.go 82%
internal/gendemo/main.go 0%
internal/gendoc/main.go 0%
keyvault/azure.go 0%
keyvault/azure.go 0%
keyvault/keyvault.go 75%
login/login.go 76%
main.go 0%
output/printer.go 79%
semver/version.go 94%
tui/app.go 0%
tui/app.go 0%
tui/tui.go 0%
tui/details/viewer.go 0%
tui/form/form.go 67%
tui/keys/builder.go 80%
tui/keys/keys.go 66%
tui/splash/splash.go 0%
tui/splash/splash.go 0%
tui/state/state.go 71%
tui/styles/azure_renderer.go 33%
tui/styles/azure_renderer.go 8%
tui/tabs/confirm.go 36%
tui/tabs/context_item.go 59%
tui/tabs/credential_items.go 92%
tui/tabs/credential_items.go 35%
tui/tabs/list_builder.go 83%
tui/tabs/tab.go 96%
tui/tabs/tab.go 53%
tui/tabs/tabs.go 51%
tui/tabs/tenant_items.go 67%
wif/factory/factory.go 0%
wif/oauth2/browser.go 0%
wif/oauth2/browser_linux.go 0%
wif/oauth2/cache.go 76%
wif/oauth2/cache.go 79%
wif/oauth2/oauth2.go 25%
wif/oauth2/oauth2.go 59%
wif/options.go 20%

Minimum allowed coverage is 70%

Generated by 🐒 cobertura-action against 3b12559

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant