From d394d229ace23c7a0f608e18ed701ad049fbf495 Mon Sep 17 00:00:00 2001 From: Jonathon Leight Date: Tue, 4 Aug 2026 21:00:29 -0400 Subject: [PATCH] Refactor configuration profile editing --- internal/core/config_flow_test.go | 27 ++- internal/core/config_profile.go | 43 +++-- internal/core/config_profile_modal_test.go | 167 ++++++++++++++++++ internal/core/endpoints_org_test.go | 9 +- internal/core/templates/config_edit.html | 30 ---- .../core/templates/config_profile_edit.html | 85 ++++++--- internal/e2e/config_profiles_test.go | 107 +++++++++++ internal/web/orgview.go | 7 +- internal/web/static/app.css | 17 ++ internal/web/templates/org_config.html | 78 ++++++-- 10 files changed, 465 insertions(+), 105 deletions(-) create mode 100644 internal/core/config_profile_modal_test.go create mode 100644 internal/e2e/config_profiles_test.go diff --git a/internal/core/config_flow_test.go b/internal/core/config_flow_test.go index b39f188..b5c165f 100644 --- a/internal/core/config_flow_test.go +++ b/internal/core/config_flow_test.go @@ -31,7 +31,8 @@ func TestOrgConfigProfilesFlow(t *testing.T) { t.Fatalf("create org: %v", err) } - // Two profiles, each created on its own page. + // Two profiles, each created through the editor. A save lands back on the + // config page with the new profile selected. for _, p := range []struct{ name, steps string }{{"ESP32", "# esp base"}, {"nRF52", "# nrf base"}} { save := post(t, ts, h.app, "/orgs/"+org.Slug+"/config/profiles", url.Values{"profile_name": {p.name}, "profile_steps": {p.steps}}, sess) @@ -39,6 +40,9 @@ func TestOrgConfigProfilesFlow(t *testing.T) { if save.StatusCode != http.StatusSeeOther { t.Fatalf("create profile %s status = %d, want 303", p.name, save.StatusCode) } + if got, want := save.Header.Get("Location"), "/orgs/"+org.Slug+"/config?profile="+url.QueryEscape(p.name); got != want { + t.Fatalf("create profile %s redirected to %q, want %q", p.name, got, want) + } } // A geofenced region, saved on the region page. rsave := post(t, ts, h.app, "/orgs/"+org.Slug+"/config/regions", url.Values{ @@ -55,9 +59,13 @@ func TestOrgConfigProfilesFlow(t *testing.T) { } body := readBody(t, do(t, ts, h.app, "/orgs/"+org.Slug+"/config", sess)) - // Profiles list with a selector; the geofenced region surfaces as the - // click-to-preview location map (regions aren't listed individually). - for _, want := range []string{"ESP32", "nRF52", "") + } + + for _, v := range []struct { + name string + body string + }{ + {"member", readBody(t, do(t, ts, h.app, "/orgs/"+org.Slug+"/config", memberSess))}, + {"anonymous", readBody(t, do(t, ts, h.root, "/orgs/"+org.Slug+"/config"))}, + } { + if !strings.Contains(v.body, "ESP32") { + t.Fatalf("%s config view should still list the profile", v.name) + } + for _, forbidden := range []string{"config-profile-add", "config-profile-edit", "config-profile-delete", + "config-profile-modal", "/config/profiles"} { + if strings.Contains(v.body, forbidden) { + t.Fatalf("%s config view leaked the admin control %q", v.name, forbidden) + } + } + } +} + +// TestConfigPageEmptyStates: an admin with no config yet gets the profile card +// (with Add profile) plus an explanation of what a profile is, so there's an entry +// point; everyone else still sees the plain "nothing defined yet" note. +func TestConfigPageEmptyStates(t *testing.T) { + t.Parallel() + st, ctx, ts, h := splitServer(t) + admin, sess := appLogin(t, ts, st, ctx, h.app, "cfgemptyadmin") + org, err := st.CreateOrg(ctx, "Empty Cfg Org", admin.ID) + if err != nil { + t.Fatalf("create org: %v", err) + } + + adminView := readBody(t, do(t, ts, h.app, "/orgs/"+org.Slug+"/config", sess)) + for _, want := range []string{`data-testid="config-profile-add"`, "What's a profile?", + "A profile is", "No profiles yet."} { + if !strings.Contains(adminView, want) { + t.Fatalf("empty admin config view missing %q", want) + } + } + + anon := readBody(t, do(t, ts, h.root, "/orgs/"+org.Slug+"/config")) + if !strings.Contains(anon, "hasn't defined any configuration yet") { + t.Fatal("anonymous view of a config-less org should say so") + } + if strings.Contains(anon, "config-profile-add") { + t.Fatal("anonymous view must not offer to add a profile") + } +} diff --git a/internal/core/endpoints_org_test.go b/internal/core/endpoints_org_test.go index 7abb43d..a5bbae4 100644 --- a/internal/core/endpoints_org_test.go +++ b/internal/core/endpoints_org_test.go @@ -87,14 +87,15 @@ func TestOrgConfigProfileUpdateDelete(t *testing.T) { upd := post(t, ts, h.app, base+"/profiles/"+strconv.FormatInt(pid, 10), url.Values{"profile_name": {"Heltec"}, "profile_steps": {"# base"}}, sess) upd.Body.Close() - if loc, _ := url.Parse(upd.Header.Get("Location")); upd.StatusCode != http.StatusSeeOther || loc.Path != base+"/edit" { - t.Fatalf("update profile = %d %q, want 303 → config/edit", upd.StatusCode, upd.Header.Get("Location")) + // Both land back on the config page, where profiles are managed. + if loc, _ := url.Parse(upd.Header.Get("Location")); upd.StatusCode != http.StatusSeeOther || loc.Path != base { + t.Fatalf("update profile = %d %q, want 303 → config", upd.StatusCode, upd.Header.Get("Location")) } del := post(t, ts, h.app, base+"/profiles/"+strconv.FormatInt(pid, 10)+"/delete", url.Values{}, sess) del.Body.Close() - if loc, _ := url.Parse(del.Header.Get("Location")); del.StatusCode != http.StatusSeeOther || loc.Path != base+"/edit" { - t.Fatalf("delete profile = %d %q, want 303 → config/edit", del.StatusCode, del.Header.Get("Location")) + if loc, _ := url.Parse(del.Header.Get("Location")); del.StatusCode != http.StatusSeeOther || loc.Path != base { + t.Fatalf("delete profile = %d %q, want 303 → config", del.StatusCode, del.Header.Get("Location")) } } diff --git a/internal/core/templates/config_edit.html b/internal/core/templates/config_edit.html index 1253035..dcffd73 100644 --- a/internal/core/templates/config_edit.html +++ b/internal/core/templates/config_edit.html @@ -14,36 +14,6 @@ {{template "org-tabs" .Nav}}
-
-
-

