Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor device editor endpoints and move under ee code tree #2109

Merged
merged 1 commit into from
May 9, 2023

Conversation

knolleary
Copy link
Member

Description

This is the result of my code review of #2042

It makes a number of changes around the underlying API endpoints and some UI tweaks.

API Changes

This summaries the admin api changes that #2042 and this PR combined apply. I have highlighted how this PR changes what is in #2042:

  • GET /api/v1/devices/:id - adds tunnel info if enabled. Changed structure of tunnel info
  • GET /api/v1/projects/:projectId/devices - add tunnel info if enabled Removed
  • GET /api/v1/team/:teamId/devices - add tunnel info if enabled Removed
  • GET /api/v1/devices/:deviceId/editor - get editor state. Moved under ee code tree
  • PUT /api/v1/devices/:deviceId/editor - enable/disable the editor. Moved under ee code tree
  • POST /api/v1/devices/:deviceId/snapshot - create a snapshot for the device
  • PUT /api/v1/devices/:deviceId/mode - Modify device mode. I want to roll this under the existing 'update device' endpoint, rather than introduce a new one. But time is running out.

The following endpoints have been moved (both in code tree, but also the url). The corresponding changes to the Device Agent have already been made via FlowFuse/device-agent#81

  • Inbound websocket endpoint
    was GET /api/v1/remote/editor/inboundWS/:deviceId/:access_token
    now GET /api/v1/devices/:deviceId/editor/comms/:access_token
  •  Verify adminAuth token
    was GET /api/v1/remote/editor/
    now GET /api/v1/devices/:deviceId/editor/token
  •  Handle editor requests
    was * /api/v1/remote/editor/:deviceId/*
    now * /api/v1/devices/:deviceId/editor/proxy/*

This puts all of the editor access routes under /api/v1/devices/:deviceId/editor/**. These routes are only added if running with an EE license.

Tunnel status response

The GET /api/v1/devices/:deviceId endpoint includes a mode property to indicate if the device is in autonomous (default) or developer mode.

If in developer mode, the response include an editor property with meta data about the current editor state:

"editor": {
    "url": "http://localhost:3000/api/v1/devices/O65j9Yj8e2/editor/proxy/?access_token=XYZ",
    "enabled": true,
    "connected": true
}

This replaces the multiple tunnelUrl tunnelUrlWithToken tunnelExists etc top-level properties. I've reduced them to the minimum required. I could see no reason for having both url and urlwithtoken - in fact, tunnelUrl was unused in the frontend code. Now we just have a url property that can be used to access the editor.

UI Changes

Having played with the UI, I've made some tweaks.

  1. The 'mode' button in the top right corner has been relabelled 'Developer Mode' and reuses the Beaker icon. The button uses secondary state when not in developer mode, primary state when in developer mode. This acts as a visual cue to the state.
    image
    image

  2. The Mode dialog has now an enable/disable dialog. When we had multiple possible modes a list of options made sense. Having a single option makes less sense. So now it presents as either an 'enable dev mode' or 'disable dev mode' - allowing for more explanation as to what each means:
    image
    image

  3. When in developer mode, I've relabelled the 'Device Options' to be 'Developer Mode Options' and changes the icon to a Beaker - reinforcing these are dev mode options only. I've also shrunk the buttons; the 'create snapshot' button was wrapping across multiple lines and looking distractingly huge. I've lower-cased the enabled/disabled status badge to be consistent with how we present text in these status pills throughout.

image

Related Issue(s)

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 flowforge/helm to update ConfigMap Template
    • Issue/PR raised on flowforge/CloudProject to update values for Staging/Production

Labels

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

@knolleary knolleary requested a review from Steve-Mcl May 9, 2023 21:49
@Steve-Mcl Steve-Mcl merged commit bb77a15 into device-editor May 9, 2023
@Steve-Mcl Steve-Mcl deleted the 1821-refactor-device-editor-api branch May 9, 2023 22:17
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.

2 participants