Add banner for scheduled maintenance #7284

fm3 · 2023-08-17T09:29:35Z

URL of deployed dev instance (used for testing):

https://___.webknossos.xyz

Steps to test:

Backend:

Create new maintenance with curl -X POST 'http://localhost:9000/api/maintenances' -H 'X-Auth-Token: secretSampleUserToken' -H "Content-Type: application/json" -d '{"startTime":1692272319000, "endTime":1692272320000, "message":"a message!"}' (adapt timestamps to something in the future or current, use https://www.epochconverter.com/ for millisecond unix timestamps)
delete and update routes, as well as readOne and listAll are also only available for super users
- curl -X DELETE 'http://localhost:9000/api/maintenances/64de02abee0100c103ab66b6' -H 'X-Auth-Token: secretSampleUserToken' (replace id)
- curl -X PUT 'http://localhost:9000/api/maintenances/64de0390ee0100a006ab66cc' -H 'X-Auth-Token: secretSampleUserToken' -H "Content-Type: application/json" -d '{"startTime":1692272319000, "message":"a message!!!"}'
You should see maintenances at GET api/maintenances/listCurrentAndUpcoming (this is what the frontend uses)
Frontend:
open wk without any maintenances -> should look as always
create an upcoming maintenance -> wk should show this in a banner. the banner should be closable. a page refresh should not show that banner anymore.
create a current maintennance -> wk should show this banner, but it should not be closable.
scrolling with the banner should behave intuitively.
in the annotation view, touching the banner with the mouse should move the banner to the bottom of the page (and vice versa)

TODOs:

Backend
- schema
- clarify permissions
- CRUD routes
Frontend
- Adapt to new format of maintenance rest api route (GET api/maintenances/listCurrentAndUpcoming)
- Banner for closest upcoming maintenance “Maintenance starting in 3h” with tooltip showing more details
- Banner for current maintenance “Current Maintenance” with tooltip showing more details

Issues:

fixes Maintenance banner #7257

(Please delete unneeded items, merge only when none are left open)

Updated changelog
Updated migration guide if applicable
Updated documentation if applicable
Adapted wk-libs python client if relevant API parts change
Removed dev-only changes like prints and application.conf edits
Considered common edge cases
Needs datastore update after deployment

fm3 · 2023-08-17T09:56:37Z

@normanrz I am unsure about the permission system here. Who should be allowed to create/update scheduled maintenances in the database?

Only superusers? Then we would have to promote the users who want to use this feature to superuser on their wk instances
All organization admins? But maintenances are not currently scoped to organizations, so that would give a lot of users on webknossos.org this permission
Or do we add an entry in the config to enable/disable this for “normal admins”?

normanrz · 2023-08-17T09:58:37Z

Either we do it just for superadmins, which seems fine to me, or we scope the maintenance banners to orgs.

app/controllers/MaintenanceController.scala

dieknolle3333 · 2023-09-05T15:27:58Z

I have some more questions :)

If there is a maintenance currently running, I'm assuming that only the "Current maintenance" banner should be shown? Or should the next upcoming maintenance still be displayed?
I am assuming that "more details" are the duration and the maintenance's message?

normanrz · 2023-09-05T15:53:26Z

If there is a maintenance currently running, I'm assuming that only the "Current maintenance" banner should be shown? Or should the next upcoming maintenance still be displayed?

Yes, either the current or the next maintenance should be shown. No more than 1 banner.

I am assuming that "more details" are the duration and the maintenance's message?

I think the message should be kept short to fit in the banner.

dieknolle3333 · 2023-09-06T16:52:25Z

this should be a basic frontend implementation without storing the maintenance info in the redux store (moved to new branch).
some useful timestamps: 1696024800000 is 30.09.23 00:00; 1695938400000 is 29.09.23 00:00; 1693778400000 is 4.9.23 00:00.

dieknolle3333 · 2023-09-07T07:28:42Z

@fm3 could you help me with the merge conflicts in MIGRATIONS.unreleased.md? I'm not sure how to resolve them

frontend/javascripts/maintenance_banner.tsx

philippotto

code looks quite good already, but I still have some feedback :)

philippotto · 2023-09-08T09:36:11Z

frontend/stylesheets/main.less

+.ant-alert-message {
+  margin-left: 5px;
+}
+.ant-alert-banner {
+  position: absolute;
+  z-index: 1000;
+  width: 100%;
+}


These are antd classes, right? Please move these (and the rule in line 121f) to antd_overwrites.less.

okay! when does something count as overwriting?
there are multiple CSS rules for antd in main.less, e.g. in l. 72 and I am not sure I understand when to write antd rules where

You are right, it's not very clear right now. I moved another rule, but the others rules mostly use antd class names in combination with other descriptors (e.g., #main-container > section > .ant-layout-header). This means that only specific occurrences of antd components are changed. In the overwrites module, on the other hand, the rules are mutating all occurrences of the mentioned antd components. So, this is can be used as a rule of thumb :)

frontend/javascripts/admin/admin_rest_api.ts

frontend/javascripts/maintenance_banner.tsx

philippotto · 2023-09-15T10:27:02Z

Nice! Works well 👍 However, there's an oddity about the banner when the user scrolls in the dashboard. See these screenshots:

I see two possible solutions:

use sticky instead of absolute so that the scrolling doesn't interfere
don't use the absolute positioning, but normal positioning (as it was in an earlier iteration) for all pages except for the tracing layout view. the "switch position to the bottom of the screen" would only be needed for the tracing layout view.

The first option is probably easier. I hope that it behaves well then. If not, the second option might be the better approach (since the layout in the tracing view should forbid scrolling anyway).

dieknolle3333 · 2023-09-15T14:21:12Z

