The angular acq admin interfaces fail silently when required fields are missing

[Jane Sandberg]

Steps to recreate:

1) Go to Administration > Acquisitions > Claim Types (or similar interface) on 3.2 or later
2) Click New Claim Type
3) Fill in all fields except "Code"
4) Click Save. Note that the modal disappears, the new claim type doesn't appear in the grid, and the user doesn't get any feedback about the problem.

An error does show in the console (open-ils.pcrud.create.acqclt failed! stat=500 msg=INSERT error -- please see the error log for more details), but this is not so helpful for the user.

[Tiffany Little (tslittle)]

It would also be a plus if those required fields were marked, even if just with an asterisk.

[Bill Erickson (berick)]

Indicating that a field is required in the auto-generated interfaces requires that the field be marked as required in the fm_IDL.xml file.

For example (from "Claim Type")

<field reporter:label="Code" name="code" reporter:datatype="text"/>

Should be:

<field reporter:label="Code" name="code" reporter:datatype="text" oils_obj:required="true"/>

This will not only indicate it's required but prevent the user from clicking "Save" when the field has no value.

There are likely quite a few fields in the IDL where this attribute should be added.

====

In the meantime... I will apply a patch to the auto admin interfaces to show a warning toast when item creation fails.

[Bill Erickson (berick)]

I have pushed a commit to add danger toasts for create and update failures in the fieldmapper editor (used by the admin UI's):

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1807461-admin-grid-warn-errors

Note this commit sits atop the commit for bug #1807458 since they touch the same code. Ideally, they would be tested and merged together, since the commit for this branch depends on the commit for bug #1807458.

[Bill Erickson (berick)]

Regarding the required fields, maybe we should open a generic bug for updating the fieldmapper to list and correctly tag required fields for (at minimum) the classes in the ACQ admin interfaces.

[Jane Sandberg (sandbergja)]

Thanks for the patch, Bill. It's not working 100% for me, though. For example:

1) Go to Administration > Acquisitions > Claim Types with this branch loaded
2) Click New Claim Type
3) Fill in all fields except "Org Unit"
4) Click Save. I get the "Claim Type Successfully Created" message, even though it wasn't created. :-(

It seems like this usually happens when I'm missing a required org unit field. When I'm missing a required text field, I did get the danger toast instead.

[Jane Sandberg (sandbergja)]

I split the fieldmapper piece into a separate bug here: https://bugs.launchpad.net/evergreen/+bug/1808005

[Jane Sandberg (sandbergja)]

Another note that I was able to recreate the issue that Erica describes in this comment (the user receiving a false "Currency type successfully updated" notice) using Bill's branch: https://bugs.launchpad.net/evergreen/+bug/1807998/comments/1

[Bill Erickson (berick)]

I've opened a new bug to address the missing org unit value scenario: bug #1808016. This is a PCRUD issue. It's not replying with the correct messages, preventing the UI code from recognizing that an error has occurred.

Also commented on bug #1807998 / bug #1808012 re: can't change currency code.

[Jane Sandberg (sandbergja)]

I signed off on your branch, Bill, since it helps a lot, and I think that all the silent failures it doen't cover are split off into other bugs. I created a signoff branch -- including a signoff for the Edit action commit -- here: user/sandbergja/lp1807461-admin-grid-warn-errors-signedoff

Thanks so much, Bill.

[Bill Erickson (berick)]

Putting this through its paces once more I found that simply dismissing a dialog resulted in displaying an error message toast. Adding a patch now to make it possible for a dialog rejection handler to determine of the dialog was dismissed by the user via the UI or it was dismissed because of an error.

[Bill Erickson (berick)]

Thanks for the sign-off's Jane. I have pushed those plus 2 new commits to a new branch:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1807461-lp1807458-dialog-dismissal-errors

The new commits address a lint issue and add support for avoiding error messages/toasts when a dialog is dismissed via the UI (vs. a back-end error).

Note a few fm-editor lint issues remain, but they are resolved in bug #1811288.

More sign-off's requested.

[Tiffany Little (tslittle)]

I tested this on a BSW sandbox, and it's not working for me. I'm still getting the same behavior as described by Jane in the original report and comment #5. I do see bug #1807458 working, though, so I'm not sure if it's missing another prerequisite bug since it sounds like there are several that touch this one?

[Dan Wells (dbw2)]

Tested on current master, and am seeing the pop-up as described on both Chrome and Firefox.

Pushed to master and rel_3_2 with bug #1807458. Thanks, Bill and Jane!