-
-
Notifications
You must be signed in to change notification settings - Fork 571
Ensuring that the value of the “Total on Hand” column is zero #5235
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
base: main
Are you sure you want to change the base?
Ensuring that the value of the “Total on Hand” column is zero #5235
Conversation
ae07152 to
761a5f8
Compare
|
Functionality looks good. Asking @dorner for a review. |
761a5f8 to
74c3bc3
Compare
|
Hey @GiovannyCordeiro A process note here -- please avoid force pushes once we've started reviews. |
|
Oh... Sorry @cielf, I just wanted the branch to be up to date with the main branch for revision. |
|
Rechecked the functional - still looks good. |
|
Hrmm @GiovannyCordeiro It looks like there is a test failing that is related to your work. |
|
The error was happening because I didn't set the filter to enter the range of the donation creation I made. Sorry. But I've fixed it! |
|
Very good! The next step is @dorner's review -- to set expectations, that may take up to 2 weeks due to him being extra busy atm. |
| @@ -0,0 +1,99 @@ | |||
| RSpec.describe "Reports Distributions", type: :system, js: true do | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're not really testing interactivity here - if we know the parameters we can just send them in and inspect the output. System tests are a lot slower and flakier - can we move this to a request test instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK! I'll do it!
|
@GiovannyCordeiro is this ready for re-review? |
|
Not yet. Unfortunately, I haven't been able to dedicate time to it in the last two weeks. I still need to identify why the test logic doesn't work properly when a code change occurs. I hope to take a look tomorrow. Sorry, @cielf |
|
I believe @GiovannyCordeiro said they have to go quiet for awhile on another issue. |
Resolves #5004
Description
Bug fix (non-breaking change which fixes an issue)
Type of change
How Has This Been Tested?
I ran a test by creating a fictitious donation of five units, followed by a simulated distribution of those same units. In this way, the visualization in the "reports" tab of both windows showed zero, as expected.