Skip to content
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

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

Merged
merged 6 commits into from
Mar 13, 2025

Conversation

droberts-ctrlo
Copy link
Contributor

  • 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

- 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
@abeverley
Copy link
Contributor

Despite me saying that this should go in the alert, I think that actually on reflection it can be kept separate (I was originally thinking that using the alert was the only way of keeping it persistent across requests). I think that actually you should set a session variable on successful submit (as you are doing), but instead it should be a more generic variable (e.g. actions_for_page). This would then be rendered as data in the body of the page (or similar) and passed using the before_template hook. Then, the Javascript should be updated to look for those particular data items and act accordingly. This could potentially be used for other actions we need the Javascript to do on page load.

As such, I think it should be rendered for all pages, not just the data page.

Finally, having seen the complicated code needed for removing all the required fields, I am wondering if we can change the data structure of the local storage, having a way of grouping all fields together for one instance, so that they can be easily deleted rather than needing to loop through and find each one.

- 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
@droberts-ctrlo droberts-ctrlo changed the title Added fix for timeout on autorecover Added fix for timeout on autorecover [B:1361] Feb 6, 2025
Copy link
Contributor

@abeverley abeverley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Dave, just some minor comments for consideration

Also added unit tests for clearAutoRecover and renamed file appropriately
@abeverley abeverley merged commit fd93355 into ctrlo:dev Mar 13, 2025
4 of 7 checks passed
@droberts-ctrlo droberts-ctrlo deleted the autorecover-timeout-check branch March 14, 2025 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants