-
-
Notifications
You must be signed in to change notification settings - Fork 503
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
4590 filtering on distribution #4719
4590 filtering on distribution #4719
Conversation
@inane-pixel This one also has changes that are not related to it. I think you branched off after having made the header changes in your main. |
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.
This LGTM from a functional pov. Sending it over to @dorner for a technical look-see
} | ||
end | ||
|
||
it "ensures the totals in 'Total in Category Diapers' match" 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.
This spec doesn't seem to test the fix. It's testing the create
method, not the index
method.
You should be making a request spec for this, not a controller spec.
Added a request - also, lint needs to be addressed. |
Hey @inane-pixel - Checking in because the issue has gone stale. There's still some requested changes on this. |
@cielf Sorry, I didn't see this until now. I fixed the lint issue, but I'm not sure when I'll have a chance to work on the test. |
Closing this as I finished up the work on this issue on #4806, which was just merged. Thanks @inane-pixel for this PR, it helped me get my bearings on the issue. |
Resolves #4590
Description
Type of change
How Has This Been Tested?
Checked "Total in Category" column does matches the total of the numbers in the column
Test for "Total in Category Diaper"
Screenshots