Merge branch 'main' into bw/no-mixed-units/4579

app/controllers/distributions_by_county_controller.rb

@@ -4,8 +4,9 @@ class DistributionsByCountyController < ApplicationController
 
   def report
     setup_date_range_picker
-    start_date = helpers.selected_range.first.iso8601
-    end_date = helpers.selected_range.last.iso8601
+    start_date = helpers.selected_range.first.utc.iso8601
+    end_date = helpers.selected_range.last.utc.iso8601
+
     @breakdown = DistributionSummaryByCountyQuery.call(
       organization_id: current_organization.id,
       start_date: start_date,

app/controllers/purchases_controller.rb

@@ -78,9 +78,14 @@ def update
 
   def destroy
     purchase = current_organization.purchases.find(params[:id])
-    PurchaseDestroyService.call(purchase)
+    begin
+      PurchaseDestroyService.call(purchase)
+    rescue => e
+      flash[:error] = e.message
+    else
+      flash[:notice] = "Purchase #{params[:id]} has been removed!"
+    end
 
-    flash[:notice] = "Purchase #{params[:id]} has been removed!"
     redirect_to purchases_path
   end
 

app/controllers/reports_controller.rb

@@ -8,7 +8,7 @@ def donations_summary
 
   def manufacturer_donations_summary
     @recent_donations_from_manufacturers = current_organization.donations.during(helpers.selected_range).by_source(:manufacturer)
-    @top_manufacturers = current_organization.manufacturers.by_donation_count(10, helpers.selected_range)
+    @recent_manufacturers = current_organization.manufacturers.by_donation_date(10, helpers.selected_range)
   end
 
   def purchases_summary

app/models/manufacturer.rb

@@ -26,14 +26,14 @@ def volume
     donations.joins(:line_items).sum(:quantity)
   end
 
-  def self.by_donation_count(count = 10, date_range = nil)
+  def self.by_donation_date(count = 10, date_range = nil)
     # selects manufacturers that have donation qty > 0 in the provided date range
-    # and sorts them by highest volume of donation
+    # and sorts them by the date of the most recent donation
     joins(donations: :line_items).where(donations: { issued_at: date_range })
-      .select('manufacturers.*, sum(line_items.quantity) as donation_count')
+      .select('manufacturers.*, sum(line_items.quantity) as donation_count, max(donations.issued_at) as donation_date')
       .group('manufacturers.id')
       .having('sum(line_items.quantity) > 0')
-      .order('donation_count DESC')
+      .order('donation_date DESC')
       .limit(count)
   end
 

app/models/organization.rb

@@ -197,7 +197,7 @@ def total_inventory
   end
 
   def self.seed_items(organization = Organization.all)
-    base_items = BaseItem.all.map(&:to_h)
+    base_items = BaseItem.without_kit.map(&:to_h)
 
     Array.wrap(organization).each do |org|
       Rails.logger.info "\n\nSeeding #{org.name}'s items...\n"

app/models/transfer.rb

@@ -33,6 +33,7 @@ class Transfer < ApplicationRecord
   validate :storage_locations_belong_to_organization
   validate :storage_locations_must_be_different
   validate :from_storage_quantities
+  validate :line_items_quantity_is_positive
 
   def self.csv_export_headers
     ["From", "To", "Comment", "Total Moved"]
@@ -83,4 +84,8 @@ def insufficient_items
     inventory = View::Inventory.new(organization_id)
     line_items.select { |i| i.quantity > inventory.quantity_for(item_id: i.item_id) }
   end
+
+  def line_items_quantity_is_positive
+    line_items_quantity_is_at_least(1)
+  end
 end

app/queries/distribution_summary_by_county_query.rb

@@ -56,6 +56,8 @@ class DistributionSummaryByCountyQuery
   SQL
 
   class << self
+    # Timestamps are stored in Postgres without timezones so
+    # start_date and end_date must be strings in UTC.
     def call(organization_id:, start_date: nil, end_date: nil)
       params = {
         organization_id: organization_id,

app/views/partners/profiles/edit/_agency_information.html.erb

@@ -11,7 +11,7 @@
             <%= profile_form.input :agency_type, collection: Partner::AGENCY_TYPES.values, label: "Agency Type", class: "form-control", wrapper: :input_group %>
             <%= profile_form.input :other_agency_type, label: "Other Agency Type", class: "form-control", wrapper: :input_group %>
             <div class="form-group row">
-              <label class="control-label col-md-3">501(c)(3) IRS Determination Letter</label>
+              <label class="control-label col-md-3">501(c)(3) IRS Determination Letter or other Proof of Agency Status</label>
               <% if profile.proof_of_partner_status.attached? %>
                 <div class="col-md-8">
                   Attached

app/views/partners/profiles/show/_agency_information.html.erb

@@ -16,7 +16,7 @@
       <dt>Status</dt>
       <dd><%= profile.partner.display_status %></dd>
 
-      <dt>501(c)(3) IRS Determination Letter</dt>
+      <dt>501(c)(3) IRS Determination Letter or other Proof of Agency Status</dt>
       <% if profile.proof_of_partner_status.attached? %>
         <dd>Attached
           file: <%= link_to profile.proof_of_partner_status.blob['filename'], rails_blob_path(profile.proof_of_partner_status), class: "font-weight-bold" %></dd>

app/views/partners/profiles/step/_agency_information_form.html.erb

@@ -12,7 +12,7 @@
   </div>
 
   <div class="form-group row">
-    <label class="control-label col-md-3">501(c)(3) IRS Determination Letter</label>
+    <label class="control-label col-md-3">501(c)(3) IRS Determination Letter or other Proof of Agency Status</label>
     <% if profile.proof_of_partner_status.attached? %>
       <div class="col-md-8">
         Attached file: <%= link_to profile.proof_of_partner_status.blob['filename'], rails_blob_path(profile.proof_of_partner_status), class: "font-weight-bold" %>

app/views/partners/show.html.erb

@@ -238,7 +238,7 @@
                 <tbody>
                 <% @partner_distributions.each do |dist| %>
                   <tr>
-                    <td><%= dist.created_at.strftime("%m/%d/%Y") %></td>
+                    <td><%= dist.issued_at.strftime("%m/%d/%Y") %></td>
                     <td><%= dist.storage_location.name %></td>
                     <td><%= dist.line_items.total %></td>
                     <td class="text-right">

app/views/profiles/_show.html.erb

@@ -10,7 +10,7 @@
       (<%= partner_profile.other_agency_type %>)
     <% end %>
     </p>
-    <p>Proof of Agency Status
+    <p>501(c)(3) IRS Determination Letter or other Proof of Agency Status:
     <% if partner_profile.proof_of_partner_status.attached? %>
       <!-- NOTE: The actual download link may not work in local storage mode due to file locations being seperate locally. -->
       (Link): <%= link_to partner_profile.proof_of_partner_status.filename, rails_blob_path(partner_profile.proof_of_partner_status, disposition: 'inline') %>

app/views/reports/manufacturer_donations_summary.html.erb

@@ -26,8 +26,8 @@
   </span>
   </h3>
   <div class="box-body">
-    <h4>Top Manufacturer Donations</h4>
-    <%= render partial: "manufacturer", collection: @top_manufacturers, as: :manufacturer %>
+    <h4>Recent Manufacturer Donations</h4>
+    <%= render partial: "manufacturer", collection: @recent_manufacturers, as: :manufacturer %>
   </div>
 
 <% end %>

docs/user_guide/bank/pm_partner_profiles.md

@@ -41,7 +41,7 @@ This section contains basic agency information that most, if not all, partners w
 - Agency Name
 - Agency Type
 - Other Agency Type (i.e, details if "other" is chosen in Agency Type)
-- 501(c)(3) IRS Determination Letter  -- this is a file to upload said letter.
+- 501(c)(3) IRS Determination Letter or other Proof of Agency Status  -- this is a file to upload said letter.
 - Agency Mission
 - Address
 - City

spec/models/manufacturer_spec.rb

@@ -46,6 +46,40 @@
         expect(mfg.volume).to eq(5)
       end
     end
+
+    describe "by_donation_date" do
+      before do
+        # Prepare manufacturers with donations for tests
+        today = Time.zone.today
+        from = (today - 1.month).beginning_of_day
+        to = today.end_of_day
+        dates_in_order = [
+          today,
+          today - 1.day,
+          today - 2.days,
+          today - 3.days
+        ]
+
+        @mfg1 = create(:manufacturer)
+        create(:donation, :with_items, item_quantity: 5, source: Donation::SOURCES[:manufacturer], manufacturer: @mfg1, issued_at: dates_in_order[0])
+        create(:donation, :with_items, item_quantity: 5, source: Donation::SOURCES[:manufacturer], manufacturer: @mfg1, issued_at: dates_in_order[3])
+        @mfg2 = create(:manufacturer)
+        create(:donation, :with_items, item_quantity: 5, source: Donation::SOURCES[:manufacturer], manufacturer: @mfg2, issued_at: dates_in_order[1])
+        create(:donation, :with_items, item_quantity: 5, source: Donation::SOURCES[:manufacturer], manufacturer: @mfg1, issued_at: dates_in_order[2])
+        create(:manufacturer)
+        mfg_no_in_range = create(:manufacturer)
+        create(:donation, :with_items, item_quantity: 5, source: Donation::SOURCES[:manufacturer], manufacturer: mfg_no_in_range, issued_at: today - 1.year)
+        @mfg_by_donation = Manufacturer.all.by_donation_date(10, from..to)
+      end
+
+      it "ignores manufacturers with no donations in the date range" do
+        expect(@mfg_by_donation.length).to eq(2)
+      end
+
+      it "returns manufacturers in order of their most recent donation" do
+        expect(@mfg_by_donation).to match_array([@mfg1, @mfg2])
+      end
+    end
   end
 
   context "Private Methods" do

spec/models/organization_spec.rb

@@ -238,6 +238,14 @@
 
         expect(organization.items.count).to eq(1)
       end
+
+      it "should exclude kit" do
+        create(:base_item, name: "Kit", partner_key: "foo")
+
+        Organization.seed_items(organization)
+
+        expect(organization.items.count).to eq(0)
+      end
     end
 
     context "when no organization is provided" do

spec/models/transfer_spec.rb

@@ -54,6 +54,12 @@
       })
       expect(transfer).not_to be_valid
     end
