From 51f49350d8e7f999c15333002c098b91e9ab1874 Mon Sep 17 00:00:00 2001 From: Dave Roberts <145559566+droberts-ctrlo@users.noreply.github.com> Date: Fri, 24 Jan 2025 13:25:50 +0000 Subject: [PATCH] Autosave fix (#508) * Refactored code and added await to stop race conditions * Added relevant comments * Further updates to mitigate curval errors * Removed console.log * Removed console statement * Removed other console statements * Added and updated comments as appropriate * Removed unusual undef code --- .../form-group/autosave/lib/modal.js | 36 +++++++++++++------ .../modal/modals/curval/lib/component.js | 5 +-- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/src/frontend/components/form-group/autosave/lib/modal.js b/src/frontend/components/form-group/autosave/lib/modal.js index 8500a8e42..95638700c 100644 --- a/src/frontend/components/form-group/autosave/lib/modal.js +++ b/src/frontend/components/form-group/autosave/lib/modal.js @@ -1,5 +1,6 @@ import { setFieldValues } from "set-field-values"; import AutosaveBase from './autosaveBase'; +import { fromJson } from "util/common"; /** * A modal that allows the user to restore autosaved values. @@ -16,16 +17,23 @@ class AutosaveModal extends AutosaveBase { e.preventDefault(); e.stopPropagation(); + // Hide all the buttons (we don't want any interaction to close the modal or the restore process fails) + $modal.find(".modal-footer").find("button").hide(); + // This need awaiting or it returns before the value is fully set meaning if the recovery is "fast" it will not clear await this.storage.setItem('recovering', true); // Count the curvals so we don't return too early - let curvalCount = $form.find('.linkspace-field[data-column-type="curval"]').length; + let curvalCount = 0; + // Only count changed curvals - as each in the array has it's own event, we count the number of changes, not the number of fields + await Promise.all($form.find('.linkspace-field[data-column-type="curval"]').map(async (_, field) => { + await this.storage.getItem(this.columnKey($(field))) && (curvalCount += fromJson(await this.storage.getItem(this.columnKey($(field)))).length); + })); let errored = false; let $list = $(""); const $body = $modal.find(".modal-body"); - $body.html("

Restoring values...

").append($list); + $body.html("

Restoring values...

Please be aware that linked records may take a moment to finish restoring.

").append($list); // Convert the fields to promise functions (using the fields) that are run in parallel // This is only done because various parts of the codebase use the fields in different ways dependent on types (i.e. curval) Promise.all($form.find('.linkspace-field').map(async (_, field) => { @@ -53,7 +61,11 @@ class AutosaveModal extends AutosaveBase { const $li = $(`

  • Error restoring ${name}, please check these values before submission
    • ${e.message}
    • `); $list.append($li); // If we've done all fields, turn off the recovery flag - if (!curvalCount) this.storage.removeItem('recovering'); + if (!curvalCount) { + // Hide the restore button and show the close button + $modal.find(".modal-footer").find(".btn-cancel").text("Close").show(); + this.storage.removeItem('recovering'); + } }); $field.on("validationPassed", () => { // Decrement the curval count @@ -61,7 +73,11 @@ class AutosaveModal extends AutosaveBase { const $li = $(`
  • Restored ${name}
    • `); $list.append($li); // If we've done all fields, turn off the recovery flag - if (!curvalCount) this.storage.removeItem('recovering'); + if (!curvalCount) { + // Hide the restore button and show the close button + $modal.find(".modal-footer").find(".btn-cancel").text("Close").show(); + this.storage.removeItem('recovering'); + } }); } setFieldValues($field, values); @@ -86,11 +102,12 @@ class AutosaveModal extends AutosaveBase { // If there are any errors that can't be handled in the mapped promises, show a critical error message $body.append(`

    Critical error restoring values

    ${e}

    `); }).finally(() => { - // Hide the restore button and show the close button - $modal.find(".modal-footer").find("button:not(.btn-cancel)").hide(); - $modal.find(".modal-footer").find(".btn-cancel").text("Close"); - // If we've done all fields, turn off the recovery flag - if (!curvalCount) this.storage.removeItem('recovering'); + // Only allow to close once recovery is finished + if(!curvalCount || errored) { + // Show the close button + $modal.find(".modal-footer").find(".btn-cancel").text("Close").show(); + this.storage.removeItem('recovering'); + } }); }); @@ -102,7 +119,6 @@ class AutosaveModal extends AutosaveBase { $modal.find('.btn-js-delete-values').attr('disabled', 'disabled').hide(); } } - } export default AutosaveModal; diff --git a/src/frontend/components/modal/modals/curval/lib/component.js b/src/frontend/components/modal/modals/curval/lib/component.js index 50d4e18db..3e29fc7b3 100644 --- a/src/frontend/components/modal/modals/curval/lib/component.js +++ b/src/frontend/components/modal/modals/curval/lib/component.js @@ -5,6 +5,7 @@ import { guid as Guid } from "guid" import { initializeRegisteredComponents } from 'component' import { validateRadioGroup, validateCheckboxGroup } from 'validation' import gadsStorage from 'util/gadsStorage' +import { fromJson } from 'util/common' class CurvalModalComponent extends ModalComponent { @@ -109,7 +110,7 @@ class CurvalModalComponent extends ModalComponent { const editButton = $( ` `, @@ -216,7 +217,7 @@ class CurvalModalComponent extends ModalComponent { const id = location.pathname.split("/").pop() const record_id = isNaN(parseInt(id)) ? 0 : parseInt(id) const parent_key = `linkspace-column-${col_id}-${$('body').data('layout-identifier')}-${record_id}`; - let existing = await gadsStorage.getItem(parent_key) ? (JSON.parse(await gadsStorage.getItem(parent_key))) : [] + let existing = fromJson(await gadsStorage.getItem(parent_key) ?? "[]") const identifier = current_id || guid // "existing" is the existing values for this curval // Pull out the current record if it exists