Skip to content

Conversation

hardillb
Copy link
Contributor

@hardillb hardillb commented Sep 10, 2025

closes #5951

Description

This PR includes a DB migration - ensure it is properly named before merging

Adds support for Team Broker to MQTT Schema capture.

Depends on

And on other container drivers:

Related Issue(s)

#5951

Checklist

  • I have read the contribution guidelines
  • Suitable unit/system level tests have been added and they pass
  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on FlowFuse/helm to update ConfigMap Template
    • Issue/PR raised on FlowFuse/CloudProject to update values for Staging/Production
  • Link to Changelog Entry PR, or note why one is not needed.

Labels

  • Includes a DB migration? -> add the area:migration label

@hardillb hardillb added this to the 2.22 milestone Sep 10, 2025
@hardillb hardillb self-assigned this Sep 10, 2025
Copy link

codecov bot commented Sep 10, 2025

Codecov Report

❌ Patch coverage is 59.80392% with 41 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.75%. Comparing base (93a2df7) to head (94cee7b).
⚠️ Report is 16 commits behind head on main.

Files with missing lines Patch % Lines
forge/ee/routes/teamBroker/3rdPartyBroker.js 28.20% 28 Missing ⚠️
forge/db/models/TeamBrokerAgent.js 74.28% 9 Missing ⚠️
forge/routes/auth/index.js 60.00% 2 Missing ⚠️
forge/db/controllers/AccessToken.js 85.71% 1 Missing ⚠️
...migrations/20250908-01-EE-add-team-broker-agent.js 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6003      +/-   ##
==========================================
- Coverage   76.76%   76.75%   -0.01%     
==========================================
  Files         376      378       +2     
  Lines       18869    18967      +98     
  Branches     4492     4516      +24     
==========================================
+ Hits        14485    14559      +74     
- Misses       4384     4408      +24     
Flag Coverage Δ
backend 76.75% <59.80%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@hardillb hardillb requested a review from knolleary September 11, 2025 09:01
@hardillb hardillb marked this pull request as ready for review September 11, 2025 09:58
Copy link
Member

@knolleary knolleary left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with localfs and all working. My main feedback is on the UX side.

There is an inconsistency between the team and 3rd party broker UX now that I think should be aligned.

Team broker has a button to start schema capture. Once clicked, there's no feedback (other than the button is now disabled) - and no way to stop the schema capture.

image

3rd Party broker already has a toggle button for the same concept:

image

I think this needs to be aligned.

@joepavitt
Copy link
Contributor

Just having a think over on this. For what is meant to be a page showing live data, it doesn't feel very alive. For Third-Party view, it's a little better with the green "Connected" pill, I think we should have an equivalent for the Team Broker too.

Toggles

I see there being three positions where toggles could live, shown here:

Screenshot 2025-09-17 at 13 32 26

For the Team Broker, there is no value is a global "disconnect" option at the top-level, the left-side toggle for the hierarchy is always on, and the right-side can be toggled on to watch payload schemas.

For Third-Party brokers, there is only a single connect/disconnect option, when that is connected then both the hierarchy and the topic are actively monitored.

I want to make sure we are communicating value here, and that it's clear to the user that there is live data being gathered

@joepavitt
Copy link
Contributor

joepavitt commented Sep 17, 2025

Screenshot 2025-09-17 at 13 40 23

Okay, here me out... we have all three toggles, with disabled states as per this screenshot for those not interactive in the different combinations. The important factor is a tooltip to explain why it's disabled. Emphasising here, that the Live data is gathered 24/7 for third-party brokers).

For the Team Broker, I don't think we need the top-level toggle, and only the two at the lower level, however, do should have the connected pill added.

So, to clarify:

  • FlowFuse Team Broker
    • Status Pill Added
    • Top Toggle: Not Available
    • Left Toggle: Disabled (with tooltip explaining this is always live)
    • Right Toggle: Available to toggle on/off to control payload monitoring
  • Third Party Broker:
    • Top Toggle: Available
    • Left Toggle: Disabled
    • Right Toggle: Disabled

Whilst the left-toggle is always disabled, I feel it still necessary to emphasise that's it's live and doing something.

@hardillb let me know if you want me to dev this up.

@hardillb
Copy link
Contributor Author

@joepavitt I think I follow what your saying and agree, but if you could start on the front end and shout if there any backend changes needed

@joepavitt
Copy link
Contributor

@hardillb I've updated the UI, I've not implemented the turning off the agent for Team Broker, but do feel free to if you think it'd be useful.

@hardillb
Copy link
Contributor Author

I've fixed the lint and removed the alert because it was triggering if you navigated from a disabled broker to the already running Team Broker.

I'm going to leave being able to turn team broker collection off manually for now

But it looks a lot more consistent across the 2 modes now.

@joepavitt joepavitt requested a review from knolleary September 19, 2025 08:32
@knolleary
Copy link
Member

Tested locally with driver-localfs.

@knolleary
Copy link
Member

@hardillb I have approved the core pr, driver-localfs and the agent. Due to their interdependencies when it comes to deploying to prod, will let you merge in the appropriate order.

@hardillb
Copy link
Contributor Author

@knolleary Thanks, will get with Steve today to wrap up the driver-docker side and the it can all be merged

@hardillb hardillb mentioned this pull request Sep 19, 2025
11 tasks
@hardillb hardillb added the area:migration Involves a database migration label Sep 19, 2025
@hardillb hardillb merged commit beb462c into main Sep 22, 2025
22 of 23 checks passed
@hardillb hardillb deleted the team-broker-schema-agent branch September 22, 2025 10:35
@joepavitt joepavitt moved this from Review to Done in 🛠 Development Sep 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:migration Involves a database migration

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Schema autodetection for FlowFuse Broker

3 participants