+
+    it "requires that the quantity must be positive" do
+      item = create(:item)
+      transfer = build(:transfer, :with_items, item: item, item_quantity: -1)
+      expect(transfer).not_to be_valid
+    end
   end
 
   context "Scopes >" do

spec/requests/partners_requests_spec.rb

@@ -87,7 +87,11 @@
       response
     end
 
-    let(:partner) { create(:partner, organization: organization, status: :approved) }
+    let(:partner) do
+      partner = create(:partner, organization: organization, status: :approved)
+      partner.distributions << create(:distribution, :with_items, :past, item_quantity: 1231)
+      partner
+    end
     let!(:family1) { FactoryBot.create(:partners_family, guardian_zip_code: '45612-123', partner: partner) }
     let!(:family2) { FactoryBot.create(:partners_family, guardian_zip_code: '45612-126', partner: partner) }
     let!(:family3) { FactoryBot.create(:partners_family, guardian_zip_code: '45612-123', partner: partner) }
@@ -107,14 +111,22 @@
     context "html" do
       let(:response_format) { 'html' }
 
+      it "displays distribution scheduled date" do
+        subject
+        partner.distributions.each do |distribution|
+          expect(subject.body).to include(distribution.issued_at.strftime("%m/%d/%Y"))
+          expect(subject.body).to_not include(distribution.created_at.strftime("%m/%d/%Y"))
+        end
+      end
+
       context "without org admin" do
         it 'should not show the manage users button' do
           expect(subject).to be_successful
           expect(subject.body).not_to include("Manage Users")
         end
       end
 
