-
-
Notifications
You must be signed in to change notification settings - Fork 568
5251 remove enable packs #5277
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?
5251 remove enable packs #5277
Conversation
…ization edit form
…d confirmation email
…uantity in DistributionPdf
…e proper unit display in CSV export
…ions and improve quantity calculations
…ganyni1/human-essentials into 5251-remove-enable-packs
…o quantity handling
This is going to take a couple of sessions to do the functional check -- we basically have to check every packs-related thing (and there are a lot of them) |
I understand, please let me know if I need to change anything. |
Looks good to me -- over to @dorner for technical insight. |
app/models/partners/item_request.rb
Outdated
else | ||
item&.name || name | ||
end | ||
unit_text = request_unit.present? ? request_unit.pluralize(quantity_override || quantity.to_i) : "items" |
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 changes the behavior in that it adds " - items" where it didn't do that before.
@cielf thoughts?
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 you're right -- the "items" is unneeded.
row += Array.new(item_headers.size, 0) | ||
|
||
request.item_requests.each do |item_request| | ||
item_name = item_request.item.present? ? item_request.name_with_unit(0) : DELETED_ITEMS_COLUMN_HEADER |
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.
Why did this have to change?
["Item 2", 30, 100], | ||
["Item 3", 50, ""], | ||
["Item 4", 120, ""], | ||
["Item 4", "120 packs", ""], |
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.
What caused this change?
end | ||
|
||
after do | ||
Flipper.disable(:enable_packs) |
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 block can be removed entirely now.
"2T Diapers -- UPDATED", | ||
"3T Diapers", | ||
"4T Diapers", | ||
"4T Diapers - packs", |
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.
Why did we have to add this?
let(:organization) { create(:organization) } | ||
let(:fake_organization_items) { instance_double('organization.items') } | ||
let(:fake_organization_item) { instance_double(Item, id: 99_999, save!: -> {}) } | ||
let(:fake_organization_item) { instance_double(Item, id: 99_999, save!: -> {}, sync_request_units!: true) } |
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.
Same question - if all we're doing is removing the feature flag, then tests should be identical with the exception of any tests that had the flag off.
Resolves #5251
Description
This change removes the Flipper feature flag for "enable packs" since the feature is now permanently enabled in production. The work involves:
Type of change
How Has This Been Tested?
Screenshots
N/A (backend changes only)