DR-187 | Fix updateReport function to handle error messages properly

dataminelab · Apr 30, 2024 · 640891e · 640891e
1 parent 29c5b2e
commit 640891e
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 9 deletions.
diff --git a/client/app/pages/reports/components/ReportPageHeader.jsx b/client/app/pages/reports/components/ReportPageHeader.jsx
@@ -254,7 +254,7 @@ export default function ReportPageHeader(props) {
         }
         setModels(newModels);
         props.onChange(extend(report.clone(), { ...updates }));
-        updateReport(updates, { successMessage: null });
+        updateReport(updates, { successMessage: null, errorMessage: null });
         handleReportChanged(true);
         recordEvent("update", "report", report.id, { data_source_id });
       } catch (err) {
@@ -292,7 +292,7 @@ export default function ReportPageHeader(props) {
         }
         if (signal && signal.aborted) return;
         props.onChange(extend(report.clone(), { ...updates }));
-        updateReport({...report.clone(), ...updates}, { successMessage: null }); // show message only on error
+        updateReport({...report.clone(), ...updates}, { successMessage: null, errorMessage: null }); // show message only on error
         handleReportChanged(true);
         setSelectedModel(modelId);
       } catch (err) {
@@ -305,7 +305,7 @@ export default function ReportPageHeader(props) {
   const handleIdChange = useCallback(async id => {
     recordEvent("update", "report", report.id, { id });
     setReport(extend(report.clone(), { id }));
-    updateReport({ id }, { successMessage: null });
+    updateReport({ id }, { successMessage: null, errorMessage: null });
     handleReportChanged(true);
   });
 
@@ -400,7 +400,7 @@ export default function ReportPageHeader(props) {
   );
 
   useEffect(() => {
-    updateReport(report, { successMessage: null });
+    updateReport(report, { successMessage: null, errorMessage: null });
   }, [report.expression, window.location.hash]);
 
   useEffect(() => {

diff --git a/client/app/pages/reports/hooks/useUpdateReport.jsx b/client/app/pages/reports/hooks/useUpdateReport.jsx
@@ -48,7 +48,7 @@ function confirmOverwrite() {
   });
 }
 
-function doSaveReport(data, { canOverwrite = false } = {}) {
+function doSaveReport(data, { canOverwrite = false, errorMessage = "Report could not be saved" } = {}) {
   if (isObject(data.options) && data.options.parameters) {
     data.options = {
       ...data.options,
@@ -71,15 +71,16 @@ function doSaveReport(data, { canOverwrite = false } = {}) {
     if (error.name == "TypeError") {
       return Promise.reject();
     }
-    return Promise.reject(new SaveReportError("Report could not be saved"));
+    if (errorMessage) return Promise.reject(new SaveReportError(errorMessage));
+    return Promise.reject();
   });
 }
 
 export default function useUpdateReport(report, onChange) {
   const handleChange = useImmutableCallback(onChange);
 
   return useCallback(
-    (data = null, { successMessage = "Report saved" } = {}) => {
+    (data = null, { successMessage = "Report saved", errorMessage = "Report could not be saved" } = {}) => {
       if (isObject(data)) {
         // Don't save new report with partial data
         if (report.isNew && report.isNew()) {
@@ -108,7 +109,7 @@ export default function useUpdateReport(report, onChange) {
         ]);
       }
       if (!report.expression && !report.hash) return 0;
-      return doSaveReport(data, { canOverwrite: report.can_edit })
+      return doSaveReport(data, { canOverwrite: report.can_edit, errorMessage: errorMessage })
         .then(updatedReport => {
           if (!updatedReport || updatedReport.message === 'No changes made') return;
           if (!isNil(successMessage)) {

diff --git a/redash/handlers/reports.py b/redash/handlers/reports.py
@@ -225,7 +225,7 @@ def delete(self, report_id):
 
 # /api/reports/<int:report_id>
 class ReportResource(BaseResource):
-    ''' A resource for a single report creation, editing and deleting '''
+    ''' A resource for a single report viewing, creating, editing and deleting '''
 
     @require_permission("view_report")
     def get(self, report_id: int):