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

Issue 4117 - Make distribution_quantity disallow 0 #4259

Merged
merged 2 commits into from
Apr 23, 2024

Conversation

pshong79
Copy link
Collaborator

@pshong79 pshong79 commented Apr 7, 2024

Resolves #4117

Description

The item quantity is being used to make some calculations in some reports. Because there some reports that use this value in some calculations, when a 0 is entered, a divide by 0 error is displayed.

To address this issue, a change to one of the validations in the items model was made to only allow for numeric value greater_than 0.

Type of change

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

How Has This Been Tested?

Tested manually
Added test specific to item distribution_quantity to check for 0

Screenshots

After:
Screenshot 2024-04-06 at 12 36 55

@dorner
Copy link
Collaborator

dorner commented Apr 9, 2024

This looks OK to me - question is what we do for existing zero numbers? @cielf should we replace them with null?

@cielf
Copy link
Collaborator

cielf commented Apr 10, 2024

Hmm... I think that's the only thing that makes sense and that we're going to have to communicate to the affected banks.

@pshong79
Copy link
Collaborator Author

Ok. Thanks. I will work on getting a script created to update the distribution_quantity to null if they are currently 0.

@pshong79
Copy link
Collaborator Author

Should there be a test for the update script? I feel like there should be?

@dorner
Copy link
Collaborator

dorner commented Apr 19, 2024

@pshong79 you'll want a migration here. And no, I don't think you need a test. It should be one line of code.

@pshong79
Copy link
Collaborator Author

@dorner Thanks! I initially thought it was a migration but recalled previously when I've done something like this to update current data in the database, I had used a script.

I will go back go back to creating a migration.

@pshong79
Copy link
Collaborator Author

Migration is done. I don't believe I need to commit my local schema.rb file as I believe when the migration is run in production, the schema.rb there will get updated? If I need to push my local one up, I can do that.

@dorner
Copy link
Collaborator

dorner commented Apr 21, 2024

Looks good to me! Will wait for @cielf to merge if there is communication that has to happen first.

@cielf
Copy link
Collaborator

cielf commented Apr 23, 2024

@dorner Thanks for the heads up re communication. I think we can handle it as part of the release announcement -- we don't need a pre-release note. So, merging.

@cielf cielf merged commit 5df5a28 into rubyforgood:main Apr 23, 2024
19 checks passed
Copy link
Contributor

@pshong79: Your PR Issue 4117 - Make distribution_quantity disallow 0 is part of today's Human Essentials production release: 2024.04.28.
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.

Disallow 0 on item quantity
3 participants