Room administration: "notification_emails" field is often shown empty even if mails are set

[olifre]

Describe the bug
After entering and saving mail addresses in the field notification_emails in the room booking administration when editing a room's details (on the page "Notifications", labelled "Notification emails"), the field is often shown as "empty" (i.e. with the standard placeholder text) when entering the same menu again. Opening the page up many times, sometimes the actual content is shown.

To Reproduce
Steps to reproduce the behavior:

1. Go to "Room Booking", enter "Administration" via the hamburger menu, then click on a location and edit a room via the pen.
2. Click on "Notifications".
3. Enter a mail address in the field "Notification emails" and save it.
4. Completely refresh the page / re-enter room booking administration, i.e. repeat step 1.
5. Click on "Notifications" again.
6. The field is shown empty, in about 1 out of 5 attempts.

Expected behavior
Field content to be shown every time.

Additional context
Tested with Firefox 111.0 on Linux, colleagues using Firefox can also reproduce it.
Testing with Google Chrome 112 on Linux, the chance to see the content of the field seems higher (about 4 in 5 times, the content is visible).

Checking with web developer tools, the JSON response always contains the field content, so this is likely something furhter down the road, i.e. some kind of race condition in Javascript?

Note: This is with Indico v3.2.3.

[ThiefMaster]

This seems to fix it:

[adrian@eluvian:~/dev/indico/py3/src:master *]> git diff
diff --git a/indico/web/client/js/react/components/EmailListField.jsx b/indico/web/client/js/react/components/EmailListField.jsx
index 307969a873..8dbab6b240 100644
--- a/indico/web/client/js/react/components/EmailListField.jsx
+++ b/indico/web/client/js/react/components/EmailListField.jsx
@@ -21,12 +21,11 @@ const isValid = value => /^\S+@\S+\.\S+$/.test(value);
  */
 const EmailListField = props => {
   const {value, disabled, onChange, onFocus, onBlur} = props;
-  const [options, setOptions] = useState(value.filter(isValid).map(x => ({text: x, value: x})));
   const [searchQuery, setSearchQuery] = useState('');
+  const options = value.filter(isValid).map(x => ({text: x, value: x}));

   const setValue = newValue => {
     newValue = _.uniq(newValue.filter(isValid));
-    setOptions(newValue.map(x => ({text: x, value: x})));
     onChange(newValue);
     onFocus();
     onBlur();

Doesn't have any negative side-effects at a first glance, and putting the options into local state instead of relying on the value (which is managed by the final-form lib) was wrong anyway.

I still need to test it in the other place where the EmailListField is used though (after the easter holidays).

[olifre]

Thanks, that sounds great — if we had a dedicated test instance, I would go ahead and test it, too.
Happy Easter holidays!

[ThiefMaster]

local dev instance ftw for this kind of stuff ;)