Add "Pick up" vs "Delivery" for Distributions (#1818)

daynew · edwinthinks · web-flow · commit 8f1fc0a74037 · 2020-09-19T17:12:24.000-05:00
* Adding 'Delivery method' for Distributions

* Updating database seed with new 'delivery_method' field

* Updating the delivery_method of a distribution will trigger a notification

* Updated notification messages to indicate 'delivery_method' change

* Updated 'partner_mailer' to specify delivery or pickup

* Updated reminder_email to specify 'pick up' or 'delivery'

* Changing 'pick up' usage to be more general

* Fixing the distribution changed email and various unit tests

* Make the Delivery Method a radio select

* Adding default 'delivery_method' to be pick_up in order to backfill existing Distributions

* Making helper methods private

* Fixing unit tests to reflect UI changes for delivery_method

* Update app/controllers/distributions_controller.rb

* Update spec/requests/distributions_requests_spec.rb

Co-authored-by: Edwin Mak &lt;edwinthinks@gmail.com&gt;
diff --git a/app/controllers/distributions_controller.rb b/app/controllers/distributions_controller.rb
@@ -109,7 +109,7 @@ def update
 
     if result.success?
       if result.resend_notification? && @distribution.partner&.send_reminders
-        send_notification(current_organization.id, @distribution.id, subject: "Your Distribution New Schedule Date is #{@distribution.issued_at}")
+        send_notification(current_organization.id, @distribution.id, subject: "Your Distribution Date Has Changed")
       end
       schedule_reminder_email(@distribution)
 
@@ -133,9 +133,9 @@ def picked_up
     distribution = current_organization.distributions.find(params[:id])
 
     if !distribution.complete? && distribution.complete!
-      flash[:notice] = 'This distribution has been marked as being picked up!'
+      flash[:notice] = 'This distribution has been marked as being completed!'
     else
-      flash[:error] = 'Sorry, we encountered an error when trying to mark this distribution as being picked up'
+      flash[:error] = 'Sorry, we encountered an error when trying to mark this distribution as being completed'
     end
 
     redirect_back(fallback_location: distribution_path)
@@ -164,7 +164,7 @@ def schedule_reminder_email(distribution)
   end
 
   def distribution_params
-    params.require(:distribution).permit(:comment, :agency_rep, :issued_at, :partner_id, :storage_location_id, :reminder_email_enabled, line_items_attributes: %i(item_id quantity _destroy))
+    params.require(:distribution).permit(:comment, :agency_rep, :issued_at, :partner_id, :storage_location_id, :reminder_email_enabled, :delivery_method, line_items_attributes: %i(item_id quantity _destroy))
   end
 
   def request_id
diff --git a/app/models/distribution.rb b/app/models/distribution.rb
@@ -6,6 +6,7 @@
 #  id                     :integer          not null, primary key
 #  agency_rep             :string
 #  comment                :text
+#  delivery_method        :integer          default("pick_up"), not null
 #  issued_at              :datetime
 #  reminder_email_enabled :boolean          default(FALSE), not null
 #  state                  :integer          default("started"), not null
@@ -31,7 +32,7 @@ class Distribution < ApplicationRecord
   has_one :request, dependent: :nullify
   accepts_nested_attributes_for :request
 
-  validates :storage_location, :partner, :organization, presence: true
+  validates :storage_location, :partner, :organization, :delivery_method, presence: true
   validate :line_item_items_exist_in_inventory
 
   include IssuedAt
@@ -40,6 +41,8 @@ class Distribution < ApplicationRecord
 
   enum state: { started: 0, scheduled: 5, complete: 10 }
 
+  enum delivery_method: { pick_up: 0, delivery: 1 }
+
   include Filterable
   # add item_id scope to allow filtering distributions by item
   scope :by_item_id, ->(item_id) { joins(:items).where(items: { id: item_id }) }
diff --git a/app/services/distribution_update_service.rb b/app/services/distribution_update_service.rb
@@ -8,6 +8,7 @@ def initialize(old_distribution, new_distribution_params)
   def call
     perform_distribution_service do
       @old_issued_at = distribution.issued_at
+      @old_delivery_method = distribution.delivery_method
       distribution.storage_location.increase_inventory(distribution.to_a)
       # Delete the line items -- they'll be replaced later
       distribution.line_items.each(&:destroy!)
@@ -16,12 +17,23 @@ def call
       distribution.update! @params
       distribution.reload
       @new_issued_at = distribution.issued_at
+      @new_delivery_method = distribution.delivery_method
       # Apply the new changes to the storage location inventory
       distribution.storage_location.decrease_inventory(distribution.to_a)
     end
   end
 
   def resend_notification?
+    issued_at_changed? || delivery_method_changed?
+  end
+
+  private
+
+  def issued_at_changed?
     @old_issued_at.to_date != @new_issued_at.to_date
   end
+
+  def delivery_method_changed?
+    @old_delivery_method != @new_delivery_method
+  end
 end
diff --git a/app/views/distribution_mailer/partner_mailer.html.erb b/app/views/distribution_mailer/partner_mailer.html.erb
@@ -341,9 +341,10 @@
               <table role="presentation" border="0" cellpadding="0" cellspacing="0">
                 <tr>
                   <td>
+                    <% delivery_method = (@distribution.delivery?) ? 'delivered' : 'picked up' %>
                     <p><%= @partner.name %>,</p>
                     <p>Your diaper request has been approved and you can find attached to this email a copy of the distribution you will be receiving.</p>
-                    <p>Your distribution has been set to be picked up on <%= @distribution.issued_at.strftime("%m/%d/%Y") %>.</p>
+                    <p>Your distribution has been set to be <b><%= delivery_method %></b> on <b><%= @distribution.issued_at.strftime("%m/%d/%Y") %></b>.</p>
                     <p>See you soon!</p>
                   </td>
                 <tr>
diff --git a/app/views/distribution_mailer/reminder_email.html.erb b/app/views/distribution_mailer/reminder_email.html.erb
@@ -4,7 +4,8 @@
     <meta content='text/html; charset=UTF-8' http-equiv='Content-Type' />
   </head>
   <body>
+    <% delivery_method = (@distribution.delivery?) ? "delivery" : "pick up"%>
     <p>Hello <%= @partner.name %>,</p>
-    <p>This is a friendly reminder that tomorrow, <%= @distribution.issued_at.strftime("%A, %B #{@distribution.issued_at.day.ordinalize} %Y at %I:%M%p") %>, is your distribution pick up date. </p>
+    <p>This is a friendly reminder that tomorrow, <b><%= @distribution.issued_at.strftime("%A, %B #{@distribution.issued_at.day.ordinalize} %Y at %I:%M%p") %></b>, is your distribution <b><%= delivery_method %></b> date. </p>
   </body>
 </html>
diff --git a/app/views/distributions/_distribution_row.html.erb b/app/views/distributions/_distribution_row.html.erb
@@ -4,7 +4,8 @@
   <td><%= distribution_row.storage_location.name %></td>
   <td class="numeric"><%= distribution_row.line_items.total %></td>
   <td class="numeric"><%= dollar_value(distribution_row.value_per_itemizable) %></td>
-  <td><%= distribution_row.state&.humanize %>
+  <td><%= distribution_row.delivery_method.humanize %></td>
+  <td><%= distribution_row.state&.humanize %></td>
 
   <td class="text-right">
     <%= view_button_to distribution_path(distribution_row) %>
diff --git a/app/views/distributions/_pickup_day_row.html.erb b/app/views/distributions/_pickup_day_row.html.erb
@@ -5,13 +5,13 @@
   <td><%= pickup_day_row.line_items.total %></td>
   <td>
     <% if pickup_day_row.complete? %>
-      Picked up
+      Complete
     <% else %>
       Scheduled
     <% end %>
   </td>
   <td class="text-right">
-    <%= update_button_to picked_up_distribution_path(pickup_day_row), {text: "Pick up Complete", size: "xs"} if !pickup_day_row.complete? %>
+    <%= update_button_to picked_up_distribution_path(pickup_day_row), {text: "Distribution Complete", size: "xs"} if !pickup_day_row.complete? %>
     <%= view_button_to distribution_path(pickup_day_row) %>
   </td>
 </tr>
diff --git a/app/views/distributions/edit.html.erb b/app/views/distributions/edit.html.erb
@@ -28,7 +28,7 @@
 
   <% unless @distribution.future? %>
     <div class="alert alert-warning" , role="alert">
-      The current date is past the date this distribution was picked up.
+      The current date is past the date this distribution was scheduled for.
       Please be very careful when editing this record;
       the contents should match what was given to the recipient.
     </div>
@@ -60,6 +60,7 @@
                   <%= f.input :issued_at, as: :datetime, ampm: true, minute_step: 15, label: "Distribution date" %>
                   <%= f.input :reminder_email_enabled, as: :boolean, checked_value: true, unchecked_value: false, label: "Send email reminder the day before?" %>
                   <%= f.input :agency_rep, label: "Agency representative" %>
+                  <%= f.input :delivery_method, as: :radio_buttons, collection: Distribution.delivery_methods.keys, label_method: :humanize, label: "Delivery method" %>
 
                   <%= render partial: "storage_locations/source", object: f %>
 
diff --git a/app/views/distributions/index.html.erb b/app/views/distributions/index.html.erb
@@ -84,6 +84,7 @@
                 <th>Source Inventory</th>
                 <th class="numeric">Total items</th>
                 <th class="numeric">Total value</th>
+                <th>Delivery method</th>
                 <th>State</th>
                 <th class="text-right" style="width: 400px">Actions</th>
               </tr>
diff --git a/app/views/distributions/new.html.erb b/app/views/distributions/new.html.erb
@@ -50,6 +50,7 @@
                   <%= f.input :issued_at, as: :datetime, ampm: true, minute_step: 15, label: "Distribution date" %>
                   <%= f.input :reminder_email_enabled, as: :boolean, checked_value: true, unchecked_value: false, label: "Send email reminder the day before?" %>
                   <%= f.input :agency_rep, label: "Agency representative" %>
+                  <%= f.input :delivery_method, as: :radio_buttons, collection: Distribution.delivery_methods.keys, label_method: :humanize, label: "Delivery method" %>
 
                   <%= render partial: "storage_locations/source", object: f %>
 
diff --git a/app/views/distributions/pick_ups.html.erb b/app/views/distributions/pick_ups.html.erb
@@ -4,9 +4,9 @@
     <div class="container-fluid">
       <div class="row mb-2">
         <div class="col-sm-7">
-          <% content_for :title, "Distribution Pick-Ups - #{current_organization.name}" %>
+          <% content_for :title, "Distribution Schedule - #{current_organization.name}" %>
           <h1>
-            Pick-Up Schedule
+            Distribution Schedule
             <small>for <%= current_organization.name %></small>
           </h1>
         </div>
diff --git a/app/views/distributions/pickup_day.html.erb b/app/views/distributions/pickup_day.html.erb
@@ -2,9 +2,9 @@
   <div class="container-fluid">
     <div class="row mb-2">
       <div class="col-sm-6">
-        <% content_for :title, "Pick up Schedule for #{@selected_date.strftime("%B %e, %Y")}" %>
+        <% content_for :title, "Distribution Schedule for #{@selected_date.strftime("%B %e, %Y")}" %>
         <h1>
-          Pick up Schedule for <%= @selected_date.strftime("%B %e, %Y") %>
+          Distribution Schedule for <%= @selected_date.strftime("%B %e, %Y") %>
         </h1>
       </div>
       <div class="col-sm-6">
diff --git a/app/views/distributions/show.html.erb b/app/views/distributions/show.html.erb
@@ -37,13 +37,15 @@
               <tr>
                 <th>Source location:</th>
                 <th>Agency representative:</th>
+                <th>Delivery method:</th>
                 <th>State:</th>
               </tr>
               </thead>
               <tbody>
               <tr>
                 <td><%= @distribution.storage_location.name %></td>
                 <td><%= @distribution.agency_rep %></td>
+                <td><%= @distribution.delivery_method.humanize %></td>
                 <td><%= @distribution.state&.humanize %></td>
               </tr>
             </table>
@@ -75,7 +77,7 @@
             </table>
           </div>
           <div class="card-footer">
-            <%= update_button_to picked_up_distribution_path(@distribution), {text: "Pick up Complete", size: "md"} if @distribution.scheduled? %>
+            <%= update_button_to picked_up_distribution_path(@distribution), {text: "Distribution Complete", size: "md"} if @distribution.scheduled? %>
             <% if @distribution.future? || current_user.organization_admin? %>
               <%= edit_button_to edit_distribution_path(@distribution), {text: "Make a Correction", size: "md"} %>
             <% end %>
diff --git a/app/views/layouts/_lte_sidebar.html.erb b/app/views/layouts/_lte_sidebar.html.erb
@@ -79,7 +79,7 @@
 
   <li class="nav-item <%= 'active' if current_page?(pick_ups_distributions_path) %>">
     <%= link_to(pick_ups_distributions_path, class: "nav-link #{"active" if current_page?(pick_ups_distributions_path)}") do %>
-      <i class="nav-icon fa fa-calendar-o"></i> <p>Pick Ups</p>
+      <i class="nav-icon fa fa-calendar-o"></i> <p>Pick Ups & Deliveries</p>
     <% end %>
   </li>
 
diff --git a/config/initializers/simple_form_bootstrap.rb b/config/initializers/simple_form_bootstrap.rb
@@ -77,7 +77,7 @@
   config.wrappers :vertical_collection, item_wrapper_class: 'form-check', item_label_class: 'form-check-label', tag: 'fieldset', class: 'form-group', error_class: 'form-group-invalid', valid_class: 'form-group-valid' do |b|
     b.use :html5
     b.optional :readonly
-    b.wrapper :legend_tag, tag: 'legend', class: 'col-form-label pt-0' do |ba|
+    b.wrapper :legend_tag, tag: 'legend', class: 'col-form-label pt-0 font-weight-bold' do |ba|
       ba.use :label_text
     end
     b.use :input, class: 'form-check-input', error_class: 'is-invalid', valid_class: 'is-valid'
@@ -89,7 +89,7 @@
   config.wrappers :vertical_collection_inline, item_wrapper_class: 'form-check form-check-inline', item_label_class: 'form-check-label', tag: 'fieldset', class: 'form-group', error_class: 'form-group-invalid', valid_class: 'form-group-valid' do |b|
     b.use :html5
     b.optional :readonly
-    b.wrapper :legend_tag, tag: 'legend', class: 'col-form-label pt-0' do |ba|
+    b.wrapper :legend_tag, tag: 'legend', class: 'col-form-label pt-0 font-weight-bold' do |ba|
       ba.use :label_text
     end
     b.use :input, class: 'form-check-input', error_class: 'is-invalid', valid_class: 'is-valid'
diff --git a/db/migrate/20200906185544_add_delivery_method_to_distribution.rb b/db/migrate/20200906185544_add_delivery_method_to_distribution.rb
@@ -0,0 +1,6 @@
+class AddDeliveryMethodToDistribution < ActiveRecord::Migration[6.0]
+  def change
+    # Default to pick_up
+    add_column :distributions, :delivery_method, :integer, default: 0, null: false
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2020_09_06_155253) do
+ActiveRecord::Schema.define(version: 2020_09_06_185544) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -131,6 +131,7 @@
     t.string "agency_rep"
     t.boolean "reminder_email_enabled", default: false, null: false
     t.integer "state", default: 0, null: false
