fix(kit_creation): update item visibility and value during kit creation

app/controllers/items_controller.rb

@@ -73,18 +73,18 @@ def show
 
   def update
     @item = current_organization.items.find(params[:id])
-    @item.attributes = item_params
     deactivated = @item.active_changed? && !@item.active
     if deactivated && !@item.can_deactivate?
       flash.now[:error] = "Can't deactivate this item - it is currently assigned to either an active kit or a storage location!"
       render action: :edit
       return
     end
+    result = ItemUpdateService.new(item: @item, params: item_params, request_unit_ids: request_unit_ids).call
 
-    if update_item
+    if result.success?
       redirect_to items_path, notice: "#{@item.name} updated!"
     else
-      flash.now[:error] = "Something didn't work quite right -- try again? #{@item.errors.map { |error| "#{error.attribute}: #{error.message}" }}"
+      flash.now[:error] = result.error.record.errors.full_messages.to_sentence
       render action: :edit
     end
   end
@@ -185,27 +185,6 @@ def request_unit_ids
     params.require(:item).permit(request_unit_ids: []).fetch(:request_unit_ids, [])
   end
 
-  # We need to update both the item and the request_units together and fail together
-  def update_item
-    if Flipper.enabled?(:enable_packs)
-      update_item_and_request_units
-    else
-      @item.save
-    end
-  end
-
-  def update_item_and_request_units
-    begin
-      Item.transaction do
-        @item.save!
-        @item.sync_request_units!(request_unit_ids)
-      end
-    rescue
-      return false
-    end
-    true
-  end
-
   helper_method \
     def filter_params(_parameters = nil)
     return {} unless params.key?(:filters)

app/services/item_update_service.rb

@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+class ItemUpdateService
+  attr_reader :item, :params, :request_unit_ids
+  def initialize(item:, params:, request_unit_ids: [])
+    @item = item
+    @request_unit_ids = request_unit_ids
+    @params = params
+  end
+
+  def call
+    ActiveRecord::Base.transaction do
+      item.update!(params)
+      update_kit_value
+      if Flipper.enabled?(:enable_packs)
+        item.sync_request_units!(request_unit_ids)
+      end
+    end
+    Result.new(value: item)
+  rescue => e
+    Result.new(error: e)
+  end
+
+  private
+
+  def update_kit_value
+    return unless item.kit
+
+    kit_value_in_cents = item.kit.items.reduce(0) do |sum, i|
+      sum + i.value_in_cents.to_i * item.kit.line_items.find_by(item_id: i.id).quantity.to_i
+    end
+    item.kit.update!(value_in_cents: kit_value_in_cents)
+  end
+end

app/services/kit_create_service.rb

@@ -43,6 +43,11 @@ def call
       unless item_creation_result.success?
         raise item_creation_result.error
       end
+      kit.items.update_all(visible_to_partners: kit.visible_to_partners)
+      kit_value_in_cents = kit.items.reduce(0) do |sum, i|
+        sum + i.value_in_cents.to_i * kit.line_items.find_by(item_id: i.id).quantity.to_i
+      end
+      kit.update!(value_in_cents: kit_value_in_cents)
     rescue StandardError => e
       errors.add(:base, e.message)
       raise ActiveRecord::Rollback

app/views/items/_form.html.erb

@@ -22,25 +22,25 @@
                 <%= f.input :reporting_category, label: 'NDBN Reporting Category', collection: Item.reporting_categories_for_select, disabled: !!@item.kit_id, required: !@item.kit_id, hint: @reporting_category_hint %>
               </div>
 
-              <%= f.input :name, label: "Value per item", wrapper: :input_group do %>
+              <%= f.input :value_in_dollars, label: "Value per item", wrapper: :input_group do %>
                 <span class="input-group-text"><i class="fa fa-money"></i></span>
                 <%= f.input_field :value_in_dollars, class: "form-control" %>
               <% end %>
 
-              <%= f.input :name, label: "Quantity Per Individual", wrapper: :input_group do %>
+              <%= f.input :distribution_quantity, label: "Quantity Per Individual", wrapper: :input_group do %>
                 <%= f.input_field :distribution_quantity, class: "form-control" %>
               <% end %>
 
               <% if current_user.has_cached_role?(Role::ORG_ADMIN, current_organization) %>
-                <%= f.input :name, label: "On hand minimum quantity", wrapper: :input_group do %>
+                <%= f.input :on_hand_minimum_quantity, label: "On hand minimum quantity", wrapper: :input_group do %>
                   <%= f.input_field :on_hand_minimum_quantity, input_html: {value: 0}, class: "form-control" %>
                 <% end %>
-                <%= f.input :name, label: "On hand recommended quantity", wrapper: :input_group do %>
+                <%= f.input :on_hand_recommended_quantity, label: "On hand recommended quantity", wrapper: :input_group do %>
                   <%= f.input_field :on_hand_recommended_quantity, class: "form-control" %>
                 <% end %>
               <% end %>
 
-              <%= f.input :name, label: "Package size", wrapper: :input_group do %>
+              <%= f.input :package_size, label: "Package size", wrapper: :input_group do %>
                 <%= f.input_field :package_size, class: "form-control", min: 0 %>
               <% end %>
 
@@ -50,7 +50,7 @@
                 <% end %>
               <% end %>
 
-              <%= f.input :visible, label: "Item is Visible to Partners?", wrapper: :input_group do %>
+              <%= f.input :visible_to_partners, label: "Item is Visible to Partners?", wrapper: :input_group do %>
                 <%= f.check_box :visible_to_partners, {class: "input-group-text", id: "visible_to_partners"}, "true", "false" %>
               <% end %>
 

