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

Improve user feedback when developer mode change fails #4469

Merged
merged 5 commits into from
Sep 17, 2024

Conversation

Steve-Mcl
Copy link
Contributor

closes #4468

Description

Presents a friendlier toast than "Request failed with status code 403"

Now displays relevant toast text with a fall back to a friendlier message if the error is generic or unrecognised.

  • Status 403: "You do not have permission to change developer mode"
  • Status 401: "You are not authorized to change developer mode"
  • Otherwise: "An error occurred while attempting to change developer mode"

Related Issue(s)

#4468

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

Labels

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

Copy link

codecov bot commented Sep 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.12%. Comparing base (6a1f73e) to head (c22c1a1).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4469   +/-   ##
=======================================
  Coverage   78.12%   78.12%           
=======================================
  Files         296      296           
  Lines       14125    14125           
  Branches     3189     3189           
=======================================
  Hits        11035    11035           
  Misses       3090     3090           
Flag Coverage Δ
backend 78.12% <ø> (ø)

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.

@knolleary
Copy link
Member

Status 403: "You do not have permission to change developer mode"
Status 401: "You are not authorized to change developer mode"

Why show different messages for these two cases? For the user, its the same fact - they aren't allowed to do it. The underlying reason may be subtly different, but the proposed text doesn't make that difference obvious (and nor should it IMHO).

But also, shouldn't we be able to know ahead of time the user isn't allowed to do the action - and thus we shouldn't have let them try in the first place?

@joepavitt
Copy link
Contributor

joepavitt commented Sep 14, 2024

Why show different messages for these two cases? For the user, its the same fact - they aren't allowed to do it.

👍 for this

But also, shouldn't we be able to know ahead of time the user isn't allowed to do the action - and thus we shouldn't have let them try in the first place?

and this actually.

@Steve-Mcl
Copy link
Contributor Author

  • consolidated toasts to single warning for permission error
    • even though in theory, the toggle should be disabled (so as to inhibit operation), this toast was left in place as a friendlier catch all for scenarios not yet considered or found
  • add support to the developer toggle to permit parent component to disable it
  • disable toggle if user, feature or state requirements are not met
  • adds a tooltip hint explaining why user cannot operate toggle

member user
image

owner user
image

@joepavitt joepavitt merged commit 89c270b into main Sep 17, 2024
14 checks passed
@joepavitt joepavitt deleted the 4468-developer-mode-feedback branch September 17, 2024 14:15
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.

Improve feedback when device developer mode fails
4 participants