with the first option I faced the problem that the lower position wasn't taken into account, so the switching didnt work any more. thus I implemented the second version. this leads to some views (at least Statistics > Overview) scrolling, not sure whether that is a problem. other than that it seems to be working fine!

…ongoing maintenance for 60s; fix store listening

philippotto · 2023-09-18T08:20:44Z

frontend/javascripts/maintenance_banner.tsx

 const INTERVAL_TO_FETCH_MAINTENANCES_MS = 60000;

 export function MaintenanceBanner() {
-  const { activeUser, uiInformation } = Store.getState();
+  const activeUser = useSelector((state: OxalisState) => state.activeUser);


@dieknolle3333 just fyi, I fixed the store access here. When rendering react components, it's always important to properly hook up with the store so that the component updates when the store updates. This listener can be set up with connect (from react-redux) or in case of functional components useSelector (which is better in my opinion since it's less verbose). Your access to Store.getState() is fine in event handlers (e.g., onClick handler in a button) when one doesn't need the synchronization. However, during rendering this would mean that the component gets outdated at some point.

oh, I understand!

philippotto

Great :) I fixed some smaller stuff and now think that the PR should be ready to go 🚀

Add banner for scheduled maintenance

f00d023

fm3 added backend frontend new feature labels Aug 17, 2023

fm3 assigned fm3 and dieknolle3333 Aug 17, 2023

WIP: new routes

7268a9d

fm3 added 2 commits August 17, 2023 13:27

implement remaining routes

fff2c75

migration guide

428c804

fm3 requested a review from frcroth September 4, 2023 09:39

frcroth reviewed Sep 4, 2023

View reviewed changes

app/controllers/MaintenanceController.scala Outdated Show resolved Hide resolved

rename variable

397ba3f

dieknolle3333 added 3 commits September 5, 2023 19:18

add first version of maintenance banner to frontend

7c88c33

format frontend

6b8827a

add maintenance banner to frontend

b838808

dieknolle3333 added 3 commits September 6, 2023 18:58

add changelog

dcca54b

remove comment

dee97f5

rename module

93ca188

dieknolle3333 requested a review from philippotto September 7, 2023 07:24

dieknolle3333 marked this pull request as ready for review September 7, 2023 07:29

fm3 and others added 3 commits September 7, 2023 10:54

Merge branch 'master' into maintenance-banner

84a0080

fix positioning issue for current maintenance banner

afe69af

unify position style

0f67867

dieknolle3333 reviewed Sep 7, 2023

View reviewed changes

frontend/javascripts/maintenance_banner.tsx Outdated Show resolved Hide resolved

philippotto reviewed Sep 8, 2023

View reviewed changes

dieknolle3333 added 5 commits September 14, 2023 20:47

WIP: start to address review

de052c8

address review

ff27ef6

merge master

c84b959

format

eb84892

capitalize constant and add const for status bar height

00e3a39

dieknolle3333 force-pushed the maintenance-banner branch from 591ac33 to 00e3a39 Compare September 15, 2023 05:48

fix formatting

fba8caf

dieknolle3333 and others added 4 commits September 15, 2023 16:41

fix positioning of maintenance banner in different views

a6867a8

pretty

5a828fb

add an initial fetch (after 5s) so that a page refresh won't show an …

7c2e6f4

…ongoing maintenance for 60s; fix store listening

move a css rule to antd overwrites

5b25707

philippotto reviewed Sep 18, 2023

View reviewed changes

philippotto approved these changes Sep 18, 2023

View reviewed changes

Merge branch 'master' into maintenance-banner

b6072bf

philippotto merged commit 21cc4c9 into master Sep 18, 2023
2 checks passed

philippotto deleted the maintenance-banner branch September 18, 2023 09:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add banner for scheduled maintenance #7284

Add banner for scheduled maintenance #7284

fm3 commented Aug 17, 2023 •

edited

fm3 commented Aug 17, 2023

normanrz commented Aug 17, 2023

dieknolle3333 commented Sep 5, 2023

normanrz commented Sep 5, 2023

dieknolle3333 commented Sep 6, 2023 •

edited

dieknolle3333 commented Sep 7, 2023

philippotto left a comment

philippotto Sep 8, 2023

dieknolle3333 Sep 14, 2023

philippotto Sep 18, 2023

philippotto commented Sep 15, 2023

dieknolle3333 commented Sep 15, 2023

philippotto Sep 18, 2023

dieknolle3333 Sep 25, 2023

philippotto left a comment

Add banner for scheduled maintenance #7284

Add banner for scheduled maintenance #7284

Conversation

fm3 commented Aug 17, 2023 • edited

URL of deployed dev instance (used for testing):

Steps to test:

TODOs:

Issues:

fm3 commented Aug 17, 2023

normanrz commented Aug 17, 2023

dieknolle3333 commented Sep 5, 2023

normanrz commented Sep 5, 2023

dieknolle3333 commented Sep 6, 2023 • edited

dieknolle3333 commented Sep 7, 2023

philippotto left a comment

Choose a reason for hiding this comment

philippotto Sep 8, 2023

Choose a reason for hiding this comment

dieknolle3333 Sep 14, 2023

Choose a reason for hiding this comment

philippotto Sep 18, 2023

Choose a reason for hiding this comment

philippotto commented Sep 15, 2023

dieknolle3333 commented Sep 15, 2023

philippotto Sep 18, 2023

Choose a reason for hiding this comment

dieknolle3333 Sep 25, 2023

Choose a reason for hiding this comment

philippotto left a comment

Choose a reason for hiding this comment

fm3 commented Aug 17, 2023 •

edited

dieknolle3333 commented Sep 6, 2023 •

edited