+    t.integer "delivery_method", default: 0, null: false
     t.index ["organization_id"], name: "index_distributions_on_organization_id"
     t.index ["partner_id"], name: "index_distributions_on_partner_id"
     t.index ["storage_location_id"], name: "index_distributions_on_storage_location_id"
diff --git a/db/seeds.rb b/db/seeds.rb
@@ -314,7 +314,8 @@ def seed_quantity(item_name, organization, storage_location, quantity)
   distribution = Distribution.create!(storage_location: storage_location,
                                       partner:          random_record_for_org(pdx_org, Partner),
                                       organization:     pdx_org,
-                                      issued_at:        Faker::Date.between(from: 4.days.ago, to: Time.zone.today))
+                                      issued_at:        Faker::Date.between(from: 4.days.ago, to: Time.zone.today),
+                                      delivery_method:  Distribution.delivery_methods.keys.sample)
 
   stored_inventory_items_sample.each do |stored_inventory_item|
     distribution_qty = rand(stored_inventory_item.quantity / 2)
diff --git a/spec/factories/distributions.rb b/spec/factories/distributions.rb
@@ -5,6 +5,7 @@
 #  id                     :integer          not null, primary key
 #  agency_rep             :string
 #  comment                :text
+#  delivery_method        :integer          default("pick_up"), not null
 #  issued_at              :datetime
 #  reminder_email_enabled :boolean          default(FALSE), not null
 #  state                  :integer          default("started"), not null
