rabbitmq / rabbitmq-server

Open source RabbitMQ: core server and tier 1 (built-in) plugins
https://www.rabbitmq.com/
Other
12.33k stars 3.92k forks source link

rabbit_feature_flags: Rework the management UI page #12643

Closed dumbbell closed 3 weeks ago

dumbbell commented 3 weeks ago

Why

The "Feature flags" admin section had several issues:

How

The feature flags table is reworked and now displays:

Here is how the new table looks like:

Screenshot 2024-10-31 at 17-45-21 RabbitMQ Management

For stable feature flags, when a user click on the toggle, the toggle goes into an intermediate state while waiting for the response from the broker. If the response is successful, the toggle is green. Otherwise it goes back to red and the error is displayed in a popup as before.

For experimental feature flags, when a user click on the toggle, a popup is displayed to let the user know of the possible constraints and consequences, with one or two required checkboxes to tick so the user confirms they understand the message. The feature flag is enabled only after the user validates the popup. The displayed message and the checkboxes depend on if the experimental feature flag is supported or not (it is a new attribute of experimental feature flags).

Here is a screenshot of the confirmation popup for unsupported experimental feature flags:

Screenshot 2024-10-31 at 17-45-39 RabbitMQ Management

The request to enable feature flags now uses the modern fetch() API. Therefore it uses Javascript promises and does not block the main thread: the UI remains responsive while a migration callback runs.

Finally, an "Enable all stable feature flags" button has been added to the warning that tells the user some stable feature flags are still disabled.

Here is a screenshot of the warning with the added button:

image

dumbbell commented 3 weeks ago

As of this writing, the branch contains a commit that adds three feature flags just to help test the branch. They will be removed before the merge.

These feature flags are:

They all use an enable callback that throws an exception after sleeping two seconds.

CI will fail until they are removed because it will fail to enable a_stable_ff.

michaelklishin commented 3 weeks ago

This looks great.

mkuratczyk commented 3 weeks ago

There are two issues related to auto-refresh:

  1. (discussed yesterday with @dumbbell) When using the "enable all FFs button", if there's an FF migration that takes more than the page refresh, feature flags that will be enabled later (are "queued" to be enabled) will turn red (because their state is not changing - they are still disabled)
  2. when the experimental feature flag dialog appears with the warning, it disappears on page refresh

Both issues are relatively minor and the overall experience is already better. I don't insist we have to fix these.

dumbbell commented 3 weeks ago

@mkuratczyk: I finally found and implemented a fix for the issues you list where the auto-refresh is involved: I pause the auto-refresh timer and resume it at the end of handling feature flags.

I think there are still issues in this area. For example, if the user switches to another page and comes back. Or if they play with the auto-refresh dropdown menu at the top right. But I feel fixing this becomes out of scope for this patch. We can wait that a user reports them :)