Profiles

-
Named sets of base settings — e.g. one per board family. Members pick the profile matching their use case.
-
-
- {{template "icon-plus" "me-1"}}Add profile -
-
- {{if .Profiles}} -
- {{range .Profiles}} -
-
-
{{.Name}}
-
{{len .Steps}} step{{if ne (len .Steps) 1}}s{{end}}
-
- Edit -
- -
-
- {{end}} -
- {{else}} -

No profiles yet. Add one to give members a base-settings preset for their hardware.

- {{end}} -
- -

Regions

diff --git a/internal/core/templates/config_profile_edit.html b/internal/core/templates/config_profile_edit.html index 5dd6c55..7646f65 100644 --- a/internal/core/templates/config_profile_edit.html +++ b/internal/core/templates/config_profile_edit.html @@ -1,3 +1,10 @@ +{{/* The profile editor. Admins normally reach it as the config-profile-modal + fragment swapped into #config-profile-modal on the Configuration page; it + posts via htmx so a validation error re-renders the fragment in place (modal + stays open, entered values preserved) and a save sends HX-Redirect back to + the config page. The full page below is the no-JS / direct-link fallback, so + it carries the same form without the htmx attributes. */}} + {{define "title"}}{{if .ProfileID}}Edit profile{{else}}Add profile{{end}} · {{.Org.Name}} · MeshTender{{end}} {{define "header"}}
@@ -6,13 +13,50 @@