-      context "without org admin" do
+      context "with org admin" do
         before(:each) do
           user.add_role(Role::ORG_ADMIN, organization)
         end

spec/requests/reports/manufacturer_donations_summary_spec.rb

@@ -55,10 +55,10 @@
           expect(response.body).to match(%r{Manufacturer 3 \(13\)})
         end
 
-        it "shows top manufacturers in desc. order" do
+        it "shows recent manufacturers in desc. order of most recent donation" do
           get reports_manufacturer_donations_summary_path(user.organization), params: {filters: {date_range: formatted_date_range}}
 
-          expect(response.body).to match(%r{Manufacturer 3 .* Manufacturer 1 .*Manufacturer 2}m)
+          expect(response.body).to match(%r{Manufacturer 2 .* Manufacturer 1 .*Manufacturer 3}m)
         end
       end
     end

spec/system/purchase_system_spec.rb

@@ -301,14 +301,30 @@
       sign_in organization_admin
     end
 
-    it "allows deletion of a purchase" do
-      visit "#{subject}/#{purchase.id}"
-      expect(page).to have_link("Delete")
-      accept_confirm do
-        click_on "Delete"
+    context "When the purchase remains in storage location" do
+      it "allows deletion of a purchase" do
+        visit "#{subject}/#{purchase.id}"
+        expect(page).to have_link("Delete")
+        accept_confirm do
+          click_on "Delete"
+        end
+        expect(page).to have_content "Purchase #{purchase.id} has been removed!"
+        expect(page).to have_content "0 (Total)"
+      end
+    end
+
+    context "When the purchase has been distributed" do
+      it "delete a purchase should get an error" do
+        allow(PurchaseDestroyService).to receive(:call).with(purchase).and_raise(InventoryError)
+
+        visit "#{subject}/#{purchase.id}"
+        expect(page).to have_link("Delete")
+        accept_confirm do
+          click_on "Delete"
+        end
+
+        expect(page).to have_css(".alert.error.alert-danger")
       end
-      expect(page).to have_content "Purchase #{purchase.id} has been removed!"
-      expect(page).to have_content "0 (Total)"
     end
   end
 end