reconcile boot and admin pages further #963
Conversation
|
Deployed as https://grist-paulfitz-smoosh.fly.dev (until 2024-06-01T20:59:50.109Z) |
|
Deployed as https://grist-paulfitz-smoosh.fly.dev (until 2024-06-07T17:38:26.512Z) |
|
Deployed as https://grist-paulfitz-smoosh.fly.dev (until 2024-06-07T19:33:43.669Z) |
|
Deployed as https://grist-paulfitz-smoosh.fly.dev (until 2024-06-08T01:56:06.616Z) |
|
Deployed as https://grist-paulfitz-smoosh.fly.dev (until 2024-06-08T21:37:29.757Z) |
|
Deployed as https://grist-paulfitz-smoosh.fly.dev (until 2024-06-08T21:41:47.286Z) |
|
Deployed as https://grist-paulfitz-smoosh.fly.dev (until 2024-06-12T21:00:36.329Z) |
|
Deployed as https://grist-paulfitz-smoosh.fly.dev (until 2024-06-12T21:18:53.365Z) |
|
Deployed as https://grist-paulfitz-smoosh.fly.dev (until 2024-06-12T21:36:57.187Z) |
|
Deployed as https://grist-paulfitz-smoosh.fly.dev (until 2024-06-12T21:56:23.056Z) |
|
Deployed as https://grist-paulfitz-smoosh.fly.dev (until 2024-06-12T22:21:18.125Z) |
|
Deployed as https://grist-paulfitz-smoosh.fly.dev (until 2024-06-13T18:18:32.163Z) |
|
Deployed as https://grist-paulfitz-smoosh.fly.dev (until 2024-06-14T20:18:21.394Z) |
|
Deployed as https://grist-paulfitz-smoosh.fly.dev (until 2024-06-15T17:59:54.147Z) |
|
Deployed as https://grist-paulfitz-smoosh.fly.dev (until 2024-06-15T18:00:41.943Z) |
|
Deployed as https://grist-paulfitz-smoosh.fly.dev (until 2024-06-15T18:16:42.039Z) |
app/client/ui/AdminPanel.ts
Outdated
| if (result.success === undefined) { return '―'; } | ||
| if (result.success) { return '✅'; } | ||
| if (result.severity === 'warning') { return '❗'; } | ||
| if (result.severity === 'hmm') { return '?'; } | ||
| // remaining case is a fault. | ||
| return '❌'; |
There was a problem hiding this comment.
I wonder if it's worth flipping severity and success around so severity comes first, in case we make a mistake on the backend and return success + warning, for example?
There was a problem hiding this comment.
I sat down to do this, but I'm not sure if it is right. It could be that the concept of severity is only really relevant if a check fails? To determine whether the failure is an error or a warning or a "hmm" (meaning ~ "debug" I guess). If the check succeeds, is the severity important?
It could be relevant if we were filtering results by severity, so we'd want to see e.g. only things-that-would-be-errors-if-they-failed.
Concretely: if a test with severity warning succeeds, what icon should we show?
There was a problem hiding this comment.
I guess we can choose what {success: true, severity: "warning"} means - is it fine or is it a bug, and if fine what should be rendered. I think from your q you'd see it as a bug @Spoffy ?
…tion is undefined
… recognize it! Co-authored-by: George Gevoian <85144792+georgegevoian@users.noreply.github.com>
Co-authored-by: George Gevoian <85144792+georgegevoian@users.noreply.github.com>
|
Deployed as https://grist-paulfitz-smoosh.fly.dev (until 2024-06-15T19:34:55.380Z) |
--inspect-brk is useful if you want to debug something during startup
This self-test isn't perfect because we're running it from the backend instead of the frontend. Conceivably the backend might have trouble resolving its own url. Eventually we should move this test or something like it to something that executes in the frontend.
|
Deployed as https://grist-paulfitz-smoosh.fly.dev (until 2024-06-16T13:23:38.607Z) |
|
Deployed as https://grist-paulfitz-smoosh.fly.dev (until 2024-06-22T19:00:25.482Z) |
|
Deployed as https://grist-paulfitz-smoosh.fly.dev (until 2024-06-22T19:06:07.986Z) |
|
I worked through our code review checklist. Weakest point seems mobile, the admin panel generally is a bit clunky though functional on mobile. This PR doesn't improve it or make it worse as far as I can see. |
|
Making one more change after discussion with @Spoffy - Replacing |
|
Deployed as https://grist-paulfitz-smoosh.fly.dev (until 2024-06-22T20:24:56.453Z) |
This adds some remaining parts of the boot page to the admin panel, including the fallback mechanism for accessing it. It then removes the boot page.
One common failure mode for Grist is for the home url to be wrong. That is pretty catastrophic for the front end, since it ends up unable to access the API. The boot page avoided the problem by having dedicated relative api endpoints, but this won't work for the admin panel, since developers of the admin panel have been using regular api mechanisms, and not allowing that would slow work down. But still, it would be sad if the admin panel were unavailable just when you needed it. So this PR uses a distinct algorithm for picking the base url for the api that should be more robust and also fine to use outside of the context of document access.
The boot page arranged also for static assets to be available at a more robust base url. I've abandoned that since it seemed tricky to graft across to the admin page. That means there will be no diagnostic UI available if static asset urls are messed up. This is a less common problem in single-server setups I think? Could revisit if it proves an achilles heel.
If visited without admin privileges, the admin page now gives some basic tips in case the user is actually a flailing admin. The tips include a fallback auth mechanism to use when all else fails.