Improvement to maintenance banner site setting (#5887)

* added .required validation on value field

* changed the toast message inside default case of handleError

* fixes to maintenanaceBanner TextForm

Improvements done :

1. Both settext and clearbanner buttons are of type brand. I changed clearbanner button to variant danger.

2. The inputbox was change from input to textarea. As here the admin would add some message hence it should look like textarea.

3. We have use localstorage to store maintenanceBannerId. Here in our code we fetch that value and pass to toast.dismiss function. As defined in documentation if the value passed to toast.dismiss is null then instead of raising any issue, it dismiss all the toast currently active which is not good for our application. Only required toast should be dismissed if its active else we should not called toast.dismiss.
   Now localstorage.getItem()  returns null if there is no stored value corresponding to key passed to it. Hence this case should be handled.

4. Its good practice that after we extract the value from localstorage and if that value is no longer needed we should remove that key-value pair from localstorage. The same is updated in the code. After dismissing the toast we no longer need the maintenanceBannerId key-value in our localstorage.

5. Both the buttons settext and clearbanner are using spinner to show loading state but both the spinner are depending on same variable `updateSiteSettingsAPI.isLoading`. So both buttons goes in loading state at same time no matter which button is clicked. This is an undesired behavior so I have used two different variable for handling mutations on each button click. This way we can distinguish loading state for both the buttons.
   const updateSiteSettingsAPISetText = useUpdateSiteSettingsAPI();
   const updateSiteSettingsAPIClearText = useUpdateSiteSettingsAPI();

6. The issue i detected was, when some message is added to input box and settext button is clicked, we see a banner toast coming. But further when that message is updated, and settext is clicked another toast pops up with updated message. The catch is previous toast still persists. It gets removed only when the page is refreshed. So a better way would be to clear the banner toast if it exists and then display the updated banner toast.

7. There is an performance issue on the buttons click for both settext and clearbanner. Even if no text is inputted still we can click both the buttons which leads to unnecessary PATCH request to my api. This behavior i tried to stop by sending PATCH request only if input box is not empty. So i added .required() validation using yup.
   Now settext wont lead to PATCH request if nothing is inputted. Also on empty input box clearbanner will not send any PATCH request.

   Further optimization is done to both the buttons to send PATCH request only when there is actual change to message. I prevented api call if settetxt button is clicked unnecessary without any change to message. For this i have introduced a useRef() variable which tracks the value of banner message stored in db. This variable is modified accordingly when PATCH request to send.

8. In useUpdateSiteSetting.jsx hook, the way error is handled is not proper.
   On error we are calling handleError from FileValidationHelper.jsx which is basically designed to handle error assuming that there is issue with file upload.
   In default case block we simply tell that there is file upload error. But this might not be apt for all cases.
   For any invalid setting name, site_setting sets to nil and we return not_found error. But this error is not handled inside useUpdateSiteSetting.jsx. Instead we are sending it to FileValidationHelper.jsx which treats this as upload error.
   Hence a better and simple way would be to change the default case error message inside FileValidationHelper,jsx to a generic error message. This will cover all the cases.

app/assets/locales/en.json

@@ -538,6 +538,11 @@
       },
       "url": {
         "invalid": "Invalid URL"
+      },
+      "text_form": {
+        "value": {
+          "required": "Please enter some message"
+        }
       }
     },
     "room": {

app/javascript/components/admin/site_settings/administration/TextForm.jsx

@@ -14,7 +14,7 @@
 // You should have received a copy of the GNU Lesser General Public License along
 // with Greenlight; if not, see <http://www.gnu.org/licenses/>.
 
-import React, { useEffect } from 'react';
+import React, { useCallback, useEffect, useRef } from 'react';
 import PropTypes from 'prop-types';
 import { Button } from 'react-bootstrap';
 import { useTranslation } from 'react-i18next';
@@ -25,39 +25,80 @@ import FormControl from '../../../shared_components/forms/FormControl';
 import useTextForm from '../../../../hooks/forms/admin/site_settings/useTextForm';
 
 export default function TextForm({ id, value, mutation: useUpdateSiteSettingsAPI }) {
-  const updateSiteSettingsAPI = useUpdateSiteSettingsAPI();
+  const updateSiteSettingsAPISetText = useUpdateSiteSettingsAPI();
+  const updateSiteSettingsAPIClearText = useUpdateSiteSettingsAPI();
+
   const { t } = useTranslation();
-  const maintenanceBannerId = localStorage.getItem('maintenanceBannerId');
 
   const { methods, fields } = useTextForm({ defaultValues: { value } });
 
+  const formText = useRef('');
+
   useEffect(() => {
-    if (!methods) { return; }
-    methods.reset({ value });
+    if (methods) {
+      methods.reset({ value });
+      formText.current = value;
+    }
   }, [methods, value]);
 
+  const dismissMaintenanceBannerToast = () => {
+    const maintenanceBannerId = localStorage.getItem('maintenanceBannerId');
+    if (maintenanceBannerId) {
+      toast.dismiss(maintenanceBannerId);
+      localStorage.removeItem('maintenanceBannerId');
+    }
+  };
+
   // Function to clear the form
   const clearForm = () => {
     methods.reset({ value: '' });
-    toast.dismiss(maintenanceBannerId);
-    updateSiteSettingsAPI.mutate('');
+    dismissMaintenanceBannerToast();
+    if (formText.current) {
+      formText.current = '';
+      updateSiteSettingsAPIClearText.mutate('');
+    }
   };
 
+  const handleSubmit = useCallback((formData) => {
+    if (formText.current !== formData[`${fields.value.hookForm.id}`]) {
+      dismissMaintenanceBannerToast();
+      formText.current = formData[`${fields.value.hookForm.id}`];
+      return updateSiteSettingsAPISetText.mutate(formData);
+    }
+    return null;
+  }, [updateSiteSettingsAPISetText.mutate]);
+
   return (
-    <Form id={id} methods={methods} onSubmit={updateSiteSettingsAPI.mutate}>
+    <Form id={id} methods={methods} onSubmit={handleSubmit}>
       <FormControl
         field={fields.value}
         aria-describedby={`${id}-submit-btn`}
         type="text"
+        as="textarea"
+        rows={3}
         noLabel
       />
-      <Button id={`${id}-clear-btn`} className="mb-2 float-end" variant="brand" onClick={clearForm} disabled={updateSiteSettingsAPI.isLoading}>
-        {updateSiteSettingsAPI.isLoading && <Spinner className="me-2" />}
-        { t('admin.site_settings.administration.clear_banner') }
+      <Button
+        id={`${id}-clear-btn`}
+        className="mb-2 float-end"
+        variant="danger"
+        onClick={clearForm}
+        disabled={updateSiteSettingsAPIClearText.isLoading}
+      >
+        {updateSiteSettingsAPIClearText.isLoading && (
+          <Spinner className="me-2" />
+        )}
+        {t('admin.site_settings.administration.clear_banner')}
       </Button>
-      <Button id={`${id}-submit-btn`} className="mb-2 float-end me-2" variant="brand" type="submit" disabled={updateSiteSettingsAPI.isLoading}>
-        {updateSiteSettingsAPI.isLoading && <Spinner className="me-2" />}
-        { t('admin.site_settings.administration.set_text') }
+      <Button
+        id={`${id}-submit-btn`}
+        className="mb-2 float-end me-2"
+        variant="brand"
+        type="submit"
+        disabled={updateSiteSettingsAPISetText.isLoading}
+      >
+        {updateSiteSettingsAPISetText.isLoading && <Spinner className="me-2" />}
+        {t('admin.site_settings.administration.set_text')}
       </Button>
     </Form>
   );

app/javascript/helpers/FileValidationHelper.jsx

@@ -68,6 +68,6 @@ export const handleError = (error, t, toast) => {
       toast.error(t('toast.error.file_type_not_supported'));
       break;
     default:
-      toast.error(t('toast.error.file_upload_error'));
+      toast.error(t('toast.error.problem_completing_action'));
   }
 };

app/javascript/hooks/forms/admin/site_settings/useTextForm.js

@@ -23,6 +23,8 @@ import { useCallback, useMemo } from 'react';
 export function useTextFormValidation() {
   return useMemo(() => (yup.object({
     // future add text validations
+    value: yup.string()
+      .required('forms.validations.text_form.value.required'),
   })), []);
 }