{{if .ProfileID}}Edit profile{{else}}Add profile{{end}}

- Back to configuration + Back to configuration
{{end}} {{define "content"}} {{template "org-tabs" .Nav}} +{{template "profile-editor-alerts" .}} + +
+
+
+ {{template "profile-editor-fields" .}} +
+ + Cancel +
+
+
+
+{{end}} + +{{/* config-profile-modal is the htmx fragment swapped into #config-profile-modal. */}} +{{define "config-profile-modal"}} +
+

{{if .ProfileID}}Edit profile{{else}}Add profile{{end}}

+ +
+
+ {{template "profile-editor-alerts" .}} +
+ {{template "profile-editor-fields" .}} +
+
+
+ + +
+{{end}} + +{{/* profile-editor-alerts renders the validation errors and the risky-command + warning, shared by the page and the modal. */}} +{{define "profile-editor-alerts"}} {{if .Errors}}
@@ -30,35 +74,18 @@
{{end}} +{{end}} -
-
-
-
- - -
-
- - - One command per line; lines starting with # are notes. Put {{"{{ region }}"}} on its own line to insert this org's region settings there — otherwise they run after all steps. -
-
- - Cancel -
-
-
+{{/* profile-editor-fields are the form controls themselves — the
element + around them differs between the page and the modal. */}} +{{define "profile-editor-fields"}} +
+ +
- -{{if .ProfileID}} -
-
- - - Removes this profile and its base settings. - -
+
+ + + One command per line; lines starting with # are notes. Put {{"{{ region }}"}} on its own line to insert this org's region settings there — otherwise they run after all steps.
{{end}} -{{end}} diff --git a/internal/e2e/config_profiles_test.go b/internal/e2e/config_profiles_test.go new file mode 100644 index 0000000..7fe6d1f --- /dev/null +++ b/internal/e2e/config_profiles_test.go @@ -0,0 +1,107 @@ +//go:build browser + +package e2e + +import ( + "strings" + "testing" + + cdplog "github.com/chromedp/cdproto/log" + "github.com/chromedp/cdproto/network" + "github.com/chromedp/chromedp" + + "github.com/jleight/meshtender/internal/store" +) + +// TestE2EConfigProfileSelect: the profile list on the Configuration page selects a +// profile — clicking a row shows that profile's base settings on the left and marks +// the row active — under the strict CSP. +func TestE2EConfigProfileSelect(t *testing.T) { + srv := newE2EServer(t) + user, cookie := srv.login(t, "e2ecfgselect") + org, err := srv.store.CreateOrg(srv.ctx, "Config Select Org", user.ID) // creator = admin + if err != nil { + t.Fatalf("create org: %v", err) + } + for _, p := range []struct{ name, comment string }{{"ESP32", "esp base"}, {"nRF52", "nrf base"}} { + if _, err := srv.store.CreateProfile(srv.ctx, org.ID, p.name, []store.ConfigStep{{Comment: p.comment}}); err != nil { + t.Fatalf("create profile %s: %v", p.name, err) + } + } + + bctx, cancel, watch := startBrowser(t) + defer cancel() + + var heading, steps string + var rowActive bool + cfgURL := srv.appURL + "/orgs/" + org.Slug + "/config" + if err := chromedp.Run(bctx, + network.Enable(), + cdplog.Enable(), + setSessionCookie(cookie), + chromedp.Navigate(cfgURL), + chromedp.WaitVisible(`[data-testid="config-profile-list"]`, chromedp.ByQuery), + // Click the second row's name to select it. + chromedp.Click(`[data-testid="config-profile-row"]:nth-of-type(2) a`, chromedp.ByQuery), + chromedp.WaitVisible(`.card-title`, chromedp.ByQuery), + chromedp.Text(`.col-lg-8 .card-title`, &heading, chromedp.ByQuery), + chromedp.Text(`.col-lg-8 .card-body pre`, &steps, chromedp.ByQuery), + chromedp.Evaluate(`!!document.querySelector('[data-testid="config-profile-row"]:nth-of-type(2).active')`, &rowActive), + ); err != nil { + t.Fatalf("browser run against %s: %v", cfgURL, err) + } + if !strings.Contains(heading, "nRF52") { + t.Fatalf("selected profile heading = %q, want it to name nRF52", heading) + } + if !strings.Contains(steps, "nrf base") { + t.Fatalf("selected profile steps = %q, want nRF52's base settings", steps) + } + if !rowActive { + t.Fatal("the selected profile's row should be marked active") + } + watch.assertClean(t) +} + +// TestE2EConfigProfileModalSave: an admin adds a profile from the Configuration page +// itself — the editor loads into the modal via htmx and saving navigates back to the +// config page with the new profile selected (no separate editor page). +func TestE2EConfigProfileModalSave(t *testing.T) { + srv := newE2EServer(t) + user, cookie := srv.login(t, "e2ecfgadd") + org, err := srv.store.CreateOrg(srv.ctx, "Config Add Org", user.ID) + if err != nil { + t.Fatalf("create org: %v", err) + } + + bctx, cancel, watch := startBrowser(t) + defer cancel() + + var heading, currentURL string + cfgURL := srv.appURL + "/orgs/" + org.Slug + "/config" + if err := chromedp.Run(bctx, + network.Enable(), + cdplog.Enable(), + setSessionCookie(cookie), + chromedp.Navigate(cfgURL), + chromedp.WaitVisible(`[data-testid="config-profile-add"]`, chromedp.ByQuery), + chromedp.Click(`[data-testid="config-profile-add"]`, chromedp.ByQuery), + // The editor arrives in the modal via htmx. + chromedp.WaitVisible(`#config-profile-modal-content input[name=profile_name]`, chromedp.ByQuery), + chromedp.SetValue(`#config-profile-modal-content input[name=profile_name]`, "Heltec", chromedp.ByQuery), + chromedp.SetValue(`#config-profile-modal-content textarea[name=profile_steps]`, "# heltec base", chromedp.ByQuery), + chromedp.Click(`[data-testid="save-profile"]`, chromedp.ByQuery), + // HX-Redirect navigates back to the config page, now showing the new profile. + chromedp.WaitVisible(`[data-testid="config-profile-list"]`, chromedp.ByQuery), + chromedp.Text(`.col-lg-8 .card-title`, &heading, chromedp.ByQuery), + chromedp.Location(¤tURL), + ); err != nil { + t.Fatalf("browser run against %s: %v", cfgURL, err) + } + if !strings.Contains(heading, "Heltec") { + t.Fatalf("after saving, left panel heading = %q, want it to name Heltec", heading) + } + if !strings.Contains(currentURL, "/config?profile=Heltec") { + t.Fatalf("after saving, URL = %q, want the config page with Heltec selected", currentURL) + } + watch.assertClean(t) +} diff --git a/internal/web/orgview.go b/internal/web/orgview.go index d18cba8..071f5ad 100644 --- a/internal/web/orgview.go +++ b/internal/web/orgview.go @@ -13,8 +13,11 @@ import ( // root/marketing host (anonymous) can render the same shared templates — the // same split that lets both surfaces render org_public.html. -// ProfileView is one named base-settings profile for display. +// ProfileView is one named base-settings profile for display. ID is carried so +// the admin surface can build its edit/delete URLs from the same list the +// read-only view renders (the public surface just ignores it). type ProfileView struct { + ID int64 Name string Steps []store.ConfigStep } @@ -68,7 +71,7 @@ func BuildConfigView(ctx context.Context, st *store.Store, orgID int64, selected PreviewActive: lat != nil && lon != nil, } for _, p := range profiles { - cv.Profiles = append(cv.Profiles, ProfileView{Name: p.Name, Steps: p.Steps}) + cv.Profiles = append(cv.Profiles, ProfileView{ID: p.ID, Name: p.Name, Steps: p.Steps}) } if len(profiles) > 0 { idx := 0 diff --git a/internal/web/static/app.css b/internal/web/static/app.css index b093f0e..8858542 100644 --- a/internal/web/static/app.css +++ b/internal/web/static/app.css @@ -209,6 +209,23 @@ code.pubkey { .leaflet-container .leaflet-control-layers-list label { margin: 0; color: var(--tblr-body-color); } .leaflet-container .leaflet-control-layers-selector { accent-color: var(--tblr-primary); } +/* Profile list on the org config page: a long list scrolls inside its card + instead of pushing the location-preview map off the page. Rows are compact — + Tabler's default list-group padding plus a full-size icon button makes a ~4.5rem + row, so only a couple fit before scrolling. Both are driven by CSS variables, so + turn those down here rather than overriding the padding itself. */ +.profile-list { + max-height: 22rem; + overflow-y: auto; +} +.profile-list .list-group-item { + --tblr-list-group-item-padding-y: 0.375rem; +} +.profile-list .btn-icon { + --tblr-btn-padding-y: 0.25rem; + --tblr-btn-icon-size: 1.25rem; +} + /* Active region block in the config editor: highlight the block currently being drawn on the shared region map. */ .region-block.region-active { diff --git a/internal/web/templates/org_config.html b/internal/web/templates/org_config.html index 6c282c6..f01d055 100644 --- a/internal/web/templates/org_config.html +++ b/internal/web/templates/org_config.html @@ -3,9 +3,9 @@ {{define "content"}} {{template "org-tabs" .Nav}} -{{if not .Config.HasConfig}} +{{if and (not .Config.HasConfig) (not .CanEdit)}}
- This organization hasn't defined any configuration yet.{{if .CanEdit}} Use Edit to add a profile or region.{{end}} + This organization hasn't defined any configuration yet.
{{else}}
@@ -16,6 +16,15 @@

