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

fix: empty item request regression #4169

Merged
merged 1 commit into from
Mar 10, 2024

Conversation

elasticspoon
Copy link
Collaborator

@elasticspoon elasticspoon commented Mar 8, 2024

A previous PR by me introduced a regression that made individuals requests fail with empty attributes.

The mistaken assumption I made was assuming an empty request would be:

params[:partners_family_request][:items_attributes] = {
    "0" => {person_count: nil, item_id: nil}
}

should be:

params[:partners_family_request][:items_attributes] = nil

This causes FamilyRequestService to fail. This PR fixes the regression.

Also expect { @partner.reload }.to change(@partner, :requestable_items).from([]).to(items_in_category) created order dependent flakiness. That has been fixed.

Type of change

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

How Has This Been Tested?

Request Tests

@elasticspoon elasticspoon force-pushed the fix-empty-individual-requests branch 2 times, most recently from 13c826b to 5124a03 Compare March 9, 2024 00:55
A previous PR by me introduced a regression that made
individuals requests fail with empty attributes.

The mistaken assumption I made was assuming an empty request
would have params family_attrs => 0 => {id: "", value: ""}

In reality they should be nil.

Also introduced a bit of flakiness in the partner_system_spec.
That has been fixed.
@elasticspoon elasticspoon force-pushed the fix-empty-individual-requests branch from 5124a03 to 41960e3 Compare March 9, 2024 01:01
@elasticspoon elasticspoon marked this pull request as ready for review March 9, 2024 01:10
@dorner
Copy link
Collaborator

dorner commented Mar 10, 2024

Nice, thanks!

@dorner dorner merged commit bdf6f75 into rubyforgood:main Mar 10, 2024
19 checks passed
@elasticspoon elasticspoon deleted the fix-empty-individual-requests branch March 10, 2024 19:30
Copy link
Contributor

@elasticspoon: Your PR fix: empty item request regression 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.

None yet

2 participants