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

Implement auto-recover functionality [B: 1403] #483

Open
wants to merge 18 commits into
base: dev
Choose a base branch
from

Conversation

droberts-ctrlo
Copy link
Contributor

No description provided.

@droberts-ctrlo droberts-ctrlo changed the title Reset and rerun of commit Reset and rerun of commit [B: 1403] Dec 2, 2024
@droberts-ctrlo droberts-ctrlo marked this pull request as draft December 2, 2024 14:53
@droberts-ctrlo droberts-ctrlo marked this pull request as ready for review December 16, 2024 10:36
@abeverley abeverley changed the title Reset and rerun of commit [B: 1403] Implement auto-recover functionality [B: 1403] Dec 19, 2024
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.

Great work Dave, this is looking really good. I've made a few minor comments so far. I'd also like to test it in practice, but I'm getting Uncaught (in promise) TypeError: crypto.subtle is undefined - any ideas?

lib/GADS.pm Outdated Show resolved Hide resolved
# mandatory values to be empty. This is because a field may have
# since been made mandatory, and we don't want this to cause the
# autosave recovery to fail
$options{force_mandatory} = 1 if param 'autosave';
Copy link
Contributor

Choose a reason for hiding this comment

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

My concern about this is that there is then a user way to always override mandatory (if they know what they are doing) - then can just add this parameter to any form submission. We probably need a way of tightening this, or preferably there would be a way of notifying the user that a saved value is now mandatory?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I'm honest, I'm having some trouble with this one. I don't quite know whether it would be best to check the fields are mandatory (and blank) in the JS and provide feedback in the auto-recover modal or to have a better check (maybe using the user_key rather than just checking the param is 1) backend to ensure this is only called in the correct circumstances. Suggestions welcomed!!

src/frontend/components/modal/modals/curval/index.js Outdated Show resolved Hide resolved
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