-
-
Notifications
You must be signed in to change notification settings - Fork 472
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
Adjustments to Adult Incontinence #4361
Adjustments to Adult Incontinence #4361
Conversation
…unting of adult wipes.
app/models/item.rb
Outdated
} | ||
|
||
scope :adult_incontinence, -> { | ||
joins(:base_item) | ||
.where(items: { partner_key: %w(adult_incontinence underpads liners) }) | ||
.or(where("items.partner_key LIKE '%adult%' AND items.partner_key NOT LIKE '%cloth%'")) | ||
.or(where("items.partner_key LIKE '%adult%' AND items.partner_key NOT LIKE '%cloth%' AND items.partner_key NOT LIKE '%wipes'")) | ||
.or(where("lower(base_items.category) LIKE '%adult%' AND lower(base_items.category) NOT LIKE '%wipes%'")) |
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 is starting to get really complex... can we document these scopes with examples? I am so ready for this and BaseItem to be gone so we can manage categories on the Item level.
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.
That's a reasonable request. Or at least to say what we believe the business rules to be... There is still some ambiguity and I don't know why we are sometimes using the partner_key.
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.
I'm also going to say that fixing all the scopes is out of scope of this PR. g (It is, and yet....)
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.
Have added some long-winded commentary, and sent off a couple of additional queries/suggestions to the business.
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.
I've got some more input from NDBN, so hold off on the final review for the moment. Should get some changes in tomrorrow.
I'm going to expand this to include everything from my recent conversations around this with NDBN. Yes, it is a bit of scope creep. There was much clarification (/change) of where things should be, but we're able to make things a bit simpler as a result. The big things are: making liners two base items -- one for menstrual (which should be the default for what we have already, and one for AI) and clarifying things we either weren't counting or were double counting. |
…es in line with prod.
Hey @dorner -- I've addressed a lot of the ambiguities and outright errors in these scopes. We should now be in a state where everything gets counted, but only gets counted once. I think we might not need quite so much commenting now! |
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.
I'm going to trust your business logic here because there's no way I can follow this, even with the simplification. 😃 I think a file got added by mistake though.
lib/tasks/fetch_latest_db.rake
Outdated
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.
I think this wasn't supposed to be checked in?
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.
You're right on that. Will fix today.
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.
Bonus, I get to learn how to do this!
Resolves #4090
Description
Moving adult diapers from disposables to adult_incontinence
Stop double counting adult wipes (now in other)
adjusted the diapers in kit code to match
Type of change
How Has This Been Tested?
Just automatic tests.