From a01ec225dde56a12b20f3cb5d5399c126776d6b6 Mon Sep 17 00:00:00 2001 From: veronikaslc Date: Thu, 2 Feb 2023 03:42:23 -0500 Subject: [PATCH 1/5] CARDS-1877: Reset to initial settings in administration pages is broken --- .../main/frontend/src/questionnaireEditor/MarkdownText.jsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/modules/data-entry/src/main/frontend/src/questionnaireEditor/MarkdownText.jsx b/modules/data-entry/src/main/frontend/src/questionnaireEditor/MarkdownText.jsx index 0a0b275a60..a77a3c0904 100644 --- a/modules/data-entry/src/main/frontend/src/questionnaireEditor/MarkdownText.jsx +++ b/modules/data-entry/src/main/frontend/src/questionnaireEditor/MarkdownText.jsx @@ -17,7 +17,7 @@ // under the License. // -import React, { useState } from 'react'; +import React, { useState, useEffect } from 'react'; import PropTypes from 'prop-types'; import withStyles from '@mui/styles/withStyles'; @@ -46,6 +46,10 @@ let MarkdownText = (props) => { cmd.push(commands.divider); cmd.push(infoButton); + useEffect(() => { + setValue(props.value || ''); + }, [props.value]); + return ( {setValue(value); onChange && onChange(value);}} extraCommands={cmd}/> ) From 1519bec632029019285eff32f6acc6e8b04c67e0 Mon Sep 17 00:00:00 2001 From: Marta Girdea Date: Fri, 3 Feb 2023 03:24:13 -0500 Subject: [PATCH 2/5] CARDS-1877: Reset to initial settings in administration pages is broken * Bugfix: saving might be triggered before the state is updated with the original config * Bugfix: resetting to an empty value sometimes fails (e.g. downtime warning dates) --- .../src/DowntimeWarningConfiguration.jsx | 1 + .../src/adminDashboard/AdminConfigScreen.jsx | 28 +++++++++++++++---- .../themePage/QuickSearchConfiguration.jsx | 1 + .../DashboardSettingsConfiguration.jsx | 1 + .../PatientAccessConfiguration.jsx | 1 + .../SurveyInstructionsConfiguration.jsx | 1 + .../src/patient-portal/ToUConfiguration.jsx | 1 + 7 files changed, 28 insertions(+), 6 deletions(-) diff --git a/modules/downtime-warning-banner/src/main/frontend/src/DowntimeWarningConfiguration.jsx b/modules/downtime-warning-banner/src/main/frontend/src/DowntimeWarningConfiguration.jsx index d682649314..05f2deec5e 100644 --- a/modules/downtime-warning-banner/src/main/frontend/src/DowntimeWarningConfiguration.jsx +++ b/modules/downtime-warning-banner/src/main/frontend/src/DowntimeWarningConfiguration.jsx @@ -76,6 +76,7 @@ function DowntimeWarningConfiguration() { ({ * * * @param {string} title - the title of the administration screen, required - * @param {string} configPath - the path of the configuration node; will be used for loading and saving the data + * @param {string} configPath - the path of the configuration node; will be used for loading and saving the data, required + * @param {object} configTemplate - what an empty config would look like; should list all the possible fields with empty values, required * @param {function} onConfigFetched - handler to pass the loaded configuration json to the parent component * @param {boolean} hasChanges - how the parent component notifies that the user changed the form values * @param {string} configError - how the parent component notifies that there's a data sanity issue @@ -97,7 +98,7 @@ const useStyles = makeStyles(theme => ({ */ function AdminConfigScreen(props) { - const { title, configPath, onConfigFetched, hasChanges, configError, buildConfigData, onConfigSaved, children } = props; + const { title, configPath, configTemplate, onConfigFetched, hasChanges, configError, buildConfigData, onConfigSaved, children } = props; const [ config, setConfig ] = useState(); const [ configIsInitial, setConfigIsInitial ] = useState(true); const [ error, setError ] = useState(); @@ -115,8 +116,9 @@ function AdminConfigScreen(props) { fetchWithReLogin(globalContext, `${configPath}.json`) .then((response) => response.ok ? response.json() : Promise.reject(response)) .then(json => { - setConfig(json); - onConfigFetched?.(json); + let conf = Object.assign({}, configTemplate, json); + setConfig(conf); + onConfigFetched?.(conf); }) .catch(err => { setConfig(null); @@ -138,7 +140,20 @@ function AdminConfigScreen(props) { // Build formData object. let formData = new URLSearchParams(); - buildConfigData?.(formData); + if (event) { + // If the submit is user-initiated, grab values form the form + buildConfigData?.(formData); + } else { + // Otherwise just save the existing config + // Use the template keys to avoid attempts to save reserved properties like "jcr:...", "sling:...", "@..." + for (let key of Object.keys(configTemplate)) { + if (Array.isArray(config[key])) { + config[key].forEach(v => formData.append(key, v)); + } else { + formData.append(key, config[key]); + } + } + } fetchWithReLogin(globalContext, configPath, { @@ -234,7 +249,8 @@ function AdminConfigScreen(props) { AdminConfigScreen.propTypes = { title: PropTypes.string.isRequired, - configPath: PropTypes.string, + configPath: PropTypes.string.isRequired, + configTemplate: PropTypes.object.isRequired, onConfigFetched: PropTypes.func, hasChanges: PropTypes.bool, configError: PropTypes.string, diff --git a/modules/homepage/src/main/frontend/src/themePage/QuickSearchConfiguration.jsx b/modules/homepage/src/main/frontend/src/themePage/QuickSearchConfiguration.jsx index de7047ceb1..9e9a7da4d8 100644 --- a/modules/homepage/src/main/frontend/src/themePage/QuickSearchConfiguration.jsx +++ b/modules/homepage/src/main/frontend/src/themePage/QuickSearchConfiguration.jsx @@ -71,6 +71,7 @@ function QuickSearchConfiguration(props) { ({...t, [k.key] : ""}), {})} onConfigFetched={readDashboardSettings} hasChanges={hasChanges} buildConfigData={buildConfigData} diff --git a/modules/patient-portal/src/main/frontend/src/patient-portal/PatientAccessConfiguration.jsx b/modules/patient-portal/src/main/frontend/src/patient-portal/PatientAccessConfiguration.jsx index 3fa1b95594..94c41f7a86 100644 --- a/modules/patient-portal/src/main/frontend/src/patient-portal/PatientAccessConfiguration.jsx +++ b/modules/patient-portal/src/main/frontend/src/patient-portal/PatientAccessConfiguration.jsx @@ -107,6 +107,7 @@ function PatientAccessConfiguration() { ({...t, [k] : ""}), {})} onConfigFetched={setPatientAccessConfig} hasChanges={hasChanges} buildConfigData={buildConfigData} diff --git a/modules/patient-portal/src/main/frontend/src/patient-portal/SurveyInstructionsConfiguration.jsx b/modules/patient-portal/src/main/frontend/src/patient-portal/SurveyInstructionsConfiguration.jsx index cda9f815fc..1a6f2538b5 100644 --- a/modules/patient-portal/src/main/frontend/src/patient-portal/SurveyInstructionsConfiguration.jsx +++ b/modules/patient-portal/src/main/frontend/src/patient-portal/SurveyInstructionsConfiguration.jsx @@ -75,6 +75,7 @@ function SurveyInstructionsConfiguration() { ({...t, [k]: getUnsetValue(k)}), {})} onConfigFetched={setSurveyInstructions} hasChanges={hasChanges} buildConfigData={buildConfigData} diff --git a/modules/patient-portal/src/main/frontend/src/patient-portal/ToUConfiguration.jsx b/modules/patient-portal/src/main/frontend/src/patient-portal/ToUConfiguration.jsx index 960ec183cf..9c65a01882 100644 --- a/modules/patient-portal/src/main/frontend/src/patient-portal/ToUConfiguration.jsx +++ b/modules/patient-portal/src/main/frontend/src/patient-portal/ToUConfiguration.jsx @@ -66,6 +66,7 @@ function ToUConfiguration() { Date: Fri, 3 Feb 2023 03:29:19 -0500 Subject: [PATCH 3/5] CARDS-1877: Reset to initial settings in administration pages is broken Improvement: don't infer that the form has changes when we're just reverting to the original config Side effect: when the admin page has just been opened and the form is untouched, the save button is disabled --- .../src/DowntimeWarningConfiguration.jsx | 10 +++------- .../themePage/QuickSearchConfiguration.jsx | 8 ++------ .../DashboardSettingsConfiguration.jsx | 8 ++------ .../PatientAccessConfiguration.jsx | 13 ++++++------- .../SurveyInstructionsConfiguration.jsx | 19 ++++++++++++------- .../src/patient-portal/ToUConfiguration.jsx | 12 ++++-------- 6 files changed, 29 insertions(+), 41 deletions(-) diff --git a/modules/downtime-warning-banner/src/main/frontend/src/DowntimeWarningConfiguration.jsx b/modules/downtime-warning-banner/src/main/frontend/src/DowntimeWarningConfiguration.jsx index 05f2deec5e..a86bdafedd 100644 --- a/modules/downtime-warning-banner/src/main/frontend/src/DowntimeWarningConfiguration.jsx +++ b/modules/downtime-warning-banner/src/main/frontend/src/DowntimeWarningConfiguration.jsx @@ -68,10 +68,6 @@ function DowntimeWarningConfiguration() { setDateRangeIsInvalid(fromDate && toDate && new Date(toDate).valueOf() < new Date(fromDate).valueOf()); }, [fromDate, toDate]); - useEffect(() => { - setHasChanges(true); - }, [enabled, fromDate, toDate]); - return ( setEnabled(event.target.checked) } + onChange={ event => { setEnabled(event.target.checked); setHasChanges(true); } } name="enabled" /> } @@ -103,7 +99,7 @@ function DowntimeWarningConfiguration() { type="datetime-local" InputLabelProps={{ shrink: true }} className={classes.textField} - onChange={(event) => setFromDate(event.target.value) } + onChange={(event) => { setFromDate(event.target.value); setHasChanges(true); } } onBlur={(event) => setFromDate(event.target.value) } placeholder={dateFormat.toLowerCase()} value={fromDate || ""} @@ -116,7 +112,7 @@ function DowntimeWarningConfiguration() { type="datetime-local" InputLabelProps={{ shrink: true }} className={classes.textField} - onChange={(event) => setToDate(event.target.value) } + onChange={(event) => { setToDate(event.target.value); setHasChanges(true); } } onBlur={(event) => setToDate(event.target.value) } placeholder={dateFormat.toLowerCase()} value={toDate || ""} diff --git a/modules/homepage/src/main/frontend/src/themePage/QuickSearchConfiguration.jsx b/modules/homepage/src/main/frontend/src/themePage/QuickSearchConfiguration.jsx index 9e9a7da4d8..35b87a9394 100644 --- a/modules/homepage/src/main/frontend/src/themePage/QuickSearchConfiguration.jsx +++ b/modules/homepage/src/main/frontend/src/themePage/QuickSearchConfiguration.jsx @@ -63,10 +63,6 @@ function QuickSearchConfiguration(props) { setAllowedResourceTypes(types); } - useEffect(() => { - setHasChanges(true); - }, [limit, showTotalRows]); - return ( setLimit(event.target.value) } + onChange={ event => { setLimit(event.target.value); setHasChanges(true); } } style={{'width' : '250px'}} helperText="How many results should be displayed" /> @@ -99,7 +95,7 @@ function QuickSearchConfiguration(props) { control={ setShowTotalRows(event.target.checked) } + onChange={ event => { setShowTotalRows(event.target.checked); setHasChanges(true); } } name="showTotalRows" /> } diff --git a/modules/patient-portal/src/main/frontend/src/patient-portal/DashboardSettingsConfiguration.jsx b/modules/patient-portal/src/main/frontend/src/patient-portal/DashboardSettingsConfiguration.jsx index 1d4da7283f..15442b475e 100644 --- a/modules/patient-portal/src/main/frontend/src/patient-portal/DashboardSettingsConfiguration.jsx +++ b/modules/patient-portal/src/main/frontend/src/patient-portal/DashboardSettingsConfiguration.jsx @@ -51,10 +51,6 @@ function DashboardSettingsConfiguration() { type: "boolean" }]; - useEffect(() => { - setHasChanges(true); - }, [enableTimeTabs, eventsLabel, eventTimeLabel]); - let buildConfigData = (formData) => { formData.append('enableTimeTabs', enableTimeTabs); formData.append('eventsLabel', eventsLabel); @@ -91,14 +87,14 @@ function DashboardSettingsConfiguration() { type="text" label={camelCaseToWords(field.key)} value={field.value} - onChange={(event) => { field.setter(event.target.value); }} + onChange={(event) => { field.setter(event.target.value); setHasChanges(true); }} /> : field.setter(event.target.checked) } + onChange={ event => { field.setter(event.target.checked); setHasChanges(true); } } name={field.key} /> } diff --git a/modules/patient-portal/src/main/frontend/src/patient-portal/PatientAccessConfiguration.jsx b/modules/patient-portal/src/main/frontend/src/patient-portal/PatientAccessConfiguration.jsx index 94c41f7a86..5511d5716f 100644 --- a/modules/patient-portal/src/main/frontend/src/patient-portal/PatientAccessConfiguration.jsx +++ b/modules/patient-portal/src/main/frontend/src/patient-portal/PatientAccessConfiguration.jsx @@ -66,10 +66,6 @@ function PatientAccessConfiguration() { } } - useEffect(() => { - setHasChanges(true); - }, [patientAccessConfig]); - let renderConfigCheckbox = (key, valueOverride) => ( setPatientAccessConfig({...patientAccessConfig, [key]: valueOverride || event.target.checked})} + onChange={event => { + setPatientAccessConfig({...patientAccessConfig, [key]: valueOverride || event.target.checked}); + setHasChanges(true); + }} />} label={labels[key]} /> @@ -91,8 +90,8 @@ function PatientAccessConfiguration() { setPatientAccessConfig({...patientAccessConfig, [key]: event.target.value})} - onBlur={event => setPatientAccessConfig({...patientAccessConfig, [key]: event.target.value})} + onChange={event => { setPatientAccessConfig({...patientAccessConfig, [key]: event.target.value}); setHasChanges(true); }} + onBlur={event => { setPatientAccessConfig({...patientAccessConfig, [key]: event.target.value}); setHasChanges(true); }} placeholder={DEFAULT_PATIENT_ACCESS_CONFIG[key] || ""} value={patientAccessConfig?.[key]} InputProps={{ diff --git a/modules/patient-portal/src/main/frontend/src/patient-portal/SurveyInstructionsConfiguration.jsx b/modules/patient-portal/src/main/frontend/src/patient-portal/SurveyInstructionsConfiguration.jsx index 1a6f2538b5..0267e2e524 100644 --- a/modules/patient-portal/src/main/frontend/src/patient-portal/SurveyInstructionsConfiguration.jsx +++ b/modules/patient-portal/src/main/frontend/src/patient-portal/SurveyInstructionsConfiguration.jsx @@ -67,10 +67,6 @@ function SurveyInstructionsConfiguration() { let getUnsetValue = (key) => (key?.startsWith("enable") ? false : ""); - useEffect(() => { - setHasChanges(true); - }, [surveyInstructions]); - return ( { setSurveyInstructions({...surveyInstructions, [key]: text}); }} + onChange={(text) => { + setSurveyInstructions({...surveyInstructions, [key]: text}); + setHasChanges(true); + }} /> : key.startsWith("enable") ? setSurveyInstructions({...surveyInstructions, [key]: !!event.target.checked})} + onChange={event => { + setSurveyInstructions({...surveyInstructions, [key]: !!event.target.checked}); + setHasChanges(true); + }} />} label={camelCaseToWords(key)} /> @@ -114,7 +116,10 @@ function SurveyInstructionsConfiguration() { label={camelCaseToWords(key)} value={surveyInstructions?.[key] || ""} placeholder={DEFAULT_INSTRUCTIONS[key] || ""} - onChange={(event) => { setSurveyInstructions({...surveyInstructions, [key]: event.target.value}); }} + onChange={(event) => { + setSurveyInstructions({...surveyInstructions, [key]: event.target.value}); + setHasChanges(true); + }} fullWidth /> } diff --git a/modules/patient-portal/src/main/frontend/src/patient-portal/ToUConfiguration.jsx b/modules/patient-portal/src/main/frontend/src/patient-portal/ToUConfiguration.jsx index 9c65a01882..9adab76846 100644 --- a/modules/patient-portal/src/main/frontend/src/patient-portal/ToUConfiguration.jsx +++ b/modules/patient-portal/src/main/frontend/src/patient-portal/ToUConfiguration.jsx @@ -58,10 +58,6 @@ function ToUConfiguration() { formData.append('text', text); } - useEffect(() => { - setHasChanges(true); - }, [title, version, text, acceptanceRequired]); - return ( { setTitle(event.target.value); }} + onChange={(event) => { setTitle(event.target.value); setHasChanges(true); }} /> @@ -97,7 +93,7 @@ function ToUConfiguration() { type="version" label="Version" value={version} - onChange={(event) => { setVersion(event.target.value); }} + onChange={(event) => { setVersion(event.target.value); setHasChanges(true); }} style={{'width' : '250px'}} /> @@ -106,7 +102,7 @@ function ToUConfiguration() { { setText(value); }} + onChange={value => { setText(value); setHasChanges(true); }} /> @@ -114,7 +110,7 @@ function ToUConfiguration() { control={ { setAcceptanceRequired(event.target.checked); }} + onChange={(event) => { setAcceptanceRequired(event.target.checked); setHasChanges(true); }} name="acceptanceRequired" /> } From c9c20584919bdace9b0acc8b9c6079fa19cd1932 Mon Sep 17 00:00:00 2001 From: Marta Girdea Date: Sun, 5 Feb 2023 20:38:24 -0500 Subject: [PATCH 4/5] CARDS-1877: Reset to initial settings in administration pages is broken Bugfix: cannot save an empty selection of resources in `allowedResourceTypes` --- .../frontend/src/adminDashboard/AdminConfigScreen.jsx | 10 +++++----- .../src/themePage/QuickSearchConfiguration.jsx | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/homepage/src/main/frontend/src/adminDashboard/AdminConfigScreen.jsx b/modules/homepage/src/main/frontend/src/adminDashboard/AdminConfigScreen.jsx index 1533014578..d5c8af6539 100644 --- a/modules/homepage/src/main/frontend/src/adminDashboard/AdminConfigScreen.jsx +++ b/modules/homepage/src/main/frontend/src/adminDashboard/AdminConfigScreen.jsx @@ -143,15 +143,15 @@ function AdminConfigScreen(props) { if (event) { // If the submit is user-initiated, grab values form the form buildConfigData?.(formData); + // Use the config template to Fill in any missing values + for (let key of Object.keys(configTemplate)) { + formData.get(key) == null && Array.of(configTemplate[key]).flat().forEach(v => formData.append(key, v)); + } } else { // Otherwise just save the existing config // Use the template keys to avoid attempts to save reserved properties like "jcr:...", "sling:...", "@..." for (let key of Object.keys(configTemplate)) { - if (Array.isArray(config[key])) { - config[key].forEach(v => formData.append(key, v)); - } else { - formData.append(key, config[key]); - } + Array.of(config[key]).flat().forEach(v => formData.append(key, v)); } } diff --git a/modules/homepage/src/main/frontend/src/themePage/QuickSearchConfiguration.jsx b/modules/homepage/src/main/frontend/src/themePage/QuickSearchConfiguration.jsx index 35b87a9394..7f57fb2790 100644 --- a/modules/homepage/src/main/frontend/src/themePage/QuickSearchConfiguration.jsx +++ b/modules/homepage/src/main/frontend/src/themePage/QuickSearchConfiguration.jsx @@ -67,7 +67,7 @@ function QuickSearchConfiguration(props) { Date: Sun, 5 Feb 2023 20:40:19 -0500 Subject: [PATCH 5/5] [misc] In the Downtime warning admin screen, console errors appear complaining of non-bolean values passed to boolean props --- .../src/main/frontend/src/DowntimeWarningConfiguration.jsx | 4 ++-- .../main/frontend/src/adminDashboard/AdminConfigScreen.jsx | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/downtime-warning-banner/src/main/frontend/src/DowntimeWarningConfiguration.jsx b/modules/downtime-warning-banner/src/main/frontend/src/DowntimeWarningConfiguration.jsx index a86bdafedd..4f907fa4e6 100644 --- a/modules/downtime-warning-banner/src/main/frontend/src/DowntimeWarningConfiguration.jsx +++ b/modules/downtime-warning-banner/src/main/frontend/src/DowntimeWarningConfiguration.jsx @@ -65,7 +65,7 @@ function DowntimeWarningConfiguration() { useEffect(() => { // Determine if the end date is earlier than the start date - setDateRangeIsInvalid(fromDate && toDate && new Date(toDate).valueOf() < new Date(fromDate).valueOf()); + setDateRangeIsInvalid(!!fromDate && !!toDate && new Date(toDate).valueOf() < new Date(fromDate).valueOf()); }, [fromDate, toDate]); return ( @@ -75,7 +75,7 @@ function DowntimeWarningConfiguration() { configTemplate={{enabled: false, fromDate: "", toDate: ""}} onConfigFetched={readDowntimeWarningSettings} hasChanges={hasChanges} - configError={dateRangeIsInvalid ? "Invalid date range" : undefined} + configError={!!dateRangeIsInvalid ? "Invalid date range" : undefined} buildConfigData={buildConfigData} onConfigSaved={() => setHasChanges(false)} > diff --git a/modules/homepage/src/main/frontend/src/adminDashboard/AdminConfigScreen.jsx b/modules/homepage/src/main/frontend/src/adminDashboard/AdminConfigScreen.jsx index d5c8af6539..495f665baa 100644 --- a/modules/homepage/src/main/frontend/src/adminDashboard/AdminConfigScreen.jsx +++ b/modules/homepage/src/main/frontend/src/adminDashboard/AdminConfigScreen.jsx @@ -204,7 +204,7 @@ function AdminConfigScreen(props) { variant="contained" color="primary" size="small" - disabled={configError || !hasChanges} + disabled={!!configError || !hasChanges} > Save