Merge pull request #5099 from rubyforgood/packs-fixes

Custom Units / Packs fixes

Author: cielf

.gitignore

@@ -88,3 +88,4 @@ package-lock.json
 out/
 
 .vscode/
+.aider*

app/controllers/partners/requests_controller.rb

@@ -81,8 +81,11 @@ def fetch_items
       @requestable_items = PartnerFetchRequestableItemsService.new(partner_id: partner.id).call
       if Flipper.enabled?(:enable_packs)
         # hash of (item ID => hash of (request unit name => request unit plural name))
-        @item_units = Item.where(id: @requestable_items.to_h.values).to_h do |i|
-          [i.id, i.request_units.to_h { |u| [u.name, u.name.pluralize] }]
+        item_ids = @requestable_items.to_h.values
+        if item_ids.present?
+          @item_units = Item.where(id: item_ids).to_h do |i|
+            [i.id, i.request_units.to_h { |u| [u.name, u.name.pluralize] }]
+          end
         end
       end
     end

app/javascript/controllers/item_units_controller.js

@@ -43,7 +43,7 @@ export default class extends Controller {
       this.requestSelectTarget.style.display = 'inline';
       this.clearOptions()
       this.addOption('-1', 'Please select a unit')
-      this.addOption('', 'Units')
+      this.addOption('', 'units')
       for (const [index, [name, displayName]] of Object.entries(Object.entries(units))) {
         this.addOption(name, displayName)
       }

app/javascript/controllers/select2_controller.js

@@ -29,5 +29,19 @@ export default class extends Controller {
         }
       }
     });
+
+    /**
+     * This is a workaround to prevent select2 from filling in an existing
+     * value even when you try to remove everything. This solution was found at
+     * https://github.com/select2/select2/issues/3320#issuecomment-1440268574
+     */
+    if ($(this.element).prop('multiple')) {
+      select2.on("select2:unselecting", function (e) {
+          $(this).on("select2:opening", function (ev) {
+              ev.preventDefault();
+              $(this).off("select2:opening");
+          });
+      });
+    }
   }
 }

app/models/partners/item_request.rb

@@ -38,9 +38,9 @@ def request_unit_is_supported
 
     def name_with_unit(quantity_override = nil)
       if Flipper.enabled?(:enable_packs) && request_unit.present?
-        "#{item.name} - #{request_unit.pluralize(quantity_override || quantity.to_i)}"
+        "#{item&.name || name} - #{request_unit.pluralize(quantity_override || quantity.to_i)}"
       else
-        item.name
+        item&.name || name
       end
     end
   end

app/models/unit.rb

@@ -10,5 +10,7 @@
 #
 class Unit < ApplicationRecord
   belongs_to :organization
-  validates :name, uniqueness: {scope: :organization}
+  validates :name,
+    uniqueness: {scope: :organization},
+    format: {without: /\Aunits?\z/i, message: "'unit' is reserved."}
 end

app/pdfs/picklists_pdf.rb

@@ -145,7 +145,7 @@ def data_with_units(line_items)
     data + line_items.map do |line_item|
       [line_item.name,
         line_item.quantity,
-        line_item.request_unit&.capitalize&.pluralize(line_item.quantity),
+        line_item.request_unit&.pluralize(line_item.quantity),
         "[  ]",
         ""]
     end

app/services/organization_update_service.rb

@@ -18,9 +18,10 @@ def update(organization, params)
         org_params["partner_form_fields"] = org_params["partner_form_fields"].compact_blank
       end
 
-      if Flipper.enabled?(:enable_packs) && org_params[:request_unit_names]
+      if Flipper.enabled?(:enable_packs)
+        request_unit_names = org_params[:request_unit_names] || []
         # Find or create units for the organization
-        request_unit_ids = org_params[:request_unit_names].compact_blank.map do |request_unit_name|
+        request_unit_ids = request_unit_names.compact_blank.map do |request_unit_name|
           Unit.find_or_create_by(organization: organization, name: request_unit_name).id
         end
         org_params.delete(:request_unit_names)

app/views/organizations/edit.html.erb

@@ -109,7 +109,7 @@
                 <%= f.input :distribute_monthly, label: 'Does your bank distribute monthly?', as: :radio_buttons, collection: [[true, 'Yes'], [false, 'No']], label_method: :second, value_method: :first %>
 
                 <% if Flipper.enabled?(:enable_packs) %>
-                  <%= label_tag "organization[request_unit_names]", 'Custom request units used (please use singular form -- e.g. pack, not packs)' %>
+                  <%= label_tag "organization[request_unit_names]", 'Custom request units used. Please use singular form -- e.g. pack, not packs. There will be a default "unit" entry provided.' %>
                   <%= select_tag(
                     "organization[request_unit_names]",
                     options_from_collection_for_select(

app/views/partners/requests/_item_request.html.erb

@@ -12,10 +12,10 @@
       <%= field.number_field :quantity, label: false, step: 1, min: 1, class: 'form-control' %>
     </td>
 
-    <% if Flipper.enabled?(:enable_packs) && current_partner.organization.request_units.any? %>
+    <% if Flipper.enabled?(:enable_packs) && (current_partner ? current_partner.organization.request_units.any? : current_organization.request_units.any?) %>
       <td>
         <%= field.label :request_unit, "Unit", {class: 'sr-only'} %>
-        <%= field.select :request_unit, [], {include_blank: 'Units'},
+        <%= field.select :request_unit, [], {include_blank: 'units'},
                          { :'data-item-units-target' => 'requestSelect', class: 'form-control'} %>
 
       </td>