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.

Sign up for GitHub

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

Jump to bottom

4134 fix inconsistent request behavior #4146

Merged
merged 2 commits into from
Mar 3, 2024
Merged

4134 fix inconsistent request behavior #4146

merged 2 commits into from
Mar 3, 2024

Conversation

elasticspoon
Copy link
Collaborator

@elasticspoon elasticspoon commented Feb 29, 2024 •
edited
Loading

Resolves #4134

Description

Individual requests and Quantity requests are very similar but have small differences in behavior:

Quantity:

Comment Item Number Result
Valid Valid Valid Valid
Valid Blank Blank Valid
Valid 1 row Blank 1 row Blank Valid (removes blank row)
Valid Blank Valid Invalid
Valid Valid Blank Invalid

Individual:

Comment Item Number Result
Valid Valid Valid Valid
Valid Blank Blank Invalid
Valid 1 row Blank 1 row Blank Invalid
Valid Blank Valid Invalid
Valid Valid Blank Invalid

This PR makes Individual requests behave like quantity requests:

Target => Both act like:

Comment Item Number Result
Valid Valid Valid Valid
Valid Blank Blank Valid
Valid 1 row Blank 1 row Blank Valid (removes blank row)
Valid Blank Valid Invalid
Valid Valid Blank Invalid

Type of change

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

How Has This Been Tested?

System Tests

@elasticspoon elasticspoon force-pushed the 4134-fix-inconsistent-request-behavior branch 2 times, most recently from 0cb9f9d to 43ab7e2 Compare February 29, 2024 20:56
@elasticspoon
Copy link
Collaborator Author

The initial implementation here has the testing done as system tests. Given that it isn't actually doing any front end interaction, just filling in fields and submitting I want to move a lot of the tests to requests specs. That seems out of scope of this PR because I will be moving other tests from the current system spec to that request test as well.

@elasticspoon
fix(rubyforgood#4134): inconsistent request behavior
f72eef8 
Fixes rubyforgood#4134

Individual requests and Quantity requests are very similar
but have small differences in behavior:
- Quantity requests allow comment only requests
- Quantity requests filter out blank lines

This PR will unify the behavior so both Individual and Quantity requests
will act like Quantity requests.
@elasticspoon elasticspoon force-pushed the 4134-fix-inconsistent-request-behavior branch from 43ab7e2 to f72eef8 Compare February 29, 2024 21:03
@elasticspoon elasticspoon marked this pull request as ready for review February 29, 2024 21:10
@cielf cielf requested a review from dorner March 1, 2024 14:24
dorner
dorner requested changes Mar 1, 2024
View reviewed changes
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.

As long as we intend to move a bunch of these to request specs in a followup, I'm fine with this. Just wondering about the weird schema.rb change.

db/schema.rb Outdated Show resolved Hide resolved
spec/system/partners/managing_requests_system_spec.rb Outdated Show resolved Hide resolved
@elasticspoon
fix: minor changes
0f4999e 
fix: typo

fix: revert automated minor schema change
@elasticspoon
Copy link
Collaborator Author

As long as we intend to move a bunch of these to request specs in a followup, I'm fine with this. Just wondering about the weird schema.rb change.

as mentioned in the other PR, i have a draft already #4147

@elasticspoon elasticspoon requested a review from dorner March 1, 2024 22:13
@dorner
Copy link
Collaborator

dorner commented Mar 3, 2024

Nice, looks good! Will wait to merge till after deploy.

@dorner dorner merged commit 8cf8d89 into rubyforgood:main Mar 3, 2024
19 checks passed
@elasticspoon elasticspoon deleted the 4134-fix-inconsistent-request-behavior branch March 4, 2024 02:11
@github-actions GitHub Actions
Copy link
Contributor

@elasticspoon: Your PR 4134 fix inconsistent request behavior 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
Reviewers

@dorner dorner dorner approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG]: Inconsistent Behavior in Requests
2 participants
@elasticspoon @dorner