usdigitalresponse / usdr-gost

USDR-hosted grants management tools
http://grants.usdigitalresponse.org
Apache License 2.0
28 stars 20 forks source link

BUG: 500 status response on when updating agencies with non-unique values #1397

Open TylerHendrickson opened 1 year ago

TylerHendrickson commented 1 year ago

Description: This issue is similar to #1376, but relates to scenarios where users are attempting to update an existing agency rather than create a new one. Although the queries issued to Postgres are different, the same unique constraint violation errors are thrown (depending on whether the request attempted to set the agencies name or code to a non-unique value, the UPDATE query violates the agencies_tenant_id_name_unique or agencies_tenant_id_code_unique unique constraints, respectively). As with #1376, the UI does not provide any indication that a problem occurred, although in this case the modal closes upon form submission, making the error even harder to detect for the end-user.

Steps to reproduce:

  1. Login as a user with permission to create and update agencies (e.g. tenant admin).
  2. Create a new agency for the tenant, providing a valid and unique name (ex. ABC Agency) and code (ex. ABC).
  3. Create another new agency for the tenant, also providing a valid and unique name (ex. DEF Agency) and code (ex. DEF).
  4. Ensure that the browser's developer console is recording requests, and attempt to edit the second agency (ex. DEF Agency) to a name that is not unique to the tenant (ex. ABC Agency). Observe that the modal closes and that no error is conveyed to the user. Also observe that the PUT /api/organizations/:organizationId/agencies/name/:agency request issued upon form has a status code of 500.
  5. Attempt to edit the agency once more, this time by setting its code to a value that is not unique to the tenant (ex. ABC). Again, observe that the modal closes and that no error is conveyed to the user. Also observe that the PUT /api/organizations/:organizationId/agencies/code/:agency request issued upon form has a status code of 500.

Impacts:

Desired behavior:

Implementation details:

TylerHendrickson commented 1 year ago

cc @caitlinwinner for input on how errors should behave in the UI.

caitlinwinner commented 1 year ago

@agn-dsgn

agn-dsgn commented 1 year ago

CC: @caitlinwinner @TylerHendrickson Also recommended for: #1376

Design recommendations My recommendation from the design side is to throw an error immediately upon user input, BEFORE they hit submit.

We should use the current pattern, wherein if any input field in the modal has an error, the "OK" (save) button is disabled.

Whenever possible we should strive to prevent user error as early as possible to prevent extra time and effort on the user's part.

Here's how that would look (Imagine each input field is improperly filled out) Screenshot 2023-05-23 at 2 18 07 PM

Text An agency with that name already exists An agency with that code already exists


In ARPA, we already throw an error post-submission. While not ideal this could be a backup solution for Grants Finder as well. We could use the alert pattern within the modal in this case. Screenshot 2023-05-23 at 2 15 32 PM

agn-dsgn commented 1 year ago

Also note, if we must throw other errors with these inputs, assume we'll throw them first, letting users address each sequentially.

Error order of operations:

caitlinwinner commented 1 year ago

@TylerHendrickson are the above mocks sufficient?