@@ -21,6 +22,7 @@
     partner
     organization { Organization.try(:first) || create(:organization) }
     issued_at { nil }
+    delivery_method { :pick_up }
     state { :started }
 
     trait :with_items do
diff --git a/spec/mailers/distribution_mailer_spec.rb b/spec/mailers/distribution_mailer_spec.rb
@@ -1,8 +1,8 @@
 RSpec.describe DistributionMailer, type: :mailer do
   before do
     @organization.default_email_text = "Default email text example"
-    partner = create(:partner, name: 'PARTNER')
-    @distribution = create(:distribution, organization: @user.organization, comment: "Distribution comment", partner: partner)
+    @partner = create(:partner, name: 'PARTNER')
+    @distribution = create(:distribution, organization: @user.organization, comment: "Distribution comment", partner: @partner)
   end
 
   describe "#partner_mailer" do
@@ -17,6 +17,22 @@
       expect(mail.body.encoded).to match("Distribution comment")
       expect(mail.subject).to eq("test subject from DEFAULT")
     end
+
+    context "with deliver_method: :pick_up" do
+      it "renders the body with 'picked up' specified" do
+        distribution = create(:distribution, organization: @user.organization, comment: "Distribution comment", partner: @partner, delivery_method: :pick_up)
+        mail = DistributionMailer.partner_mailer(@organization, distribution, 'test subject')
+        expect(mail.body.encoded).to match("picked up")
+      end
+    end
+
+    context "with deliver_method: :delivery" do
+      it "renders the body with 'delivered' specified" do
+        distribution = create(:distribution, organization: @user.organization, comment: "Distribution comment", partner: @partner, delivery_method: :delivery)
+        mail = DistributionMailer.partner_mailer(@organization, distribution, 'test subject')
+        expect(mail.body.encoded).to match("delivered")
+      end
+    end
   end
 
   describe "#reminder_email" do
