From 478cf621e7d461b544486dd54d01139eeee6a8fc Mon Sep 17 00:00:00 2001 From: Rob Barreca Date: Fri, 27 Jul 2018 00:10:36 -1000 Subject: [PATCH] Add guard so seller_order_updated and other mailer won't send to empty recipients list. - Filter out unconfirmed users in more mailers - Add and refactor more specs --- app/mailers/order_mailer.rb | 44 +++--- spec/mailers/order_mailer_spec.rb | 219 +++++++++++++++++------------- 2 files changed, 145 insertions(+), 118 deletions(-) diff --git a/app/mailers/order_mailer.rb b/app/mailers/order_mailer.rb index 33daf7a003..b99c70975f 100644 --- a/app/mailers/order_mailer.rb +++ b/app/mailers/order_mailer.rb @@ -5,7 +5,7 @@ def buyer_confirmation(order) @order = BuyerOrder.new(order) - to_list = recipient_list(order) + to_list = recipient_list(order.organization) if !to_list.blank? mail( @@ -22,11 +22,7 @@ def seller_confirmation(order, seller) @order = SellerOrder.new(order, seller) # Selling users organizations only see @seller = seller - to_list = seller. - users. - confirmed. - map { |u| u.enabled_for_organization?(seller) && !u.pretty_email.nil? ? u.pretty_email : nil}. - compact + to_list = recipient_list(seller) if !to_list.blank? mail( @@ -49,14 +45,14 @@ def market_manager_confirmation(order) end def invoice(order_id) - @order = BuyerOrder.new(Order.find(order_id)) @market = @order.market return if @market.is_consignment_market? attachments["invoice.pdf"] = {mime_type: "application/pdf", content: @order.invoice_pdf.try(:data)} - to_list = recipient_list(@order) + to_list = recipient_list(@order.organization) + if !to_list.blank? mail( to: to_list, @@ -72,7 +68,7 @@ def buyer_order_updated(order) @order = BuyerOrder.new(order) # Market Managers should see all items mail( - to: recipient_list(order), + to: recipient_list(order.organization), subject: "#{@market.name}: Order #{order.order_number} Updated", template_name: "order_updated" ) @@ -85,7 +81,7 @@ def buyer_order_removed(order) @order = BuyerOrder.new(order) # Market Managers should see all items mail( - to: recipient_list(order), + to: recipient_list(order.organization), subject: "#{@market.name}: Order #{order.order_number} Updated - Item Removed", template_name: "order_updated" ) @@ -98,16 +94,15 @@ def seller_order_updated(order, seller) @order = SellerOrder.new(order, seller) # Selling users organizations only see @seller = seller - to_list = seller. - users. - map { |u| u.enabled_for_organization?(seller) ? u.pretty_email : nil}. - compact + to_list = recipient_list(seller) - mail( - to: to_list, - subject: "#{@market.name}: Order #{order.order_number} Updated", - template_name: "order_updated" - ) + if !to_list.blank? + mail( + to: to_list, + subject: "#{@market.name}: Order #{order.order_number} Updated", + template_name: "order_updated" + ) + end end def market_manager_order_updated(order) @@ -135,8 +130,10 @@ def seller_order_item_removal(order, seller, pdf, csv) attachments["packing_list.csv"] = {mime_type: "application/csv", content: csv} end + to_list = recipient_list(seller) + mail( - to: seller.users.map(&:pretty_email), + to: to_list, subject: "#{@market.name}: Order #{order.order_number} Updated - Item Removed", template_name: "order_updated" ) @@ -144,12 +141,11 @@ def seller_order_item_removal(order, seller, pdf, csv) private - def recipient_list(order) - order. - organization. + def recipient_list(organization) + organization. users. confirmed. - map { |u| u.enabled_for_organization?(order.organization) ? u.pretty_email : nil}. + map { |u| u.enabled_for_organization?(organization) ? u.pretty_email : nil}. compact end end diff --git a/spec/mailers/order_mailer_spec.rb b/spec/mailers/order_mailer_spec.rb index 7aca694c77..c97fce7e29 100644 --- a/spec/mailers/order_mailer_spec.rb +++ b/spec/mailers/order_mailer_spec.rb @@ -1,103 +1,98 @@ -require "spec_helper" +require 'spec_helper' describe OrderMailer do let!(:market) { create(:market) } let!(:delivery_schedule) { create(:delivery_schedule, market: market) } let!(:delivery) { create(:delivery, delivery_schedule: delivery_schedule) } - let!(:seller1) { create(:organization, :seller, name: "Grandville Farms", can_sell: true, markets: [market]) } - let!(:seller2) { create(:organization, :seller, name: "Zeeland Farms", can_sell: true, markets: [market]) } - let!(:buyer) { create(:organization, :buyer, name: "Hudsonville Restaurant", can_sell: false, markets: [market]) } - let!(:users) { create_list(:user, 2, organizations: [seller1]) } + let!(:supplier1) { create(:organization, :seller, name: 'Grandville Farms', markets: [market]) } + let!(:supplier2) { create(:organization, :seller, name: 'Zeeland Farms', markets: [market]) } + let!(:buyer) { create(:organization, :buyer, name: 'Hudsonville Restaurant', markets: [market]) } + let!(:users) { create_list(:user, 2, organizations: [supplier1]) } let!(:buyer_user) { create(:user, :buyer, organizations: [buyer]) } - let!(:product1) { create(:product, :sellable, organization: seller1) } - let!(:product2) { create(:product, :sellable, organization: seller2) } + let!(:product1) { create(:product, :sellable, organization: supplier1) } + let!(:product2) { create(:product, :sellable, organization: supplier2) } - let!(:order) { create(:order, market: market, delivery: delivery, delivery_fees: 3, placed_by: buyer_user, organization: buyer, payment_method: "ach", total_cost: 30.0) } + let!(:order) { create(:order, market: market, delivery: delivery, delivery_fees: 3, placed_by: buyer_user, organization: buyer, payment_method: 'ach', total_cost: 30.0) } let!(:order_item1) { create(:order_item, order: order, product: product1, quantity: 11, unit_price: 2.00) } let!(:order_item2) { create(:order_item, order: order, product: product2, quantity: 4, unit_price: 2.50) } let!(:csv) { 'CSV' } - describe "seller_confirmation" do - before do - Audit.all.update_all(request_uuid: SecureRandom.uuid) - @notification = OrderMailer.seller_confirmation(order.reload, seller1) - end + describe '.seller_confirmation' do + let(:notification) { OrderMailer.seller_confirmation(order.reload, supplier1) } - it "delivers to all users in the organization" do - expect(@notification).to deliver_to(users.map(&:email)) + it 'delivers to all users in the organization' do + expect(notification).to deliver_to(users.map(&:email)) end - it "shows the seller what order the notification relates to" do - expect(@notification).to have_body_text("Order Number: #{order.order_number}") + it 'shows the seller what order the notification relates to' do + expect(notification).to have_body_text("Order Number: #{order.order_number}") end - it "shows what market the order is from" do - expect(@notification).to have_subject("New order on #{market.name}") + it 'shows what market the order is from' do + expect(notification).to have_subject("New order on #{market.name}") end - it "shows what buyer made the order" do - expect(@notification).to have_body_text( + it 'shows what buyer made the order' do + expect(notification).to have_body_text( "An order was just placed by #{buyer.name}" ) end - it "shows how the seller should view the order details" do - expect(@notification).to have_body_text("following the link below and logging in to your #{seller1.name} account") + it 'shows how the seller should view the order details' do + expect(notification).to have_body_text("following the link below and logging in to your #{supplier1.name} account") end - it "does not show a previous quantity for an item" do - within(".previous-value") do - expect(@notification).to_not have_body_text("11 per box") + it 'does not show a previous quantity for an item' do + within('.previous-value') do + expect(notification).to_not have_body_text('11 per box') end end - it "does not show delivery fee" do - expect(@notification).not_to have_body_text("Delivery Fee") + it 'does not show delivery fee' do + expect(notification).not_to have_body_text('Delivery Fee') end end - describe "buyer_confirmation" do - before do - Audit.all.update_all(request_uuid: SecureRandom.uuid) - @notification = OrderMailer.buyer_confirmation(order.reload) - end - it "shows the delivery fee" do - expect(@notification).to have_body_text("Delivery Fee") + describe '.buyer_confirmation' do + let(:notification) { OrderMailer.buyer_confirmation(order) } + + it 'shows the delivery fee' do + expect(notification).to have_body_text('Delivery Fee') end end - describe "buyer_order_updated" do - context "quantities changed" do + describe '.buyer_order_updated' do + context 'quantities changed' do before do Order.enable_auditing OrderItem.enable_auditing - order.reload.update(updated_at: Time.current, items_attributes: {"0" => {id: order_item1.id, quantity: 15}}) + order.reload.update(updated_at: Time.current, items_attributes: {'0' => {id: order_item1.id, quantity: 15}}) OrderItem.disable_auditing Order.disable_auditing Audit.all.update_all(request_uuid: SecureRandom.uuid) @notification = OrderMailer.buyer_order_updated(order.reload) end - it "has a subject indicating it is an update" do + it 'has a subject indicating it is an update' do expect(@notification).to have_subject("#{market.name}: Order #{order.order_number} Updated") end - it "shows the old order quantity" do - expect(@notification).to have_body_text("11 per box") + it 'shows the old order quantity' do + expect(@notification).to have_body_text('11 per box') end - it "shows the updated order quantity" do - expect(@notification).to have_body_text("15 per box") + it 'shows the updated order quantity' do + expect(@notification).to have_body_text('15 per box') end end - context "canceled items" do + context 'canceled items' do before do Order.enable_auditing OrderItem.enable_auditing - order.reload.update(updated_at: Time.current, items_attributes: {"0" => {id: order_item1.id, quantity: 0, delivery_status: "canceled"}}) + order.reload.update(updated_at: Time.current, items_attributes: {'0' => {id: order_item1.id, quantity: 0, delivery_status: 'canceled'}}) OrderItem.disable_auditing Order.disable_auditing @@ -105,28 +100,28 @@ @notification = OrderMailer.buyer_order_updated(order.reload) end - it "has a subject indicating it is an update" do + it 'has a subject indicating it is an update' do expect(@notification).to have_subject("#{market.name}: Order #{order.order_number} Updated") end - it "shows canceled items quantity as 0" do - expect(@notification).to have_body_text("0 per box") + it 'shows canceled items quantity as 0' do + expect(@notification).to have_body_text('0 per box') end - it "does not show the canceled items previous quantity" do - expect(@notification).to_not have_body_text("11 per box") + it 'does not show the canceled items previous quantity' do + expect(@notification).to_not have_body_text('11 per box') end - it "shows the item as being canceled" do - expect(@notification).to have_body_text("canceled") + it 'shows the item as being canceled' do + expect(@notification).to have_body_text('canceled') end end - context "refund amount" do + context 'refund amount' do before do Order.enable_auditing OrderItem.enable_auditing - order.reload.update(updated_at: Time.current, items_attributes: {"0" => {id: order_item1.id, quantity: 5}}) + order.reload.update(updated_at: Time.current, items_attributes: {'0' => {id: order_item1.id, quantity: 5}}) OrderItem.disable_auditing Order.disable_auditing @@ -134,16 +129,16 @@ @notification = OrderMailer.buyer_order_updated(order.reload) end - it "shows the refund amount" do - expect(@notification).to have_body_text("refund of $3.00") + it 'shows the refund amount' do + expect(@notification).to have_body_text('refund of $3.00') end end - context "increasing the quantity" do + context 'increasing the quantity' do before do Order.enable_auditing OrderItem.enable_auditing - order.reload.update(updated_at: Time.current, items_attributes: {"0" => {id: order_item1.id, quantity: 15}}) + order.reload.update(updated_at: Time.current, items_attributes: {'0' => {id: order_item1.id, quantity: 15}}) OrderItem.disable_auditing Order.disable_auditing @@ -151,107 +146,143 @@ @notification = OrderMailer.buyer_order_updated(order.reload) end - it "does not show the refund section" do - expect(@notification).to_not have_body_text("refund") + it 'does not show the refund section' do + expect(@notification).to_not have_body_text('refund') end end end - describe "seller_order_updated" do - context "quantities changed" do + describe '.seller_order_updated' do + let(:notification) { OrderMailer.seller_order_updated(order, supplier_org) } + + context 'quantities changed' do + let(:supplier_org) { supplier1 } + before do Order.enable_auditing OrderItem.enable_auditing - order.reload.update(updated_at: Time.current, items_attributes: {"0" => {id: order_item1.id, quantity: 15}}) + order.reload.update(updated_at: Time.current, items_attributes: {'0' => {id: order_item1.id, quantity: 15}}) OrderItem.disable_auditing Order.disable_auditing Audit.all.update_all(request_uuid: SecureRandom.uuid) - @notification = OrderMailer.seller_order_updated(order.reload, seller1) + @notification = OrderMailer.seller_order_updated(order.reload, supplier1) end - it "has a subject indicating it is an update" do + it 'has a subject indicating it is an update' do expect(@notification).to have_subject("#{market.name}: Order #{order.order_number} Updated") end - it "shows the old order quantity" do - expect(@notification).to have_body_text("11 per box") + it 'shows the old order quantity' do + expect(@notification).to have_body_text('11 per box') end - it "shows the updated order quantity" do - expect(@notification).to have_body_text("15 per box") + it 'shows the updated order quantity' do + expect(@notification).to have_body_text('15 per box') end - it "does not show other seller items" do + it 'does not show other seller items' do expect(@notification).to_not have_body_text(product2.name) end end - context "canceled items" do + context 'canceled items' do + let(:supplier_org) { supplier1 } + before do Order.enable_auditing OrderItem.enable_auditing - order.reload.update(updated_at: Time.current, items_attributes: {"0" => {id: order_item1.id, quantity: 0, delivery_status: "canceled"}}) + order.reload.update(updated_at: Time.current, items_attributes: {'0' => {id: order_item1.id, quantity: 0, delivery_status: 'canceled'}}) OrderItem.disable_auditing Order.disable_auditing Audit.all.update_all(request_uuid: SecureRandom.uuid) - @notification = OrderMailer.seller_order_updated(order.reload, seller1) + @notification = OrderMailer.seller_order_updated(order.reload, supplier1) end - it "has a subject indicating it is an update" do + it 'has a subject indicating it is an update' do expect(@notification).to have_subject("#{market.name}: Order #{order.order_number} Updated") end - it "shows canceled items quantity as 0" do - expect(@notification).to have_body_text("0 per box") + it 'shows canceled items quantity as 0' do + expect(@notification).to have_body_text('0 per box') end - it "shows the item as being canceled" do - expect(@notification).to have_body_text("canceled") + it 'shows the item as being canceled' do + expect(@notification).to have_body_text('canceled') end - it "does not show the canceled items previous quantity" do - expect(@notification).to_not have_body_text("10 per box") + it 'does not show the canceled items previous quantity' do + expect(@notification).to_not have_body_text('10 per box') end - it "does not show other seller items" do + it 'does not show other seller items' do expect(@notification).to_not have_body_text(product2.name) end end - context "refund amount" do + xcontext 'refund amount' do + let(:supplier_org) { supplier1 } + before do Order.enable_auditing OrderItem.enable_auditing - order.reload.update(updated_at: Time.current, items_attributes: {"0" => {id: order_item1.id, quantity: 5}}) + order.reload.update(updated_at: Time.current, items_attributes: {'0' => {id: order_item1.id, quantity: 5}}) OrderItem.disable_auditing Order.disable_auditing - pdf = PdfResult.new(data: "data", path: "/") + pdf = PdfResult.new(data: 'data', path: '/') Audit.all.update_all(request_uuid: SecureRandom.uuid) - @notification = OrderMailer.seller_order_updated(order.reload, seller1, pdf, csv) + @notification = OrderMailer.seller_order_updated(order.reload, supplier1) end - #it "shows the refund amount" do - # expect(@notification).to have_body_text("refund of $3.00") - #end + it 'shows the refund amount' do + expect(@notification).to have_body_text('refund of $3.00') + end end - context "increasing the quantity" do + context 'increasing the quantity' do before do Order.enable_auditing OrderItem.enable_auditing - order.reload.update(updated_at: Time.current, items_attributes: {"0" => {id: order_item1.id, quantity: 15}}) + order.reload.update(updated_at: Time.current, items_attributes: {'0' => {id: order_item1.id, quantity: 15}}) OrderItem.disable_auditing Order.disable_auditing Audit.all.update_all(request_uuid: SecureRandom.uuid) - @notification = OrderMailer.seller_order_updated(order.reload, seller1) + @notification = OrderMailer.seller_order_updated(order.reload, supplier1) + end + + it 'does not show the refund section' do + expect(@notification).to_not have_body_text('refund') end + end + + context 'when supplier org has no users' do + let(:supplier_org) { create(:organization, :seller, markets: [market]) } + + it 'no email is sent' do + expect(notification).to be_kind_of(ActionMailer::Base::NullMail) + end + end + + context 'when supplier org has only unconfirmed users' do + let(:supplier_org) { create(:organization, :seller, markets: [market]) } + let!(:unconfirmed_users) { create_list(:user, 2, confirmed_at: nil, organizations: [supplier_org]) } + + it 'no email is sent' do + expect(notification).to be_kind_of(ActionMailer::Base::NullMail) + end + end + + context 'when supplier org has both confirmed and unconfirmed users' do + let(:supplier_org) { create(:organization, :seller, markets: [market]) } + let!(:confirmed_user1) { create(:user, confirmed_at: Time.now, organizations: [supplier_org]) } + let!(:confirmed_user2) { create(:user, confirmed_at: Time.now, organizations: [supplier_org]) } + let!(:unconfirmed_user1) { create(:user, confirmed_at: nil, organizations: [supplier_org]) } - it "does not show the refund section" do - expect(@notification).to_not have_body_text("refund") + it 'email is sent to only confirmed users' do + expect(notification).to be_delivered_to([confirmed_user1.email, confirmed_user2.email]) end end end @@ -260,7 +291,7 @@ let(:notification) { OrderMailer.invoice(order.id) } it 'delivers to all users in the buyer organization' do - expect(notification).to deliver_to(buyer_user.email) + expect(notification).to be_delivered_to(buyer_user.email) end end end