ARXIVNG-897 added InvalidStack handling to verify_user controller, and tests

[erickpeirson]

This also contributes to ARXIVNG-911 by changing the form request method on verify_user to POST.

The basic idea is that we are catching InvalidStack, and displaying the exception messages on its constituent InvalidEvents at the top of the page. Here's a mockup (not real exceptions):

In addition, we should leverage the event's own validation on the form. Will do this next.

[coveralls]

Pull Request Test Coverage Report for Build 84

• 135 of 148 (91.22%) changed or added relevant lines in 6 files are covered.
• 2 unchanged lines in 1 file lost coverage.
• Overall coverage increased (+64.2%) to 65.642%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
submit/controllers/util.py	11	12	91.67%
submit/routes/ui.py	0	12	0.0%
<!--	Total:	135	148	91.22%	-->

Files with Coverage Reduction	New Missed Lines	%
submit/routes/ui.py	2	0.0%
<!--	Total:	2		-->

Totals
Change from base Build 68:	64.2%
Covered Lines:	235
Relevant Lines:	358

---

💛 - Coveralls

[erickpeirson]

@eawoods Ok, made a few changes, and addressed the authorship controller. In the first two steps, we are now also pulling the current state of the submission and pre-populating the form on GET requests (so that the state is reflected in the form).

[erickpeirson]

Maybe something to think about, can we dump the existing page and allow navigation if the option selected is something other than "Continue"? I think we had talked about saving the data on a page if the form validates, but otherwise allowing prev/exit navigation without saving if it doesn't validate.

Yes, I had similar thoughts. Let's discuss (add to sprint agenda?)

[erickpeirson]

This actually delivers ARXIVNG-911 for all implemented controllers.

[erickpeirson]

I noticed that the authorship page defaults to "I am an author of this paper" -- didn't see how that could be happening in code? Jim's point to not having defaults in order to force the user to think about the choice probably applies here.

This appears to be because a default is being set in the core events. Will address as: https://culibrary.atlassian.net/browse/ARXIVNG-917

[DavidLFielding]

I skimmed over the code and it looks good to me. Only comment I have is on validation of form. Sounds like there is a desire to dump what's there if the form does not validate then user clicks previous or save&exist. I thought we had discussed saving the data fields that validate. If a user enters a bunch of metadata and then clicks 'save&exit' I suspect they might be upset if we detect a validation error and dump all of their hard work (specially if they spent an hour entering individual authors). Maybe at least warn user? Data contains validation errors, fix or we save remaining valid fields? Likely only matters for pages with a significant amount of input data.

[erickpeirson]

That's an excellent point. Let's revisit this when we do the metadata controller, sinxe it will be especially relevant in that context

On Fri, Jun 15, 2018, 1:40 PM DavidLFielding notifications@github.com wrote:

I skimmed over the code and it looks good to me. Only comment I have is on validation of form. Sounds like there is a desire to dump what's there if the form does not validate then user clicks previous or save&exist. I thought we had discussed saving the data fields that validate. If a user enters a bunch of metadata and then clicks 'save&exit' I suspect they might be upset if we detect a validation error and dump all of their hard work (specially if they spent an hour entering individual authors). Maybe at least warn user? Data contains validation errors, fix or we save remaining valid fields? Likely only matters for pages with a significant amount of input data.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/cul-it/arxiv-submission-ui/pull/8#issuecomment-397693085, or mute the thread https://github.com/notifications/unsubscribe-auth/ADSqynVgt9YiC9_V30z99BIoDs4EYgW9ks5t8_GTgaJpZM4UpghZ .