@@ -26,5 +42,21 @@
       expect(mail.body.encoded).to match("This is a friendly reminder")
       expect(mail.subject).to eq("PARTNER Distribution Reminder")
     end
+
+    context "with deliver_method: :pick_up" do
+      it "renders the body with 'pick up' specified" do
+        distribution = create(:distribution, organization: @user.organization, comment: "Distribution comment", partner: @partner, delivery_method: :pick_up)
+        mail = DistributionMailer.reminder_email(distribution.id)
+        expect(mail.body.encoded).to match("pick up")
+      end
+    end
+
+    context "with deliver_method: :delivery" do
+      it "renders the body with 'delivery' specified" do
+        distribution = create(:distribution, organization: @user.organization, comment: "Distribution comment", partner: @partner, delivery_method: :delivery)
+        mail = DistributionMailer.reminder_email(distribution.id)
+        expect(mail.body.encoded).to match("delivery")
+      end
+    end
   end
 end
diff --git a/spec/models/distribution_spec.rb b/spec/models/distribution_spec.rb
@@ -5,6 +5,7 @@
 #  id                     :integer          not null, primary key
 #  agency_rep             :string
 #  comment                :text
+#  delivery_method        :integer          default("pick_up"), not null
 #  issued_at              :datetime
 #  reminder_email_enabled :boolean          default(FALSE), not null
 #  state                  :integer          default("started"), not null