app/views/kits/_form.html.erb

@@ -14,11 +14,6 @@
                 <%= f.check_box :visible_to_partners, {class: "input-group-text", id: "visible_to_partners"}, "true", "false" %>
               <% end %>
 
-              <%= f.input :value_in_cents, label: "Value for kit", wrapper: :input_group do %>
-                <span class="input-group-text"><i class="fa fa-dollar"></i></span>
-                <%= f.input_field :value_in_dollars, class: "form-control", min: 0 %>
-              <% end %>
-
               <fieldset style="margin-bottom: 2rem;" class='w-70'>
                 <legend>Items in this Kit</legend>
                 <div id="kit_line_items" class="line-item-fields" data-capture-barcode="true">
@@ -38,5 +33,6 @@
           </div>
         </div>
       </div>
+    </div>
   </section>
 <% end %>

app/views/kits/_table.html.erb

@@ -3,6 +3,7 @@
 <tr>
   <th>Name</th>
   <th>Items</th>
+  <th>Value for kit</th>
   <th>Allocations</th>
   <th class="text-right">Actions</th>
 </tr>
@@ -18,6 +19,12 @@
           <% end %>
         </ul>
       </td>
+      <td>
+        <span class="tooltip-target" data-toggle="tooltip" data-placement="top" title="Total value of all items in this kit based on current inventory item values">
+          <%= number_to_currency(kit.value_in_dollars) %>
+          <i class="fa fa-info-circle"></i>
+        </span>
+      </td>
       <td class='d-flex flex-column'>
         <table>
           <thead>

spec/services/item_update_service_spec.rb

@@ -0,0 +1,79 @@
+# frozen_string_literal: true
+
+require "rspec"
+
+RSpec.describe ItemUpdateService, type: :service do
+  describe ".call" do
+    subject { described_class.new(item: item, params: params, request_unit_ids: request_unit_ids).call }
+
+    let(:kit) { create(:kit) }
+    let(:item) { create(:item, kit: kit) }
+    let(:item2) { create(:item, kit: kit) }
+    let(:params) do
+      {
+        name: "Updated Item Name",
+        reporting_category: "pads",
+        value_in_cents: 2000
+      }
+    end
+    let(:request_unit_ids) { [] }
+    let(:kit_value_in_cents) do
+      kit.line_items.reduce(0) do |sum, li|
+        item = Item.find(li.item_id)
+        sum + item.value_in_cents.to_i * li.quantity.to_i
+      end
+    end
+
+    context "params are ok" do
+      it "returns a Result with success? true and the item" do
+        result = subject
+        expect(result).to be_a_kind_of(Result)
+        expect(result.success?).to eq(true)
+        expect(result.value).to eq(item)
+      end
+
+      it "updates the item attributes" do
+        subject
+        item.reload
+        expect(item.name).to eq("Updated Item Name")
+        expect(item.value_in_cents).to eq(2000)
+      end
+
+      it "updates the kit value_in_cents" do
+        subject
+        kit.reload
+        expect(kit.value_in_cents).to eq(kit_value_in_cents)
+      end
+    end
+
+    context "params are invalid" do
+      let(:params) do
+        {
+          name: "" # Invalid as name can't be blank
+        }
+      end
+
+      it "returns a Result with success? false and an error" do
+        result = subject
+        expect(result).to be_a_kind_of(Result)
+        expect(result.success?).to eq(false)
+        expect(result.error).to be_a(ActiveRecord::RecordInvalid)
+        expect(result.error.message).to include("Validation failed: Name can't be blank")
+      end
+
+      it "does not update the item attributes" do
+        original_name = item.name
+        subject
+        item.reload
+        expect(item.name).to eq(original_name)
+      end
+
+      it "does not update the kit value_in_cents" do
+        original_kit_value = kit.value_in_cents
+        subject
+        kit.reload
+        expect(kit.value_in_cents).to eq(original_kit_value)
+      end
+    end
+  end
+end

spec/services/kit_create_service_spec.rb

@@ -24,22 +24,25 @@
         }
       end
     end
+    let(:kit_value_in_cents) do
+      line_items_attr.sum do |li|
+        item = Item.find(li[:item_id])
+        item.value_in_cents.to_i * li[:quantity].to_i
+      end
+    end
 
     it 'should return an the instance' do
       expect(subject).to be_a_kind_of(described_class)
     end
 
     context 'when the parameters are valid' do
       it 'should create a new Kit' do
-        expect { subject }.to change { Kit.all.count }.by(1)
+        expect { subject }.to change { Kit.count }.by(1)
+        expect(Kit.last.value_in_cents).to eq(kit_value_in_cents)
       end
 
       it 'should create a new Item' do
-        expect { subject }.to change { Item.all.count }.by(1)
-      end
-
-      it 'should create the new Item associated with the Kit' do
-        expect { subject }.to change { Kit.all.count }.by(1)
+        expect { subject }.to change { Item.count }.by(1)
       end
 
       context 'but an unexpected error gets raised' do
@@ -92,6 +95,14 @@
         end
       end
 
+      context 'line_items_attributes is empty' do
+        let(:line_items_attr) { [] }
+
+        it 'should have an error At least one item is required' do
+          expect(subject.errors.full_messages).to include("At least one item is required")
+        end
+      end
+
       context 'because the kit_params is invalid for kit creation' do
         let(:kit_params) { { organization_id: organization_id } }
         let(:kit_validation_errors) do