Skip to content

Added fix for timeout on autorecover [B:1361] #513

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Mar 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion lib/GADS.pm
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -2299,7 +2302,7 @@ prefix '/:layout_name' => sub {
any ['get', 'post'] => '/data' => require_login sub {

my $layout = var('layout') or pass;

my $user = logged_in_user;

my @additional_filters;
Expand Down Expand Up @@ -4675,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')
Expand Down Expand Up @@ -4801,6 +4805,9 @@ 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->{clear_saved_values} = $id ? $id: 0;
session 'actions' => $actions;

return forwardHome(
{ success => 'Submission has been completed successfully for record ID '.$record->current_id }, $forward );
}
Expand Down
2 changes: 1 addition & 1 deletion src/frontend/components/button/lib/cancel-button.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 4 additions & 12 deletions src/frontend/components/button/lib/common.ts
Original file line number Diff line number Diff line change
@@ -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();
}

/**
Expand Down Expand Up @@ -48,5 +40,5 @@ export function table_key() {
* @returns The storage object
*/
export function storage() {
return gadsStorage;
return new StorageProvider(table_key());
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +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');
// As the draft should save all changed values, we clear them from the local storage
await clearSavedFormValues(ev.target.closest("form"));
clearSavedFormValues();
});
}
3 changes: 0 additions & 3 deletions src/frontend/components/button/lib/submit-record-button.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {validateRequiredFields} from "validation";
import { clearSavedFormValues } from "./common";

/**
* Button to submit records
Expand Down Expand Up @@ -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', '');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Component } from "component";
import gadsStorage from "util/gadsStorage";
import StorageProvider from "util/storageProvider";

/**
* Base class for autosave
Expand Down Expand Up @@ -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}`);
}

/**
Expand Down
7 changes: 4 additions & 3 deletions src/frontend/components/modal/modals/curval/lib/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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
Expand All @@ -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')
Expand Down
5 changes: 5 additions & 0 deletions src/frontend/js/lib/util/actionsHandler/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// 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;
26 changes: 26 additions & 0 deletions src/frontend/js/lib/util/actionsHandler/lib/actionsLoader.test.ts
Original file line number Diff line number Diff line change
@@ -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'});
});
});
15 changes: 15 additions & 0 deletions src/frontend/js/lib/util/actionsHandler/lib/actionsLoader.ts
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -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);
});
});
Original file line number Diff line number Diff line change
@@ -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);
11 changes: 11 additions & 0 deletions src/frontend/js/lib/util/actionsHandler/lib/handler.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});
});
25 changes: 25 additions & 0 deletions src/frontend/js/lib/util/actionsHandler/lib/handler.ts
Original file line number Diff line number Diff line change
@@ -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 };
2 changes: 1 addition & 1 deletion src/frontend/js/lib/util/gadsStorage/lib/AppStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions src/frontend/js/lib/util/storageProvider/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import StorageProvider from './lib/storageProvider';

export default StorageProvider;
Original file line number Diff line number Diff line change
@@ -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' });
});
});
Loading
Loading