Merge branch 'rubyforgood:main' into 4681-donation-sites-export-csv

Author: Nisuha

app/controllers/concerns/importable.rb

@@ -27,8 +27,12 @@ def import_csv
       data = File.read(params[:file].path, encoding: "BOM|UTF-8")
       csv = CSV.parse(data, headers: true).reject { |row| row.to_hash.values.any?(&:nil?) }
       if csv.count.positive? && csv.first.headers.all? { |header| !header.nil? }
-        resource_model.import_csv(csv, current_organization.id)
-        flash[:notice] = "#{resource_model_humanized} were imported successfully!"
+        errors = resource_model.import_csv(csv, current_organization.id)
+        if errors.empty?
+          flash[:notice] = "#{resource_model_humanized} were imported successfully!"
+        else
+          flash[:error] = "The following #{resource_model_humanized} did not import successfully:\n#{errors.join("\n")}"
+        end
       else
         flash[:error] = "Check headers in file!"
       end

app/models/concerns/provideable.rb

@@ -19,6 +19,7 @@ def self.import_csv(csv, organization)
 
         loc.save!
       end
+      []
     end
 
     def self.csv_export_headers

app/models/donation_site.rb

@@ -46,6 +46,7 @@ def self.import_csv(csv, organization)
       loc.organization_id = organization
       loc.save!
     end
+    []
   end
 
   def self.csv_export_headers

app/models/partner.rb

@@ -158,14 +158,19 @@ def approvable?
 
   # better to extract this outside of the model
   def self.import_csv(csv, organization_id)
+    errors = []
     organization = Organization.find(organization_id)
 
     csv.each do |row|
       hash_rows = Hash[row.to_hash.map { |k, v| [k.downcase, v] }]
 
       svc = PartnerCreateService.new(organization: organization, partner_attrs: hash_rows)
       svc.call
+      if svc.errors.present?
+        errors << "#{svc.partner.name}: #{svc.partner.errors.full_messages.to_sentence}"
+      end
     end
+    errors
   end
 
   def self.csv_export_headers

app/models/storage_location.rb

@@ -121,6 +121,7 @@ def self.import_csv(csv, organization)
       loc.organization_id = organization
       loc.save!
     end
+    []
   end
 
   # NOTE: We should generalize this elsewhere -- Importable concern?

app/services/partner_create_service.rb

@@ -11,27 +11,27 @@ def initialize(organization:, partner_attrs:)
   def call
     @partner = organization.partners.build(partner_attrs)
 
-    unless @partner.valid?
+    if @partner.valid?
+      ActiveRecord::Base.transaction do
+        @partner.save!
+
+        Partners::Profile.create!({
+                                    partner_id: @partner.id,
+                                    name: @partner.name,
+                                    enable_child_based_requests: organization.enable_child_based_requests,
+                                    enable_individual_requests: organization.enable_individual_requests,
+                                    enable_quantity_based_requests: organization.enable_quantity_based_requests
+                                  })
+      rescue StandardError => e
+        errors.add(:base, e.message)
+        raise ActiveRecord::Rollback
+      end
+    else
       @partner.errors.each do |error|
         errors.add(error.attribute, error.message)
       end
     end
 
-    ActiveRecord::Base.transaction do
-      @partner.save!
-
-      Partners::Profile.create!({
-                                  partner_id: @partner.id,
-                                  name: @partner.name,
-                                  enable_child_based_requests: organization.enable_child_based_requests,
-                                  enable_individual_requests: organization.enable_individual_requests,
-                                  enable_quantity_based_requests: organization.enable_quantity_based_requests
-                                })
-    rescue StandardError => e
-      errors.add(:base, e.message)
-      raise ActiveRecord::Rollback
-    end
-
     self
   end
 

app/services/reports/period_supply_report_service.rb

