Works - Failed submission clears entered information

[dlim87]

Descriptive summary

Submitting a work with an invalid field results in the work failing to be created (which is expected), and any fields you entered, or files you upload, are cleared.

Tested on mac - Firefox Quantum 70.0b3

hyrax info

Hyrax Version: 3.0.0.pre.rc1 Hyrax Branch: master Hyrax Revision: 48ed925

Nurax Version
ENV: production
SHA: 40b47b0d71af1b958e016ebe49212c1740683133
Branch: 2019-09-12-10-00
Updated: 12 Sep 2019 15:00:37

Rationale

It is not great to have to completely rebuild a work submission if a submission fails.

Expected behavior

The form data could be cached?

Actual behavior

Form data is lost if the submission doesn't go through

Steps to reproduce the behavior

1. Log in
2. Create a work
3. Set an embargo or lease date to before today
4. Press the save button

Related work

May be somewhat similar to #3414

[no-reply]

@samvera/hyrax-repo-managers @samvera/hyrax-ui-ux-advisors i want to check in about this issue.

it seems clear to me that we should make the suggested improvement, so i've tagged this for the 3.x release series (effectively making it current work).

however, i think this is long standing behavior of the work form and, while unfortunate, isn't a release blocker.

thoughts?

[no-reply]

discussion in slack seems to represent a consensus that this is not a release blocker.

i've marked it priority to reflect its status as a high value item for the 3.x release series.

[Dananji]

Some work was done on the JavaScript side of the things to save the form data when POST request fails, and repopulate the form as it renders with the error alert. This adds a lot to maintaining the code in the future and seems like not the best way to do this. There should be much cleaner way to do this with forms in Ruby side of things. I'm not sure whether this is the same to the this question, but maybe this can be taken as an example of how to do this in server-side.

The current work done on this in the JS side is in the branch fix-issue-3978, which uses localStorage of the browser to retain form data. But the current work does not restore any files uploaded in the Files tab and users/groups added in the Sharing tab which are not previously saved.

[blancoj]

@no-reply how about if we just don't let them pick a date that is invalid. This would be a simple/quick solution.

[no-reply]

@blancoj this issue is broader than the date validation problem (which is represented by #3979).

the issue is that for any invalid form input, data isn't retained. since we do validation outside the form itself, we're always going to have cases where these kinds of problems could arise, right?

note that we closed #3979 as a browser issue, unrelated to hyrax work. i.e.: we already validate input dates at the form layer when the browser supports that using HTML5 features.

[jeremyf]

@no-reply - In my analysis of the situation, I'm thinking that I'd likely want to change the Hyrax::Forms::Workform behavior to use controller.params to set values.

hyrax/app/forms/hyrax/forms/work_form.rb

Line 37 in 118a825

def initialize(model, current_ability, controller)

What I'm thinking is that #new would call a newly defined set_attributes_from_parameters! method. That method would likely intersect controller.params keys with the terms/attributes for the form.

I'm writing this down to think through the approach, and to put a bookmark in it for today's work.

[MariaStarPower]

Bug - When I go to create a new work, I am redirected to the Home page, instead of the work form.

[jeremyf]

@MariaStarPower I'm moving this to the front of my queue.

[jlhardes]

Testing on nurax-dev using embargo date from past as invalid value - title, creator, rights statement, and alternative title (so required fields and non-required fields) are preserved in the form when the save fails.

[rjkati]

Tested on nurax-dev in both Firefox and Chrome. Required fields and non-required fields are preserved, however file is removed. Is this expected? @jlhardes, let me know if this needs a new ticket.

[jlhardes]

@rjkati I think this passes for what this issue was fixing (preserving the descriptive form field info on a failed submit). It is worth it to create a new ticket for the lost file upload, though, so we can see if anything can be done to preserve that as well. Link to this ticket for additional context.

[jeremyf]

Lost file uploads are EXTREMELY expensive to solve. There's a browser security issue (e.g. the browser can't say what file to upload). So there's a "hold this file on the server in a tmp directory and wait for a user to submit something that says use this file."

…

On Wed, Feb 24, 2021 at 12:52 PM Julie Hardesty ***@***.***> wrote: @rjkati <https://github.com/rjkati> I think this passes for what this issue was fixing (preserving the descriptive form field info on a failed submit). It is worth it to create a new ticket for the lost file upload, though, so we can see if anything can be done to preserve that as well. Link to this ticket for additional context. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#3978 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAAQUTLN7NSTQFBRDG3F4DTAU4FBANCNFSM4IWJPZ4Q> .

-- Jeremy Friesen takeonrules.com

[rjkati]

Testing on iOS safari now displays an error page when setting a past embargo: https://nurax-dev.curationexperts.com/concern/generic_works?locale=en

I've been able to reproduce this error on Windows Chrome, so it seems that it's not browser related. If I set a future date for the embargo, the deposit proceeds successfully.