diff --git a/spec/requests/distributions_requests_spec.rb b/spec/requests/distributions_requests_spec.rb
@@ -53,7 +53,7 @@
       let!(:storage_location) { create(:storage_location) }
       let!(:partner) { create(:partner) }
       let(:distribution) do
-        { distribution: { storage_location_id: storage_location.id, partner_id: partner.id } }
+        { distribution: { storage_location_id: storage_location.id, partner_id: partner.id, delivery_method: :delivery } }
       end
 
       it "redirects to #show on success" do
@@ -242,7 +242,7 @@
 
           it "does send the email" do
             subject
-            expect(PartnerMailerJob).to have_enqueued_sidekiq_job(distribution.organization.id, distribution.id, /Your Distribution New Schedule Date is/)
+            expect(PartnerMailerJob).to have_enqueued_sidekiq_job(distribution.organization.id, distribution.id, /Your Distribution Date Has Changed/)
           end
         end
 
diff --git a/spec/services/distribution_create_service_spec.rb b/spec/services/distribution_create_service_spec.rb
diff --git a/spec/services/distribution_update_service_spec.rb b/spec/services/distribution_update_service_spec.rb
diff --git a/spec/system/distribution_system_spec.rb b/spec/system/distribution_system_spec.rb
diff --git a/spec/system/request_system_spec.rb b/spec/system/request_system_spec.rb