@@ -14,8 +14,7 @@ def initialize(year:, organization:)
     def report
       @report ||= {name: "Period Supplies",
                    entries: {
-                     "Period supplies distributed" => number_with_delimiter(distributed_supplies),
-                     "Period supplies per adult per month" => monthly_supplies&.round || 0,
+                     "Period supplies distributed" => number_with_delimiter(total_distributed_period_supplies),
                      "Period supplies" => types_of_supplies,
                      "% period supplies donated" => "#{percent_donated.round}%",
                      "% period supplies bought" => "#{percent_bought.round}%",
@@ -24,7 +23,7 @@ def report
     end
 
     # @return [Integer]
-    def distributed_supplies
+    def distributed_loose_period_supplies
       @distributed_supplies ||= organization
         .distributions
         .for_year(year)
@@ -33,17 +32,12 @@ def distributed_supplies
         .sum("line_items.quantity")
     end
 
-    # @return [Integer]
-    def monthly_supplies
-      # NOTE: This is asking "per adult per month" but there doesn't seem to be much difference
-      # in calculating per month or per any other time frame, since all it's really asking
-      # is the value of the `distribution_quantity` field for the items we're giving out.
-      organization
-        .distributions
-        .for_year(year)
-        .joins(line_items: :item)
-        .merge(Item.period_supplies)
-        .average("COALESCE(items.distribution_quantity, 50)")
+    def distributed_period_supplies_from_kits
+      kit_items_calculation("distributions", "Distribution")
+    end
+
+    def total_distributed_period_supplies
+      distributed_loose_period_supplies + distributed_period_supplies_from_kits
     end
 
     def types_of_supplies
@@ -54,14 +48,14 @@ def types_of_supplies
     def percent_donated
       return 0.0 if total_supplies.zero?
 
-      (donated_supplies / total_supplies.to_f) * 100
+      (total_donated_supplies / total_supplies.to_f) * 100
     end
 
     # @return [Float]
     def percent_bought
       return 0.0 if total_supplies.zero?
 
-      (purchased_supplies / total_supplies.to_f) * 100
+      (total_purchased_supplies / total_supplies.to_f) * 100
     end
 
     # @return [String]
@@ -72,24 +66,67 @@ def money_spent_on_supplies
     ###### HELPER METHODS ######
 
     # @return [Integer]
-    def purchased_supplies
+    def total_purchased_supplies
       @purchased_supplies ||= LineItem.joins(:item)
         .merge(Item.period_supplies)
         .where(itemizable: organization.purchases.for_year(year))
         .sum(:quantity)
+
+      @purchased_supplies + purchased_supplies_from_kits
+    end
+
+    def purchased_supplies_from_kits
+      kit_items_calculation("purchases", "Purchase")
     end
 
     # @return [Integer]
     def total_supplies
-      @total_supplies ||= purchased_supplies + donated_supplies
+      @total_supplies ||= total_purchased_supplies + total_donated_supplies
     end
 
     # @return [Integer]
-    def donated_supplies
-      @donated_supplies ||= LineItem.joins(:item)
+    def total_donated_supplies
+      loose_donated_supplies = LineItem.joins(:item)
         .merge(Item.period_supplies)
         .where(itemizable: organization.donations.for_year(year))
         .sum(:quantity)
+
+      loose_donated_supplies + donated_supplies_from_kits
+    end
+
+    def donated_supplies_from_kits
+      kit_items_calculation("donations", "Donation")
+    end
+
+    private
+
+    def kit_items_calculation(itemizable_type, string_itemizable_type)
+      organization_id = @organization.id
+      year = @year
+
+      # Sanitize and validate inputs
+      itemizable_type = ActiveRecord::Base.connection.quote_table_name(itemizable_type)
+      string_itemizable_type = ActiveRecord::Base.connection.quote(string_itemizable_type)
+
+      sql_query = <<-SQL
+        SELECT SUM(line_items.quantity * kit_line_items.quantity)
+        FROM #{itemizable_type} 
+        INNER JOIN line_items ON line_items.itemizable_type = #{string_itemizable_type} AND line_items.itemizable_id = #{itemizable_type}.id 
+        INNER JOIN items ON items.id = line_items.item_id 
+        INNER JOIN kits ON kits.id = items.kit_id 
+        INNER JOIN line_items AS kit_line_items ON kits.id = kit_line_items.itemizable_id
+        INNER JOIN items AS kit_items ON kit_items.id = kit_line_items.item_id
+        INNER JOIN base_items ON base_items.partner_key = kit_items.partner_key 
+        WHERE #{itemizable_type}.organization_id = ?
+          AND EXTRACT(year FROM issued_at) = ?
+          AND LOWER(base_items.category) LIKE '%menstrual supplies%'
+          AND NOT (LOWER(base_items.category) LIKE '%diaper%' OR LOWER(base_items.name) LIKE '%cloth%')
+          AND kit_line_items.itemizable_type = 'Kit';
+      SQL
+
+      sanitized_sql = ActiveRecord::Base.send(:sanitize_sql_array, [sql_query, organization_id, year])
+      result = ActiveRecord::Base.connection.execute(sanitized_sql)
+      result.first["sum"].to_i
     end
   end
 end

spec/fixtures/files/partners_with_invalid_email.csv

@@ -0,0 +1,4 @@
+name,email
+Partner 1,[email protected]
+Partner 2,this is not an email address
+Partner 3,[email protected]

spec/requests/partners_requests_spec.rb

@@ -222,6 +222,26 @@
         expect(response).to have_error "Check headers in file!"
       end
     end
+
+    context "csv file with invalid email address" do
+      let(:file) { fixture_file_upload("partners_with_invalid_email.csv", "text/csv") }
+      subject { post import_csv_partners_path, params: { file: file } }
+
+      it "invokes .import_csv" do
+        expect(model_class).to respond_to(:import_csv).with(2).arguments
+      end
+
+      it "redirects to :index" do
+        subject
+        expect(response).to be_redirect
+      end
+
+      it "presents a flash notice message displaying the import errors" do
+        subject
+        expect(response).to have_error(/The following #{model_class.name.underscore.humanize.pluralize} did not import successfully:/)
+        expect(response).to have_error(/Partner 2: Email is invalid/)
+      end
+    end
   end
 
   describe "POST #create" do

spec/services/reports/period_supply_report_service_spec.rb

@@ -7,19 +7,20 @@
   end
 
   describe "#report" do
-    it "should report zero values" do
-      expect(report.report[:entries]).to match(hash_including({
-        "% period supplies bought" => "0%",
-        "% period supplies donated" => "0%",
-        "Period supplies distributed" => "0",
-        "Period supplies per adult per month" => 0,
-        "Money spent purchasing period supplies" => "$0.00"
-      }))
-      expect(report.report[:entries]["Period supplies"].split(", "))
-        .to contain_exactly("Tampons", "Pads", "Liners (Menstrual)")
+    context "with no values" do
+      it "should report zero values" do
+        expect(report.report[:entries]).to match(hash_including({
+          "% period supplies bought" => "0%",
+          "% period supplies donated" => "0%",
+          "Period supplies distributed" => "0",
+          "Money spent purchasing period supplies" => "$0.00"
+        }))
+        expect(report.report[:entries]["Period supplies"].split(", "))
+          .to contain_exactly("Tampons", "Pads", "Liners (Menstrual)")
+      end
     end
 
-    describe "with values" do
+    context "with values" do
       before(:each) do
         Organization.seed_items(organization)
 
@@ -32,6 +33,49 @@
         # We will create data both within and outside our date range, and both period_supplies and non period_supplies.
         # Spec will ensure that only the required data is included.
 
+        # Kits
+        period_supplies_kit = create(:kit, :with_item, organization: organization)
+        another_period_supply_kit = create(:kit, :with_item, organization: organization)
+        donated_period_supply_kit = create(:kit, :with_item, organization: organization)
+        purchased_period_supply_kit = create(:kit, :with_item, organization: organization)
+        pad_and_tampon_kit = create(:kit, :with_item, organization: organization)
+
+        create(:base_item, name: "Adult Pads", partner_key: "adult pads", category: "Menstrual Supplies")
+        create(:base_item, name: "Adult Tampons", partner_key: "adult tampons", category: "Menstrual Supplies")
+
+        period_supplies_kit_item = create(:item, name: "Adult Pads", partner_key: "adult pads")
+        another_period_supplies_kit_item = create(:item, name: "Adult Tampons", partner_key: "adult tampons")
+        purchased_period_supplies_kit_item = create(:item, name: "Liners", partner_key: "adult tampons")
+
+        period_supplies_kit.line_items.first.update!(item_id: period_supplies_kit_item.id, quantity: 5)
+        another_period_supply_kit.line_items.first.update!(item_id: another_period_supplies_kit_item.id, quantity: 5)
+        donated_period_supply_kit.line_items.first.update!(item_id: another_period_supplies_kit_item.id, quantity: 5)
+        purchased_period_supply_kit.line_items.first.update!(item_id: purchased_period_supplies_kit_item.id, quantity: 5)
+
+        pad_and_tampon_kit.line_items.first.update!(item_id: period_supplies_kit_item.id, quantity: 10)
+        pad_and_tampon_kit.line_items.first.update!(item_id: another_period_supplies_kit_item.id, quantity: 10)
+
+        period_supplies_kit_distribution = create(:distribution, organization: organization, issued_at: within_time)
+        another_period_supplies_kit_distribution = create(:distribution, organization: organization, issued_at: within_time)
+        pad_and_tampon_kit_distribution = create(:distribution, organization: organization, issued_at: within_time)
+
+        kit_donation = create(:donation, product_drive: nil, issued_at: within_time, money_raised: 1000, organization: organization)
+
+        kit_purchase = create(:purchase, issued_at: within_time, organization: organization, purchased_from: "TikTok Shop", amount_spent_in_cents: 1000, amount_spent_on_period_supplies_cents: 1000, line_items: [
+          create(:line_item, :purchase, item: period_supplies_kit_item, quantity: 5),
+          create(:line_item, :purchase, item: purchased_period_supplies_kit_item, quantity: 5)
+        ])
+
+        create(:line_item, :distribution, quantity: 10, item: period_supplies_kit.item, itemizable: period_supplies_kit_distribution)
+        create(:line_item, :distribution, quantity: 10, item: another_period_supply_kit.item, itemizable: another_period_supplies_kit_distribution)
+
+        create(:line_item, :distribution, quantity: 10, item: pad_and_tampon_kit.item, itemizable: pad_and_tampon_kit_distribution)
+        create(:line_item, :distribution, quantity: 10, item: pad_and_tampon_kit.item, itemizable: pad_and_tampon_kit_distribution)
+
+        create(:line_item, :donation, quantity: 10, item: donated_period_supply_kit.item, itemizable: kit_donation)
+
+        create(:line_item, :purchase, quantity: 30, item: purchased_period_supply_kit.item, itemizable: kit_purchase)
+
         # Distributions
         distributions = create_list(:distribution, 2, issued_at: within_time, organization: organization)
         outside_distributions = create_list(:distribution, 2, issued_at: outside_time, organization: organization)
@@ -84,19 +128,32 @@
         end
       end
 
-      it "should report normal values" do
-        organization.items.period_supplies.first.update!(distribution_quantity: 20)
+      describe "with values" do
+        it "should report normal values" do
+          organization.items.period_supplies.first.update!(distribution_quantity: 20)
 
-        expect(report.report[:name]).to eq("Period Supplies")
-        expect(report.report[:entries]).to match(hash_including({
-          "% period supplies bought" => "60%",
-          "% period supplies donated" => "40%",
-          "Period supplies distributed" => "2,000",
-          "Period supplies per adult per month" => 20,
-          "Money spent purchasing period supplies" => "$30.00"
-        }))
-        expect(report.report[:entries]["Period supplies"].split(", "))
-          .to contain_exactly("Tampons", "Pads", "Liners (Menstrual)")
+          expect(report.report[:name]).to eq("Period Supplies")
+          expect(report.report[:entries]).to match(hash_including({
+            "% period supplies bought" => "67%",
+            "% period supplies donated" => "33%",
+            "Period supplies distributed" => "2,300",
+            "Money spent purchasing period supplies" => "$40.00"
+          }))
+          expect(report.report[:entries]["Period supplies"].split(", "))
+            .to contain_exactly("Adult Pads", "Adult Tampons", "Liners", "Liners (Menstrual)", "Pads", "Tampons")
+        end
+
+        it "returns the correct quantity of period supplies from kits" do
+          expect(report.distributed_period_supplies_from_kits).to eq(300)
+        end
+
+        it "returns the correct quantity of donated period supplies from kits" do
+          expect(report.donated_supplies_from_kits).to eq(50)
+        end
+
+        it "returns the correct quantity of purchased items in kits" do
+          expect(report.purchased_supplies_from_kits).to eq(150)
+        end
       end
     end
   end