Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

4135 Individuals Requests Preserve Values #4142

Merged

Conversation

elasticspoon
Copy link
Collaborator

@elasticspoon elasticspoon commented Feb 27, 2024

Fixes #4135

When individuals requests were submitted with incorrect values the error page did not preserve correct or incorrect inputs.

This was happening because the controller created a new request on error. Now the new request will be populated with the items from the failed request.

Notes

I attempted to reuse the failed request, however, that was not possible.

The form expects a FamilyRequest. The service object that deals with the request does not create a FamilyRequest, instead it reuses the family request service which does not store or create a FamilyRequest. The other option would be do a large refactoring and add additional objects such as an IndividualRequest and IndividaulRequestService.

That might be something worth doing, I chose not to just to get this bug fixed.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

System Tests

Fixes rubyforgood#4135

When individuals requests were submitted with incorrect values
the error page did not preserve correct or incorrect inputs. This
was happening because the controller created a new request on
error.

Now the new request will be populated with the items from the
failed request.

I attempted to reuse the failed request, however, because the
individuals request is a wrapper around a family request that
behavior was not possible.
@elasticspoon elasticspoon marked this pull request as ready for review February 28, 2024 00:14
@elasticspoon elasticspoon changed the title fix: individuals request preserve values 4135 Individuals Requests Preserve Values Feb 29, 2024
@cielf cielf requested a review from dorner March 1, 2024 14:21
Copy link
Collaborator

@dorner dorner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Just had one suggestion.

expect(page).to have_content('Still need help? Submit a support ticket here and we will do our best to follow up with you via email.')
end

it "should show invalid values in the form" do
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we combine these 2? System specs are expensive, so let's get all our expectations together :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually already drafted a PR to move the input tests to request specs: #4147. So they are separate because the check for invalid values will remain a system test while the other stuff will moved.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dorner do you still feel this way? or can this PR proceed? I would also like to rebase this PR on top of #4099 cause they have some overlap

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, since we're addressing it in a followup PR I'm OK with it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dorner - so does that mean this one meets your approval now (sounds like it)?

Copy link
Collaborator

@dorner dorner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep!

@cielf cielf merged commit 2a9f2a6 into rubyforgood:main Mar 7, 2024
19 checks passed
@elasticspoon elasticspoon deleted the 4135-request-value-preservation branch March 7, 2024 22:34
Copy link
Contributor

@elasticspoon: Your PR 4135 Individuals Requests Preserve Values is part of today's Human Essentials production release: 2024.03.17.
Thank you very much for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG]: Incorrect Item Request no preserving values
3 participants