codeforpdx / PASS

PASS project - with HMIS module integration
MIT License
24 stars 23 forks source link

627/resolve failing basic info tests #630

Closed JaeGif closed 1 month ago

JaeGif commented 1 month ago

This PR:

Resolves #627

Fixes 2 broken tests.

Test 1: Clear all returns everything to default state.

The functionality was correct, the test was failing because it was not selecting the correct element to view if the new state is rendered properly. I decided to check that /Decline to answer/ was in the DOM rather than burrowing looking for the value of 9. Changed DOB reset value to empty string.

Test 2: submits a basic info profile update when you click the submit button

This one was harder to test. You can't type any specific date into this MUI component using RTL because it automatically opens the calendar view. What I decided to do was press enter to automatically select the current date, as the current date will always be default highlighted in the component. There's some variation in how Date.now() and the DatePicker element format the current date, so I converted Date.now() to the raw formatted yyyy-mm-dd date, and back into a days object to compare to the DatePicker's value.

leekahung commented 1 month ago

A separate issue, not exactly related to this one, but I was wondering if we would want to make the clear and submit buttons visible only when the user wants to edit the information. This is similar to how we have the edit button in Profile.

JaeGif commented 1 month ago

The changes looks good. Not getting any errors when submitting new values for the form.

One thing I've noticed however is that once the user submits a date for DOB, they could no longer reset it to an empty string/null state. They have to select a different date. Was wondering if we would want to keep that given that the field was supposed to be optional.

Hmmm. I didn't notice this before. Will look into it after work.

JaeGif commented 1 month ago

A separate issue, not exactly related to this one, but I was wondering if we would want to make the clear and submit buttons visible only when the user wants to edit the information. This is similar to how we have the edit button in Profile.

I personally feel that having a clear button is more user friendly for initially filling out a form, while when editing a user would probably want to edit individual fields at a time. I don't see a UI design for it though so I'm willing to try either.

JaeGif commented 1 month ago

The changes looks good. Not getting any errors when submitting new values for the form.

One thing I've noticed however is that once the user submits a date for DOB, they could no longer reset it to an empty string/null state. They have to select a different date. Was wondering if we would want to keep that given that the field was supposed to be optional.

@leekahung , I'm trying to replicate this but on my end everything appears to be fully cleared even after submitting previous information. Can you share the steps you took to replicate the behavior?

leekahung commented 1 month ago

@leekahung , I'm trying to replicate this but on my end everything appears to be fully cleared even after submitting previous information. Can you share the steps you took to replicate the behavior?

Hey @JaeGif, no problem.

For the steps, I simply went to the Basic Info form and first submitted some initial information.

From there I tried submitting the form after clearing it. While clearing the form itself with the button is fine, however, when I want to submit the new information, it was prevented from doing so even though the message says my form was updated with the cleared fields.

When I refresh the page or go to a different page and back, the value for DOB has not changed nor has my Legal Name (though I think it's more related DOB using an invalid input, see clip below). Only when I switched the DOB values to a new date did the values finally change.

From what I see from the errors, it seems to be related to the handler functions for input changes and form submission.

https://github.com/codeforpdx/PASS/assets/14917816/b3afd0fe-ca3f-45a9-91cc-406ee3570be3

Here's what it looks like from the storage standpoint:

https://github.com/codeforpdx/PASS/assets/14917816/33abe03a-c78d-494b-86df-6be0b91b0e31

JaeGif commented 1 month ago

For the steps, I simply went to the Basic Info form and first submitted some initial information.

From there I tried submitting the form after clearing it. While clearing the form itself with the button is fine, however, when I want to submit the new information, it was prevented from doing so even though the message says my form was updated with the cleared fields.

When I refresh the page or go to a different page and back, the value for DOB has not changed nor has my Legal Name (though I think it's more related DOB using an invalid input, see clip below). Only when I switched the DOB values to a new date did the values finally change.

From what I see from the errors, it seems to be related to the handler functions for input changes and form submission.

@leekahung I believe I've fixed the bug. It occurs because I was using '' as the default value of DatePicker. I changed the default value to null, however null is coerced by the DatePicker element to an empty string representation in the DatePicker's value, yet passed on as null to handler functions. The test therefore needs to check that the DatePicker's value is '' after clearing the form. If you check for null it will fail because the internal representation is ''.