{{.Config.Selected}} · base settings

{{template "step-list" .Config.SelectedSteps}}
+ {{else if .CanEdit}} + +
+

What's a profile?

+
+

A profile is this organization's recommended configuration: the commands a repeater owner runs to bring their repeater in spec, not just onto the mesh. Profiles vary by hardware and use case, so give each one a name that tells members which fits theirs.

+

Use Add profile to create your first one.

+
+
{{end}} {{if .Config.PreviewActive}} @@ -31,28 +40,54 @@ {{end}}
- +
- {{if .Config.Profiles}} + {{if or .Config.Profiles .CanEdit}}
-

Profile

-
-
- - {{if .PreviewLat}}{{end}} - {{if .PreviewLon}}{{end}} -
-

- Pick the profile matching your hardware to see its base settings.{{if .Config.HasRegions}} A repeater's regions are assigned separately, by location.{{end}} -

+
+

Profiles

+ {{if .CanEdit}} +
+ +
+ {{end}}
+ {{if .Config.Profiles}} +
+ {{range .Config.Profiles}} +
+ {{.Name}} + {{if $.CanEdit}} + +
+ +
+ {{end}} +
+ {{end}} +
+ {{else}} +
No profiles yet.
+ {{end}} + {{if or .Config.Profiles .Config.HasRegions}} +
+ {{if .Config.Profiles}}Select a profile to see its base settings.{{end}}{{if .Config.HasRegions}} A repeater's regions are assigned separately, by location.{{end}} +
+ {{end}}
{{end}} {{if .Config.HasRegionShapes}} -
+

Preview a location

@@ -74,6 +109,15 @@ {{end}}
+ +{{if .CanEdit}} + + +{{end}} {{end}} {{end}}