From 9e843b07ab2af06dfbe6c904157f7d46179a1d99 Mon Sep 17 00:00:00 2001 From: Dave Roberts <d.roberts@ctrlo.com> Date: Fri, 31 Jan 2025 16:05:38 +0000 Subject: [PATCH 1/6] Added fix for timeout on autorecover - Added a record identifier to the session - Removed clear from the save button code (we want the data cleared on successful submission) - Created a new Alert component that checks for the data attribute that contains the record ID, and clears the data using that if it's present - Added data attribute to the message_center view --- lib/GADS.pm | 5 ++ src/frontend/components/alert/index.ts | 7 ++ .../components/alert/lib/component.ts | 68 +++++++++++++++++++ .../button/lib/submit-draft-record-button.ts | 4 -- .../button/lib/submit-record-button.ts | 3 - src/frontend/js/site.js | 2 + views/snippets/message_center.tt | 2 +- 7 files changed, 83 insertions(+), 8 deletions(-) create mode 100644 src/frontend/components/alert/index.ts create mode 100644 src/frontend/components/alert/lib/component.ts diff --git a/lib/GADS.pm b/lib/GADS.pm index 0d0db0750..08724648a 100644 --- a/lib/GADS.pm +++ b/lib/GADS.pm @@ -2299,6 +2299,8 @@ prefix '/:layout_name' => sub { any ['get', 'post'] => '/data' => require_login sub { my $layout = var('layout') or pass; + my $record_id = session('record_id') if session('record_id'); + session->delete('record_id') if session('record_id'); my $user = logged_in_user; @@ -2815,6 +2817,7 @@ prefix '/:layout_name' => sub { Crumb($base_url."table/", "Tables"), Crumb("", "Table: " . $layout->name) ]; + $params->{record_id} = $record_id if defined $record_id; template 'data' => $params; }; @@ -4801,6 +4804,8 @@ sub _process_edit my $forward = (!$id && $layout->forward_record_after_create) || param('submit') eq 'submit-and-remain' ? 'record/'.$record->current_id : $layout->identifier.'/data'; + session 'record_id' => ($id ? $id : 0); + return forwardHome( { success => 'Submission has been completed successfully for record ID '.$record->current_id }, $forward ); } diff --git a/src/frontend/components/alert/index.ts b/src/frontend/components/alert/index.ts new file mode 100644 index 000000000..e77c07ed2 --- /dev/null +++ b/src/frontend/components/alert/index.ts @@ -0,0 +1,7 @@ +import { initializeComponent } from "component" +import AlertComponent from "./lib/component" + +export default (scope) =>{ + // @ts-expect-error The components are not typesafe + initializeComponent(scope, '.alert', AlertComponent) +} \ No newline at end of file diff --git a/src/frontend/components/alert/lib/component.ts b/src/frontend/components/alert/lib/component.ts new file mode 100644 index 000000000..d67252d69 --- /dev/null +++ b/src/frontend/components/alert/lib/component.ts @@ -0,0 +1,68 @@ +import { Component } from "component"; +import gadsStorage from "util/gadsStorage"; + +class AlertComponent extends Component { + $el: JQuery<HTMLElement>; + + constructor(element: HTMLElement) { + super(element); + this.$el = $(element); + if(this.recordId === undefined) return; + this.clearData(); + } + + /** + * Clear the data stored in the local storage for the record once it has been saved + */ + async clearData() { + const layout = this.layoutId; + const record = this.recordId; + const ls = this.storage; + const item = await ls.getItem(this.table_key); + + if (item) { + ls.removeItem(this.table_key); + const len = ls.length; + for (let i = 0; i < len; i++) { + const key = ls.key(i); + if (key && key.startsWith(`linkspace-column`) && key.endsWith(`${layout}-${record}`)) { + ls.removeItem(key); + } + } + } + } + + /** + * Get the layout identifier from the body data + * @returns The layout identifier + */ + get layoutId() { + return $('body').data('layout-identifier'); + } + + /** + * Get the record identifier from the body data + * @returns The record identifier + */ + get recordId() { + return this.$el.data('record-id'); + } + + /** + * Get the key for the table used for saving form values + * @returns The key for the table + */ + get table_key() { + return `linkspace-record-change-${this.layoutId}-${this.recordId}`; + } + + /** + * Get the storage object - this originally was used in debugging to allow for the storage object to be mocked + * @returns The storage object + */ + get storage() { + return gadsStorage; + } +} + +export default AlertComponent; \ No newline at end of file diff --git a/src/frontend/components/button/lib/submit-draft-record-button.ts b/src/frontend/components/button/lib/submit-draft-record-button.ts index 8c601ae74..204c84e01 100644 --- a/src/frontend/components/button/lib/submit-draft-record-button.ts +++ b/src/frontend/components/button/lib/submit-draft-record-button.ts @@ -1,5 +1,3 @@ -import { clearSavedFormValues } from "./common"; - /** * Create a submit draft record button * @param element {JQuery<HTMLElement>} The button element @@ -11,7 +9,5 @@ export default function createSubmitDraftRecordButton(element: JQuery<HTMLElemen // Remove the required attribute from hidden required dependent fields $form.find(".form-group *[aria-required]").removeAttr('required'); - // As the draft should save all changed values, we clear them from the local storage - await clearSavedFormValues(ev.target.closest("form")); }); } diff --git a/src/frontend/components/button/lib/submit-record-button.ts b/src/frontend/components/button/lib/submit-record-button.ts index 2977a7416..a857fa9c2 100644 --- a/src/frontend/components/button/lib/submit-record-button.ts +++ b/src/frontend/components/button/lib/submit-record-button.ts @@ -1,5 +1,4 @@ import {validateRequiredFields} from "validation"; -import { clearSavedFormValues } from "./common"; /** * Button to submit records @@ -47,8 +46,6 @@ export default class SubmitRecordButton { if ($button.prop("name")) { $button.after(`<input type="hidden" name="${$button.prop("name")}" value="${$button.val()}" />`); } - // Clear the saved form values from local storage as they should now be saved to the record - await clearSavedFormValues($form); } else { // Re-add the required attribute to required dependent fields $requiredHiddenRecordDependentFields.attr('required', ''); diff --git a/src/frontend/js/site.js b/src/frontend/js/site.js index 73dbc0ecb..ac1b88563 100644 --- a/src/frontend/js/site.js +++ b/src/frontend/js/site.js @@ -5,6 +5,7 @@ import 'components/graph/lib/chart'; import 'util/filedrag'; // Components +import AlertComponent from 'components/alert'; import AddTableModalComponent from 'components/modal/modals/new-table'; import AutosaveComponent from 'components/form-group/autosave'; import CalcFieldsComponent from 'components/form-group/calc-fields'; @@ -44,6 +45,7 @@ import HelpView from "components/help-view"; import PeopleFilterComponent from "components/form-group/people-filter"; // Register them +registerComponent(AlertComponent); registerComponent(AddTableModalComponent); registerComponent(ButtonComponent); registerComponent(CalcFieldsComponent); diff --git a/views/snippets/message_center.tt b/views/snippets/message_center.tt index cfd0843e6..687bc28cb 100644 --- a/views/snippets/message_center.tt +++ b/views/snippets/message_center.tt @@ -1,6 +1,6 @@ [% IF messages.size %] [% FOR message IN messages %] - <div class="alert alert-[% message.bootstrap_color %]" role="alert"> + <div class="alert alert-[% message.bootstrap_color %]" role="alert"[% IF record_id.defined %] data-record-id=[% record_id %][% END %]> [% message.reason %]: [% IF message.inClass("html") %][% message.toString %][% ELSE %][% message.toString | html_entity %][% END %] </div> From f01dcbb0786e62807cb68e6866f9111b56c0c57b Mon Sep 17 00:00:00 2001 From: Dave Roberts <d.roberts@ctrlo.com> Date: Tue, 4 Feb 2025 14:27:26 +0000 Subject: [PATCH 2/6] Various changes - still WIP - Removed the `alert` component - Moved the session variable for the updated record into a new variable to - promote extensibility - enable other actions to be added as required for other purposes in future - Promote extensible handlers - Added a JS handler implementation --- lib/GADS.pm | 12 ++-- src/frontend/components/alert/index.ts | 7 -- .../components/alert/lib/component.ts | 68 ------------------- .../js/lib/util/actionsHandler/index.ts | 4 ++ .../util/actionsHandler/lib/handler.test.ts | 11 +++ .../js/lib/util/actionsHandler/lib/handler.ts | 25 +++++++ .../actionsHandler/lib/updateRecordAction.ts | 43 ++++++++++++ src/frontend/js/site.js | 9 ++- views/layouts/main.tt | 2 +- 9 files changed, 98 insertions(+), 83 deletions(-) delete mode 100644 src/frontend/components/alert/index.ts delete mode 100644 src/frontend/components/alert/lib/component.ts create mode 100644 src/frontend/js/lib/util/actionsHandler/index.ts create mode 100644 src/frontend/js/lib/util/actionsHandler/lib/handler.test.ts create mode 100644 src/frontend/js/lib/util/actionsHandler/lib/handler.ts create mode 100644 src/frontend/js/lib/util/actionsHandler/lib/updateRecordAction.ts diff --git a/lib/GADS.pm b/lib/GADS.pm index 08724648a..022f4b638 100644 --- a/lib/GADS.pm +++ b/lib/GADS.pm @@ -340,6 +340,9 @@ hook before_template => sub { # Base 64 encoder for use in templates $tokens->{b64_filter} = sub { encode_base64(encode_json shift, '') }; + $tokens->{actions} = session 'actions'; + session->delete('actions'); + # This line used to be pre-request. However, occasionally errors have been # experienced with pages not submitting CSRF tokens. I think these may have # been race conditions where the session had been destroyed between the @@ -2299,9 +2302,7 @@ prefix '/:layout_name' => sub { any ['get', 'post'] => '/data' => require_login sub { my $layout = var('layout') or pass; - my $record_id = session('record_id') if session('record_id'); - session->delete('record_id') if session('record_id'); - + my $user = logged_in_user; my @additional_filters; @@ -2817,7 +2818,6 @@ prefix '/:layout_name' => sub { Crumb($base_url."table/", "Tables"), Crumb("", "Table: " . $layout->name) ]; - $params->{record_id} = $record_id if defined $record_id; template 'data' => $params; }; @@ -4678,6 +4678,7 @@ sub _process_edit ); $params{layout} = var('layout') if var('layout'); # Used when creating a new record + my $actions; my $layout; if (my $delete_id = param 'delete') @@ -4804,7 +4805,8 @@ sub _process_edit my $forward = (!$id && $layout->forward_record_after_create) || param('submit') eq 'submit-and-remain' ? 'record/'.$record->current_id : $layout->identifier.'/data'; - session 'record_id' => ($id ? $id : 0); + $actions->{update_record} = $id ? $id: 0; + session 'actions' => $actions; return forwardHome( { success => 'Submission has been completed successfully for record ID '.$record->current_id }, $forward ); diff --git a/src/frontend/components/alert/index.ts b/src/frontend/components/alert/index.ts deleted file mode 100644 index e77c07ed2..000000000 --- a/src/frontend/components/alert/index.ts +++ /dev/null @@ -1,7 +0,0 @@ -import { initializeComponent } from "component" -import AlertComponent from "./lib/component" - -export default (scope) =>{ - // @ts-expect-error The components are not typesafe - initializeComponent(scope, '.alert', AlertComponent) -} \ No newline at end of file diff --git a/src/frontend/components/alert/lib/component.ts b/src/frontend/components/alert/lib/component.ts deleted file mode 100644 index d67252d69..000000000 --- a/src/frontend/components/alert/lib/component.ts +++ /dev/null @@ -1,68 +0,0 @@ -import { Component } from "component"; -import gadsStorage from "util/gadsStorage"; - -class AlertComponent extends Component { - $el: JQuery<HTMLElement>; - - constructor(element: HTMLElement) { - super(element); - this.$el = $(element); - if(this.recordId === undefined) return; - this.clearData(); - } - - /** - * Clear the data stored in the local storage for the record once it has been saved - */ - async clearData() { - const layout = this.layoutId; - const record = this.recordId; - const ls = this.storage; - const item = await ls.getItem(this.table_key); - - if (item) { - ls.removeItem(this.table_key); - const len = ls.length; - for (let i = 0; i < len; i++) { - const key = ls.key(i); - if (key && key.startsWith(`linkspace-column`) && key.endsWith(`${layout}-${record}`)) { - ls.removeItem(key); - } - } - } - } - - /** - * Get the layout identifier from the body data - * @returns The layout identifier - */ - get layoutId() { - return $('body').data('layout-identifier'); - } - - /** - * Get the record identifier from the body data - * @returns The record identifier - */ - get recordId() { - return this.$el.data('record-id'); - } - - /** - * Get the key for the table used for saving form values - * @returns The key for the table - */ - get table_key() { - return `linkspace-record-change-${this.layoutId}-${this.recordId}`; - } - - /** - * Get the storage object - this originally was used in debugging to allow for the storage object to be mocked - * @returns The storage object - */ - get storage() { - return gadsStorage; - } -} - -export default AlertComponent; \ No newline at end of file diff --git a/src/frontend/js/lib/util/actionsHandler/index.ts b/src/frontend/js/lib/util/actionsHandler/index.ts new file mode 100644 index 000000000..896bcec64 --- /dev/null +++ b/src/frontend/js/lib/util/actionsHandler/index.ts @@ -0,0 +1,4 @@ +import './lib/updateRecordAction'; +import { handleActions } from "./lib/handler"; + +export default handleActions; diff --git a/src/frontend/js/lib/util/actionsHandler/lib/handler.test.ts b/src/frontend/js/lib/util/actionsHandler/lib/handler.test.ts new file mode 100644 index 000000000..b5aee7132 --- /dev/null +++ b/src/frontend/js/lib/util/actionsHandler/lib/handler.test.ts @@ -0,0 +1,11 @@ +import {describe, it, expect, jest} from '@jest/globals'; +import { addAction, handleActions } from './handler'; + +describe('addAction', () => { + it('should add an action to the list of actions', () => { + const action = jest.fn(); + addAction(action); + expect(()=>handleActions()).not.toThrow(); + expect(action).toHaveBeenCalled(); + }); +}); diff --git a/src/frontend/js/lib/util/actionsHandler/lib/handler.ts b/src/frontend/js/lib/util/actionsHandler/lib/handler.ts new file mode 100644 index 000000000..d37686a9a --- /dev/null +++ b/src/frontend/js/lib/util/actionsHandler/lib/handler.ts @@ -0,0 +1,25 @@ +/** + * This is the main file of the actions handler. These will trigger any actions as they are created in the application. + */ + +/** + * Array of actions to be handled + */ +const actions: (()=>void)[] = []; + +/** + * Add an action to the handler + * @param action {()=>void} Action to add to the handler + */ +const addAction = (action: () => void) => { + actions.push(action); +}; + +/** + * Handle all actions + */ +const handleActions: () => void = () => { + actions.forEach(action => action?.()); +}; + +export { addAction, handleActions }; \ No newline at end of file diff --git a/src/frontend/js/lib/util/actionsHandler/lib/updateRecordAction.ts b/src/frontend/js/lib/util/actionsHandler/lib/updateRecordAction.ts new file mode 100644 index 000000000..6a6ab94a9 --- /dev/null +++ b/src/frontend/js/lib/util/actionsHandler/lib/updateRecordAction.ts @@ -0,0 +1,43 @@ +import gadsStorage from "util/gadsStorage"; +import { addAction } from "./handler"; + +/** + * Action handler for when a record is created or updated + */ + +addAction(async () => { + // Create a reference to the body element as a jQuery object (saves multiple calls to the jQuery function) + const $body = $('body'); + // Get the layout identifier from the data attribute + const layoutId = $body.data('layout-identifier'); + // If the layout identifier is undefined, return + if (typeof layoutId == 'undefined') return; + // Get the record identifier from the data attribute + const recordId = $body.data('update-record') + // If the record identifier is undefined, return + if (typeof recordId == 'undefined') return; + // Create a key to identify the record change in the storage + const tableKey = `linkspace-record-change-${layoutId}-${recordId}`; + // Get the storage object + const storage = gadsStorage; + // Get the item from the storage + const item = await storage.getItem(tableKey); + + // If the item exists + if (item) { + // Remove the item from storage + storage.removeItem(tableKey); + // Get the length of the storage + const len = storage.length; + // Iterate over the storage + for (let i = 0; i < len; i++) { + // Get the key from the storage + const key = storage.key(i); + // If the key does not start with 'linkspace-column' and does not end with the layout and record identifier, + // skip ahead, this reduces the cyclomatic complexity + if (!(key && key.startsWith('linkspace-column') && key.endsWith(`-${layoutId}-${recordId}`))) continue; + // Remove the item from the storage + storage.removeItem(key); + } + } +}); diff --git a/src/frontend/js/site.js b/src/frontend/js/site.js index ac1b88563..da498f009 100644 --- a/src/frontend/js/site.js +++ b/src/frontend/js/site.js @@ -3,9 +3,9 @@ import { initializeRegisteredComponents, registerComponent } from 'component'; import 'bootstrap'; import 'components/graph/lib/chart'; import 'util/filedrag'; +import 'util/actionsHandler'; // Components -import AlertComponent from 'components/alert'; import AddTableModalComponent from 'components/modal/modals/new-table'; import AutosaveComponent from 'components/form-group/autosave'; import CalcFieldsComponent from 'components/form-group/calc-fields'; @@ -43,9 +43,9 @@ import ButtonComponent from "components/button"; import SelectAllComponent from "components/select-all"; import HelpView from "components/help-view"; import PeopleFilterComponent from "components/form-group/people-filter"; +import handleActions from "util/actionsHandler"; // Register them -registerComponent(AlertComponent); registerComponent(AddTableModalComponent); registerComponent(ButtonComponent); registerComponent(CalcFieldsComponent); @@ -86,3 +86,8 @@ registerComponent(AutosaveComponent); // Initialize all components at some point initializeRegisteredComponents(document.body); + +// This is wrapped like this just to ensure that the actions are loaded before they are triggered, it's (almost) shorthand for $(document).ready() +(() => { + handleActions(); +})(); \ No newline at end of file diff --git a/views/layouts/main.tt b/views/layouts/main.tt index f4af8a0d0..cc31c1a80 100755 --- a/views/layouts/main.tt +++ b/views/layouts/main.tt @@ -9,7 +9,7 @@ <link rel="stylesheet" href="[% url.css %]/external.css?v=7"> <link rel="stylesheet" href="[% url.css %]/general.css?v=8"> </head> - <body class="[% body_class || 'page' %]"[% IF csrf_token %] data-csrf="[%- csrf_token -%]"[% END; IF layout OR layout_obj; %] data-layout-identifier="[% layout.identifier || layout_obj.identifier %]"[% END %]> + <body class="[% body_class || 'page' %]"[% IF csrf_token %] data-csrf="[%- csrf_token -%]"[% END; IF layout OR layout_obj; %] data-layout-identifier="[% layout.identifier || layout_obj.identifier %]"[% END %][% IF actions; %][% IF actions.update_record.defined %] data-update-record="[% actions.update_record %]"[% END %][% END; %]> <div class="[% container_class || 'container-fluid' %]"> [% IF page == "login" OR page == "register" OR page == "reset" OR page == "invalidsite"; From ad35d3c9daf55c8490f152207f79682d41519f7a Mon Sep 17 00:00:00 2001 From: Dave Roberts <d.roberts@ctrlo.com> Date: Wed, 5 Feb 2025 10:36:32 +0000 Subject: [PATCH 3/6] Implemented new storage provider --- .../components/button/lib/cancel-button.ts | 2 +- src/frontend/components/button/lib/common.ts | 16 ++--- .../form-group/autosave/lib/autosaveBase.ts | 4 +- .../modal/modals/curval/lib/component.js | 7 +- .../actionsHandler/lib/updateRecordAction.ts | 27 ++------ .../js/lib/util/gadsStorage/lib/AppStorage.ts | 2 +- .../js/lib/util/storageProvider/index.ts | 3 + .../lib/storageProvider.test.ts | 68 +++++++++++++++++++ .../storageProvider/lib/storageProvider.ts | 47 +++++++++++++ 9 files changed, 134 insertions(+), 42 deletions(-) create mode 100644 src/frontend/js/lib/util/storageProvider/index.ts create mode 100644 src/frontend/js/lib/util/storageProvider/lib/storageProvider.test.ts create mode 100644 src/frontend/js/lib/util/storageProvider/lib/storageProvider.ts diff --git a/src/frontend/components/button/lib/cancel-button.ts b/src/frontend/components/button/lib/cancel-button.ts index 0bbaf24d8..a81518d80 100644 --- a/src/frontend/components/button/lib/cancel-button.ts +++ b/src/frontend/components/button/lib/cancel-button.ts @@ -6,7 +6,7 @@ export default function createCancelButton(el: HTMLElement | JQuery<HTMLElement> $el.data('cancel-button', "true"); $el.on('click', async () => { const href = $el.data('href'); - await clearSavedFormValues($el.closest('form')); + await clearSavedFormValues(); if (href) window.location.href = href; else diff --git a/src/frontend/components/button/lib/common.ts b/src/frontend/components/button/lib/common.ts index 2c2909e80..7654c2103 100644 --- a/src/frontend/components/button/lib/common.ts +++ b/src/frontend/components/button/lib/common.ts @@ -1,22 +1,14 @@ -import gadsStorage from "util/gadsStorage"; +import StorageProvider from "util/storageProvider"; /** * Clear all saved form values for the current record * @param $form The form to clear the data for */ -export async function clearSavedFormValues($form: JQuery<HTMLElement>) { - if (!$form || $form.length === 0) return; - const layout = layoutId(); - const record = recordId(); +export async function clearSavedFormValues() { const ls = storage(); const item = await ls.getItem(table_key()); - if (item) ls.removeItem(`linkspace-record-change-${layout}-${record}`); - await Promise.all($form.find(".linkspace-field").map(async (_, el) => { - const field_id = $(el).data("column-id"); - const item = await gadsStorage.getItem(`linkspace-column-${field_id}-${layout}-${record}`); - if (item) gadsStorage.removeItem(`linkspace-column-${field_id}-${layout}-${record}`); - })); + if (item) ls.clear(); } /** @@ -48,5 +40,5 @@ export function table_key() { * @returns The storage object */ export function storage() { - return gadsStorage; + return new StorageProvider(table_key()); } \ No newline at end of file diff --git a/src/frontend/components/form-group/autosave/lib/autosaveBase.ts b/src/frontend/components/form-group/autosave/lib/autosaveBase.ts index f4127e876..912947632 100644 --- a/src/frontend/components/form-group/autosave/lib/autosaveBase.ts +++ b/src/frontend/components/form-group/autosave/lib/autosaveBase.ts @@ -1,5 +1,5 @@ import { Component } from "component"; -import gadsStorage from "util/gadsStorage"; +import StorageProvider from "util/storageProvider"; /** * Base class for autosave @@ -39,7 +39,7 @@ export default abstract class AutosaveBase extends Component { * The storage object to use for autosave - this is a variable to allow for mocking in testing */ get storage() { - return gadsStorage; + return new StorageProvider(`linkspace-record-change-${this.layoutId}-${this.recordId}`); } /** diff --git a/src/frontend/components/modal/modals/curval/lib/component.js b/src/frontend/components/modal/modals/curval/lib/component.js index 3e29fc7b3..5e0cfbba6 100644 --- a/src/frontend/components/modal/modals/curval/lib/component.js +++ b/src/frontend/components/modal/modals/curval/lib/component.js @@ -4,8 +4,8 @@ import { setFieldValues } from "set-field-values" 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' +import StorageProvider from 'util/storageProvider' class CurvalModalComponent extends ModalComponent { @@ -217,7 +217,8 @@ 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 = fromJson(await gadsStorage.getItem(parent_key) ?? "[]") + const storageProvider = new StorageProvider(`linkspace-record-change-${$('body').data('layout-identifier')}-${record_id}`) + let existing = fromJson(await storageProvider.getItem(parent_key) ?? "[]") const identifier = current_id || guid // "existing" is the existing values for this curval // Pull out the current record if it exists @@ -235,7 +236,7 @@ class CurvalModalComponent extends ModalComponent { } existing.push(existing_row) // Store as array for consistency with other field types - await gadsStorage.setItem(parent_key, JSON.stringify(existing)) + await storageProvider.setItem(parent_key, JSON.stringify(existing)) } $(this.element).modal('hide') diff --git a/src/frontend/js/lib/util/actionsHandler/lib/updateRecordAction.ts b/src/frontend/js/lib/util/actionsHandler/lib/updateRecordAction.ts index 6a6ab94a9..9bcdb7a74 100644 --- a/src/frontend/js/lib/util/actionsHandler/lib/updateRecordAction.ts +++ b/src/frontend/js/lib/util/actionsHandler/lib/updateRecordAction.ts @@ -1,10 +1,9 @@ -import gadsStorage from "util/gadsStorage"; +import StorageProvider from "util/storageProvider"; import { addAction } from "./handler"; /** * Action handler for when a record is created or updated */ - addAction(async () => { // Create a reference to the body element as a jQuery object (saves multiple calls to the jQuery function) const $body = $('body'); @@ -19,25 +18,7 @@ addAction(async () => { // Create a key to identify the record change in the storage const tableKey = `linkspace-record-change-${layoutId}-${recordId}`; // Get the storage object - const storage = gadsStorage; - // Get the item from the storage - const item = await storage.getItem(tableKey); - - // If the item exists - if (item) { - // Remove the item from storage - storage.removeItem(tableKey); - // Get the length of the storage - const len = storage.length; - // Iterate over the storage - for (let i = 0; i < len; i++) { - // Get the key from the storage - const key = storage.key(i); - // If the key does not start with 'linkspace-column' and does not end with the layout and record identifier, - // skip ahead, this reduces the cyclomatic complexity - if (!(key && key.startsWith('linkspace-column') && key.endsWith(`-${layoutId}-${recordId}`))) continue; - // Remove the item from the storage - storage.removeItem(key); - } - } + const storage = new StorageProvider(tableKey); + // Clear the storage + await storage.clear(); }); diff --git a/src/frontend/js/lib/util/gadsStorage/lib/AppStorage.ts b/src/frontend/js/lib/util/gadsStorage/lib/AppStorage.ts index ce2bce537..8bcde26d9 100644 --- a/src/frontend/js/lib/util/gadsStorage/lib/AppStorage.ts +++ b/src/frontend/js/lib/util/gadsStorage/lib/AppStorage.ts @@ -6,7 +6,7 @@ import { NullStorage } from "./NullStorage"; */ export abstract class AppStorage { static CreateStorageInstance(): AppStorage { - return crypto.subtle ? new GadsStorage(): new NullStorage(); + return crypto.subtle && typeof crypto.subtle != "undefined" ? new GadsStorage(): new NullStorage(); } /** * Store a value in the browsers' storage diff --git a/src/frontend/js/lib/util/storageProvider/index.ts b/src/frontend/js/lib/util/storageProvider/index.ts new file mode 100644 index 000000000..d410627a3 --- /dev/null +++ b/src/frontend/js/lib/util/storageProvider/index.ts @@ -0,0 +1,3 @@ +import StorageProvider from './lib/storageProvider'; + +export default StorageProvider; \ No newline at end of file diff --git a/src/frontend/js/lib/util/storageProvider/lib/storageProvider.test.ts b/src/frontend/js/lib/util/storageProvider/lib/storageProvider.test.ts new file mode 100644 index 000000000..e8e98eaf1 --- /dev/null +++ b/src/frontend/js/lib/util/storageProvider/lib/storageProvider.test.ts @@ -0,0 +1,68 @@ +import { describe, it, expect, beforeEach } from '@jest/globals'; +import StorageProvider from './storageProvider'; + +describe('StorageProvider', () => { + beforeEach(() => { + localStorage.clear(); + }); + + it('should add a key-value pair', async () => { + const storage = new StorageProvider('test', localStorage); + await expect(storage.setItem('key', 'value')).resolves.toBeUndefined(); + expect(localStorage.getItem('test')).toBe('{"key":"value"}'); + }); + + it('should get a value by key', async () => { + const storage = new StorageProvider('test', localStorage); + await expect(storage.setItem('key', 'value')).resolves.toBeUndefined(); + await expect(storage.getItem('key')).resolves.toBe('value'); + }); + + it('should return undefined if key does not exist', async () => { + const storage = new StorageProvider('test', localStorage); + await expect(storage.getItem('key')).resolves.toBe(undefined); + }); + + it('should return all key-value pairs', async () => { + const storage = new StorageProvider('test', localStorage); + await expect(storage.setItem('key', 'value')).resolves.toBeUndefined(); + await expect(storage.getAll()).resolves.toEqual({ key: 'value' }); + }); + + it('should return an empty object if no key-value pairs exist', async () => { + const storage = new StorageProvider('test', localStorage); + await expect(storage.getAll()).resolves.toEqual({}); + }); + + it('should get different values for different instances', async () => { + const storage1 = new StorageProvider('test1', localStorage); + const storage2 = new StorageProvider('test2', localStorage); + await expect(storage1.setItem('key', 'value1')).resolves.toBeUndefined(); + await expect(storage2.setItem('key', 'value2')).resolves.toBeUndefined(); + await expect(storage1.getItem('key')).resolves.toBe('value1'); + await expect(storage2.getItem('key')).resolves.toBe('value2'); + }); + + it('should update a key-value pair', async () => { + const storage = new StorageProvider('test', localStorage); + await expect(storage.setItem('key', 'value')).resolves.toBeUndefined(); + await expect(storage.getItem('key')).resolves.toBe('value'); + await expect(storage.setItem('key', 'new value')).resolves.toBeUndefined(); + await expect(storage.getItem('key')).resolves.toBe('new value'); + }); + + it('should remove a key-value pair', async () => { + const storage = new StorageProvider('test', localStorage); + await expect(storage.setItem('key', 'value')).resolves.toBeUndefined(); + await expect(storage.getItem('key')).resolves.toBe('value'); + await expect(storage.removeItem('key')).resolves.toBeUndefined(); + await expect(storage.getItem('key')).resolves.toBe(undefined); + }); + + it('should set multipe key value pairs on the same instance', async () => { + const storage = new StorageProvider('test', localStorage); + await expect(storage.setItem('key1', 'value1')).resolves.toBeUndefined(); + await expect(storage.setItem('key2', 'value2')).resolves.toBeUndefined(); + await expect(storage.getAll()).resolves.toEqual({ key1: 'value1', key2: 'value2' }); + }); +}); diff --git a/src/frontend/js/lib/util/storageProvider/lib/storageProvider.ts b/src/frontend/js/lib/util/storageProvider/lib/storageProvider.ts new file mode 100644 index 000000000..152215377 --- /dev/null +++ b/src/frontend/js/lib/util/storageProvider/lib/storageProvider.ts @@ -0,0 +1,47 @@ +import gadsStorage from "util/gadsStorage"; +import { AppStorage } from "util/gadsStorage/lib/AppStorage"; +import { fromJson } from "util/common" + +type StringMap = { [key: string]: string }; + +class StorageProvider { + get provider() {return this.storage;} + + constructor(private readonly instance: string, private readonly storage:Storage | AppStorage = gadsStorage) { + } + + async setItem(key:string, value: string) { + let item = await this.storage.getItem(this.instance); + if(!item) item = '{}'; + const map: StringMap = fromJson(item); + map[key] = value; + await this.storage.setItem(this.instance, JSON.stringify(map)); + } + + async getItem(key: string): Promise<string| undefined> { + const item = await this.storage.getItem(this.instance); + if(!item) return undefined; + const map: StringMap = fromJson(item); + return map[key] || undefined; + } + + async getAll(): Promise<StringMap> { + const item = await this.storage.getItem(this.instance); + if(!item) return {}; + return fromJson(item); + } + + async clear() { + this.storage.removeItem(this.instance); + } + + async removeItem(key: string) { + const item = await this.storage.getItem(this.instance); + if(!item) return; + const map: StringMap = fromJson(item); + delete map[key]; + await this.storage.setItem(this.instance, JSON.stringify(map)); + } +} + +export default StorageProvider; \ No newline at end of file From bb2a71d875838144ba1510ab9400d176a431eb38 Mon Sep 17 00:00:00 2001 From: Dave Roberts <d.roberts@ctrlo.com> Date: Mon, 24 Feb 2025 12:22:00 +0000 Subject: [PATCH 4/6] Added clear to save draft --- .../components/button/lib/submit-draft-record-button.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/frontend/components/button/lib/submit-draft-record-button.ts b/src/frontend/components/button/lib/submit-draft-record-button.ts index 204c84e01..2b3d67e89 100644 --- a/src/frontend/components/button/lib/submit-draft-record-button.ts +++ b/src/frontend/components/button/lib/submit-draft-record-button.ts @@ -1,3 +1,5 @@ +import { clearSavedFormValues } from "./common"; + /** * Create a submit draft record button * @param element {JQuery<HTMLElement>} The button element @@ -9,5 +11,6 @@ export default function createSubmitDraftRecordButton(element: JQuery<HTMLElemen // Remove the required attribute from hidden required dependent fields $form.find(".form-group *[aria-required]").removeAttr('required'); + clearSavedFormValues(); }); } From 9a41b2135cfa903f80336c501dca0bf8d5703044 Mon Sep 17 00:00:00 2001 From: Dave Roberts <d.roberts@ctrlo.com> Date: Thu, 13 Mar 2025 10:42:40 +0000 Subject: [PATCH 5/6] Updated code as suggested Also added unit tests for clearAutoRecover and renamed file appropriately --- .../js/lib/util/actionsHandler/index.ts | 3 +- .../actionsHandler/lib/actionsLoader.test.ts | 26 +++++++++++++++ .../util/actionsHandler/lib/actionsLoader.ts | 15 +++++++++ .../lib/clearAutorecoverAction.test.ts | 30 +++++++++++++++++ .../lib/clearAutorecoverAction.ts | 33 +++++++++++++++++++ .../actionsHandler/lib/updateRecordAction.ts | 24 -------------- src/frontend/js/site.js | 5 +-- views/layouts/main.tt | 2 +- views/snippets/message_center.tt | 2 +- 9 files changed, 109 insertions(+), 31 deletions(-) create mode 100644 src/frontend/js/lib/util/actionsHandler/lib/actionsLoader.test.ts create mode 100644 src/frontend/js/lib/util/actionsHandler/lib/actionsLoader.ts create mode 100644 src/frontend/js/lib/util/actionsHandler/lib/clearAutorecoverAction.test.ts create mode 100644 src/frontend/js/lib/util/actionsHandler/lib/clearAutorecoverAction.ts delete mode 100644 src/frontend/js/lib/util/actionsHandler/lib/updateRecordAction.ts diff --git a/src/frontend/js/lib/util/actionsHandler/index.ts b/src/frontend/js/lib/util/actionsHandler/index.ts index 896bcec64..e9ea72a01 100644 --- a/src/frontend/js/lib/util/actionsHandler/index.ts +++ b/src/frontend/js/lib/util/actionsHandler/index.ts @@ -1,4 +1,5 @@ -import './lib/updateRecordAction'; +// We load all the actions in this file, nowhere else. This is to preserve encapsulation. +import './lib/clearAutorecoverAction'; import { handleActions } from "./lib/handler"; export default handleActions; diff --git a/src/frontend/js/lib/util/actionsHandler/lib/actionsLoader.test.ts b/src/frontend/js/lib/util/actionsHandler/lib/actionsLoader.test.ts new file mode 100644 index 000000000..44d5be4cf --- /dev/null +++ b/src/frontend/js/lib/util/actionsHandler/lib/actionsLoader.test.ts @@ -0,0 +1,26 @@ +import "../../../../../testing/globals.definitions"; +import {describe, it, expect} from '@jest/globals'; +import loadActions from './actionsLoader'; + +describe('loadActions', () => { + it('should return undefined if actions_b64 is undefined', async () => { + const actions = await loadActions(); + expect(actions).toBe(undefined); + }); + + it.skip('should return undefined if action_json is undefined', async () => { + // Skipped as this is not as easy to test as I'd expect + const $body = $('body'); + $body.data('actions', ''); + const actions = await loadActions(); + expect(actions).toBe(undefined); + }); + + it('should return the actions object', async () => { + const actions_b64 = btoa(JSON.stringify({action: 'test'})); + const $body = $('body'); + $body.data('actions', actions_b64); + const actions = await loadActions(); + expect(actions).toEqual({action: 'test'}); + }); +}); diff --git a/src/frontend/js/lib/util/actionsHandler/lib/actionsLoader.ts b/src/frontend/js/lib/util/actionsHandler/lib/actionsLoader.ts new file mode 100644 index 000000000..474fd8647 --- /dev/null +++ b/src/frontend/js/lib/util/actionsHandler/lib/actionsLoader.ts @@ -0,0 +1,15 @@ +/** + * Load the actions object from the body data (Base64 encoded JSON) + * @returns An object representing the actions or undefined if no actions are defined + */ +const loadActions = async () => { + const $body = $('body'); + const actions_b64 = $body.data('actions'); + if (typeof actions_b64 == 'undefined') return; + const action_json = atob(actions_b64); + const actions = JSON.parse(action_json); + if (typeof actions == 'undefined') return; + return actions; +}; + +export default loadActions; \ No newline at end of file diff --git a/src/frontend/js/lib/util/actionsHandler/lib/clearAutorecoverAction.test.ts b/src/frontend/js/lib/util/actionsHandler/lib/clearAutorecoverAction.test.ts new file mode 100644 index 000000000..2cbe47271 --- /dev/null +++ b/src/frontend/js/lib/util/actionsHandler/lib/clearAutorecoverAction.test.ts @@ -0,0 +1,30 @@ +import "../../../../../testing/globals.definitions"; +import { describe, it, expect } from '@jest/globals'; +import { clearAutorecoverAction } from './clearAutorecoverAction'; +import StorageProvider from "../../storageProvider/lib/storageProvider"; + +describe('clearAutorecoverAction', () => { + it('Should not action if layout-identifier is undefined', async () => { + await expect(clearAutorecoverAction()).resolves.toBe(false); + }); + + it('Should not action if actions object is undefined', async () => { + $('body').data('layout-identifier', 'test'); + await expect(clearAutorecoverAction()).resolves.toBe(false); + }); + + it('Should not action if clear_saved_values is not in actions object', async () => { + $('body').data('layout-identifier', 'test'); + $('body').data('actions', btoa(JSON.stringify({}))); + await expect(clearAutorecoverAction()).resolves.toBe(false); + }); + + it('Should clear storage provider', async () => { + $('body').data('layout-identifier', 'test'); + $('body').data('actions', btoa(JSON.stringify({ clear_saved_values: 1 }))); + const storage = new StorageProvider('linkspace-record-change-test-1'); + await storage.setItem('key', 'value'); + await expect(clearAutorecoverAction()).resolves.toBe(true); + await expect(storage.getItem('key')).resolves.toBe(undefined); + }); +}); diff --git a/src/frontend/js/lib/util/actionsHandler/lib/clearAutorecoverAction.ts b/src/frontend/js/lib/util/actionsHandler/lib/clearAutorecoverAction.ts new file mode 100644 index 000000000..029fa1ec9 --- /dev/null +++ b/src/frontend/js/lib/util/actionsHandler/lib/clearAutorecoverAction.ts @@ -0,0 +1,33 @@ +import StorageProvider from "util/storageProvider"; +import { addAction } from "./handler"; +import loadActions from "./actionsLoader"; + +// Exported to be used in tests - return values are to be used in tests +export const clearAutorecoverAction = async () => { + // Load the body as a jQuery object + const $body = $('body'); + // Get the layout identifier from the body data + const layoutId = $body.data('layout-identifier'); + // If the layout identifier is undefined, return + if (typeof layoutId == 'undefined') return false; + // Load the actions object + const actions = await loadActions(); + // If the actions object is undefined, return + if (typeof actions == 'undefined') return false; + // Get the record identifier from the actions object + const recordId = actions.clear_saved_values; + // If the record identifier is undefined, return + if (typeof recordId == 'undefined') return false; + // Create a table key using the layout identifier and record identifier + const tableKey = `linkspace-record-change-${layoutId}-${recordId}`; + // Create a new storage provider + const storage = new StorageProvider(tableKey); + // Clear the storage provider + await storage.clear(); + return true; +} + +/** + * Action handler for when a record is created or updated - only runs outside of a test environment + */ +if(!window.test) addAction(clearAutorecoverAction); diff --git a/src/frontend/js/lib/util/actionsHandler/lib/updateRecordAction.ts b/src/frontend/js/lib/util/actionsHandler/lib/updateRecordAction.ts deleted file mode 100644 index 9bcdb7a74..000000000 --- a/src/frontend/js/lib/util/actionsHandler/lib/updateRecordAction.ts +++ /dev/null @@ -1,24 +0,0 @@ -import StorageProvider from "util/storageProvider"; -import { addAction } from "./handler"; - -/** - * Action handler for when a record is created or updated - */ -addAction(async () => { - // Create a reference to the body element as a jQuery object (saves multiple calls to the jQuery function) - const $body = $('body'); - // Get the layout identifier from the data attribute - const layoutId = $body.data('layout-identifier'); - // If the layout identifier is undefined, return - if (typeof layoutId == 'undefined') return; - // Get the record identifier from the data attribute - const recordId = $body.data('update-record') - // If the record identifier is undefined, return - if (typeof recordId == 'undefined') return; - // Create a key to identify the record change in the storage - const tableKey = `linkspace-record-change-${layoutId}-${recordId}`; - // Get the storage object - const storage = new StorageProvider(tableKey); - // Clear the storage - await storage.clear(); -}); diff --git a/src/frontend/js/site.js b/src/frontend/js/site.js index da498f009..1517accdf 100644 --- a/src/frontend/js/site.js +++ b/src/frontend/js/site.js @@ -87,7 +87,4 @@ registerComponent(AutosaveComponent); // Initialize all components at some point initializeRegisteredComponents(document.body); -// This is wrapped like this just to ensure that the actions are loaded before they are triggered, it's (almost) shorthand for $(document).ready() -(() => { - handleActions(); -})(); \ No newline at end of file +handleActions(); \ No newline at end of file diff --git a/views/layouts/main.tt b/views/layouts/main.tt index cc31c1a80..e0347ba63 100755 --- a/views/layouts/main.tt +++ b/views/layouts/main.tt @@ -9,7 +9,7 @@ <link rel="stylesheet" href="[% url.css %]/external.css?v=7"> <link rel="stylesheet" href="[% url.css %]/general.css?v=8"> </head> - <body class="[% body_class || 'page' %]"[% IF csrf_token %] data-csrf="[%- csrf_token -%]"[% END; IF layout OR layout_obj; %] data-layout-identifier="[% layout.identifier || layout_obj.identifier %]"[% END %][% IF actions; %][% IF actions.update_record.defined %] data-update-record="[% actions.update_record %]"[% END %][% END; %]> + <body class="[% body_class || 'page' %]"[% IF csrf_token %] data-csrf="[%- csrf_token -%]"[% END; IF layout OR layout_obj; %] data-layout-identifier="[% layout.identifier || layout_obj.identifier %]"[% END %][% IF actions; %] data-actions="[% b64_filter(actions) %]"[% END; %]> <div class="[% container_class || 'container-fluid' %]"> [% IF page == "login" OR page == "register" OR page == "reset" OR page == "invalidsite"; diff --git a/views/snippets/message_center.tt b/views/snippets/message_center.tt index 687bc28cb..cfd0843e6 100644 --- a/views/snippets/message_center.tt +++ b/views/snippets/message_center.tt @@ -1,6 +1,6 @@ [% IF messages.size %] [% FOR message IN messages %] - <div class="alert alert-[% message.bootstrap_color %]" role="alert"[% IF record_id.defined %] data-record-id=[% record_id %][% END %]> + <div class="alert alert-[% message.bootstrap_color %]" role="alert"> [% message.reason %]: [% IF message.inClass("html") %][% message.toString %][% ELSE %][% message.toString | html_entity %][% END %] </div> From 166efc897bb9c8e9b7aea5ad53966a16f85514ba Mon Sep 17 00:00:00 2001 From: Dave Roberts <d.roberts@ctrlo.com> Date: Thu, 13 Mar 2025 12:49:22 +0000 Subject: [PATCH 6/6] Added file missed on previous commit --- lib/GADS.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/GADS.pm b/lib/GADS.pm index 022f4b638..4ab2dc0d0 100644 --- a/lib/GADS.pm +++ b/lib/GADS.pm @@ -4805,7 +4805,7 @@ sub _process_edit my $forward = (!$id && $layout->forward_record_after_create) || param('submit') eq 'submit-and-remain' ? 'record/'.$record->current_id : $layout->identifier.'/data'; - $actions->{update_record} = $id ? $id: 0; + $actions->{clear_saved_values} = $id ? $id: 0; session 'actions' => $actions; return forwardHome(