Skip to content

feat(mqtt): slim bridge source-edit modal to basics + deep-link to Configuration page#3299

Merged
Yeraze merged 1 commit into
mainfrom
feat/slim-mqtt-bridge-modal
Jun 2, 2026
Merged

feat(mqtt): slim bridge source-edit modal to basics + deep-link to Configuration page#3299
Yeraze merged 1 commit into
mainfrom
feat/slim-mqtt-bridge-modal

Conversation

@Yeraze
Copy link
Copy Markdown
Owner

@Yeraze Yeraze commented Jun 2, 2026

Summary

Follow-up to the MQTT bridge Configuration page (#3295). That page is now the home for all advanced bridge settings, but the create/edit Source popup still duplicated them (mode, forwarding, ok_to_mqtt, subscribe topic-block, geofence, publish channel/topic filters, topic rewrites) — two places to maintain.

This trims the popup to just the basics needed to create/connect a bridge, and points users to the Configuration page for the rest.

Changes

  • Slim modal (mqtt_bridge): keeps Name, Parent broker, Upstream URL, Username, Password, Upstream topics. Removes the advanced fields (and their state / reset / hydration, plus the now-unused BBoxMapEditor + bbox-seed imports/helper).
  • Hint + deep-link: a one-line hint plus a "Configuration →" button that, when editing an existing source, closes the modal and navigates to /source/<id>/#mqtt-config. Hidden on create (no source exists yet).
  • Shared serializer is now partial-aware: buildBridgeConfig accepts a Partial<BridgeConfigForm> and manages only the fields a caller supplies, merging over the stored config (base) for the rest. A modal save therefore touches only the basics and preserves all advanced settings untouched — single source of truth, no double-maintenance. This also fixes a latent bug where a modal edit could clobber fields it didn't render.
  • Configuration page now also passes the loaded config as base, so it preserves config sub-keys it doesn't surface (e.g. node/portnum filters).

Files

File Change
src/pages/DashboardPage.tsx Slimmed bridge modal; removed advanced state/reset/hydration + bbox imports/helper; added the deep-link button.
src/components/MQTT/mqttBridgeConfig.ts buildBridgeConfig partial-form + preserve semantics.
src/components/MQTT/MqttBridgeConfigurationView.tsx Pass loaded config as base on save.
src/components/MQTT/mqttBridgeConfig.test.ts +1 test: "modal posts only basics → advanced preserved" (18 total).

Testing

  • tsc clean; lint introduces no new errors.
  • Full Vitest suite: 5880 passed, 0 failures.
  • Manual, dev container, end-to-end:
    • Bridge edit popup shows only the basics + hint + "Configuration →" button.
    • Changed a username and saved from the popup → API confirmed mode, forwardingMode, ignoreOkToMqtt, uplinkFilters.channels, downlinkFilters.geo, and downlinkTopicRewrite all preserved (username updated).
    • Clicked "Configuration →" → closed the popup and opened the correct bridge's Configuration page.

🤖 Generated with Claude Code

…figuration page

The MQTT bridge create/edit popup duplicated every advanced setting that now
lives on the dedicated Configuration page (mode, forwarding, ok_to_mqtt,
subscribe topic-block, geofence, publish channel/topic filters, rewrites),
forcing them to be maintained in two places.

Trim the popup to the essentials needed to create/connect a bridge — name,
parent broker, upstream URL, username, password, and subscriptions — plus a
hint and, when editing an existing source, a "Configuration →" button that
deep-links straight to that bridge's Configuration page
(/source/<id>/#mqtt-config).

buildBridgeConfig now accepts a *partial* form and manages only the fields a
caller actually supplies, merging over the stored config (base) for the rest.
So a modal save touches just the basics and preserves all advanced settings
untouched — single source of truth, no double-maintenance. This also fixes a
latent bug where a modal edit could clobber fields it didn't render. The
Configuration page passes the loaded config as base too, so it preserves any
config sub-keys it doesn't surface (e.g. node/portnum filters).

Removed the now-unused advanced bridge form state, reset/hydration, and the
BBoxMapEditor / bbox-seed imports + helper from DashboardPage.

Tests: mqttBridgeConfig.test.ts gains a "modal posts only basics → advanced
preserved" case (18 total). Full suite green (5880 passed). Verified live in
the dev container: slimmed modal, modal save preserves advanced config, and
the deep-link button opens the right bridge's Configuration page.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 2, 2026

Claude finished @Yeraze's task —— View job


Code Review Analysis ✅

Todo List

  • Read and analyze changed files
  • Review code quality and architecture decisions
  • Check for potential bugs or security issues
  • Assess performance considerations
  • Evaluate test coverage
  • Verify adherence to project guidelines
  • Provide comprehensive feedback

🎯 Summary

This is a well-executed refactor that successfully separates concerns between the lightweight modal and the full Configuration page. The implementation demonstrates thoughtful architecture decisions and solid testing practices.

✅ Code Quality & Architecture

Excellent separation of concerns:

  • Modal now handles only the connection essentials (broker, URL, credentials, subscriptions)
  • Advanced settings (mode, forwarding, filters, geofence, rewrites) moved to dedicated Configuration page
  • Shared serializer (buildBridgeConfig) elegantly handles partial forms via Partial<BridgeConfigForm>

Smart preservation mechanism:

  • The base parameter pattern in buildBridgeConfig is clever - enables partial updates without data loss
  • Field-level management (lines 169-264 in mqttBridgeConfig.ts) only touches fields the caller supplies
  • Prevents the common anti-pattern of accidentally clobbering unrendered fields

Clean user experience:

  • Deep-linking to Configuration page with hash anchor (#mqtt-config) provides seamless navigation
  • Contextual hint text explains where advanced settings live
  • Modal stays focused on quick setup tasks

🐛 Potential Issues

Minor: Deep-link button placement

  • The "Configuration →" button in DashboardPage.tsx:1847-1858 only shows for existing sources (editingSourceId), which is correct
  • However, consider adding a note for new bridges about where to find advanced settings after creation

Edge case: Navigation state

  • Deep-link closes modal and navigates, but doesn't handle potential unsaved changes in the modal
  • This appears intentional since only basic fields are managed, but worth noting

🔒 Security Considerations

Password handling is secure:

  • Empty password on edit preserves stored credentials (DashboardPage.tsx:682-685)
  • buildBridgeConfig properly drops password field when empty (lines 179, 232)
  • No password leakage in serialization (test line 235 confirms this)

Credential preservation:

  • GETs strip passwords for non-admins, with defensive handling in bridge rewrite logic (lines 1060-1063)

⚡ Performance Considerations

Efficient form management:

  • Removed unnecessary state variables and imports (BBoxMapEditor, bbox-seed helpers)
  • Eliminated complex form hydration/reset for unused advanced fields
  • Modal now manages ~50% fewer state variables

Network optimization:

  • Partial updates only send changed fields, preserving bandwidth
  • Server-side merging reduces client-side complexity

🧪 Test Coverage

Comprehensive new test case:

  • Test at lines 133-167 specifically validates the core feature: "modal posts only basics → advanced preserved"
  • Covers the exact scenario this PR enables - modal editing without clobbering advanced settings
  • Tests 18 different fields across all categories (connection, mode, filters, rewrites)

Existing test coverage remains strong:

  • 18 total tests covering all edge cases (validation, round-trip serialization, geo bounds, topic filters)
  • Good coverage of the Partial<BridgeConfigForm> functionality

📋 Adherence to Project Guidelines

Follows CLAUDE.md conventions:

  • ✅ No raw SQL outside repositories
  • ✅ Proper async patterns maintained
  • ✅ Multi-source architecture preserved (sourceId scoping intact)
  • ✅ Shared utilities properly used (appBasename, translation keys)

Code style compliance:

  • Clean TypeScript interfaces and proper typing
  • Consistent naming conventions
  • Appropriate commenting for complex logic (preservation semantics)

🏆 Strengths

  1. Single source of truth: Advanced settings now live exclusively on Configuration page
  2. Backwards compatibility: Existing stored configs work seamlessly with the new partial-update logic
  3. Developer experience: Shared serializer prevents drift between modal and full page
  4. User experience: Clear progressive disclosure - basics in modal, advanced settings on dedicated page
  5. Robust testing: New test specifically validates the preservation behavior

📝 Minor Suggestions

  1. Consider adding a brief comment in the modal explaining that advanced settings can be found on the Configuration page after source creation (for new bridges)

  2. The deep-link hash anchor implementation is clean, but consider whether the Configuration page should scroll to the relevant section automatically

🎉 Conclusion

This is a high-quality refactor that successfully reduces modal complexity while maintaining full functionality. The partial-form architecture is elegant and well-tested. The code follows project conventions and demonstrates good architectural thinking.

Recommendation: ✅ Approve - This PR is ready for merge.


@Yeraze Yeraze merged commit 826b795 into main Jun 2, 2026
19 checks passed
@Yeraze Yeraze deleted the feat/slim-mqtt-bridge-modal branch June 2, 2026 16:19
Yeraze added a commit that referenced this pull request Jun 2, 2026
…3304)

Bump version to 4.8.3 across package.json, package-lock.json,
desktop/package.json, desktop/src-tauri/tauri.conf.json,
helm/meshmonitor/Chart.yaml (version + appVersion), and the CLAUDE.md
header.

Add a [4.8.3] CHANGELOG entry rolling up the PRs merged since 4.8.2:
active MeshCore node discovery (#3302) and reciprocal discoverability
(#3303), the dedicated MQTT bridge Configuration page with per-bridge
publish filter (#3294) and slimmed source-edit modal (#3299), plus
fixes to MeshCore private-key validation (#3301), hashtag-channel
creation (#3298), reconnect stability (#3270), the npm legacy-peer-deps
pin (#3283), OIDC auto-admin docs (#3292), and Dependabot bumps.

Review docs for accuracy against these changes:
- docs/features/mqtt-broker.md: topic rewrites and the publish filter
  now live on the dedicated bridge Configuration page, not the broker's
  edit modal (which is slimmed to basics in 4.8.3).
- docs/features/meshcore.md: document the new Active Node Discovery
  ("Discover Nearby Nodes" / "Discover Repeaters") and discoverability.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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