Merge pull request #94 from ruby-no-kai/create_broadcast_deliveries_j…

…ob_fix_filters

fix recipient filters

Gemfile

@@ -32,9 +32,6 @@ gem 'simpacker'
 
 gem 'premailer-rails'
 
-gem 'letter_opener_web', git: 'https://github.com/fgrehm/letter_opener_web', ref: 'ab50ad09a2af5350bdca9c079bba73523e64f4cd' # https://github.com/fgrehm/letter_opener_web/pull/83
-gem 'rspec-rails'
-
 gem 'revision_plate'
 gem "sentry-ruby"
 gem "sentry-rails"
@@ -48,6 +45,10 @@ end
 group :development, :test do
   # gem 'byebug', platforms: [:mri, :mingw, :x64_mingw]
   gem 'listen'
+
+  gem 'letter_opener_web', git: 'https://github.com/fgrehm/letter_opener_web', ref: 'ab50ad09a2af5350bdca9c079bba73523e64f4cd' # https://github.com/fgrehm/letter_opener_web/pull/83
+  gem 'rspec-rails'
+  gem 'factory_bot_rails'
 end
 
 group :development do

Gemfile.lock

@@ -124,6 +124,11 @@ GEM
     diff-lcs (1.5.1)
     drb (2.2.1)
     erubi (1.13.0)
+    factory_bot (6.5.1)
+      activesupport (>= 6.1.0)
+    factory_bot_rails (6.4.4)
+      factory_bot (~> 6.5)
+      railties (>= 5.0.0)
     faraday (1.10.4)
       faraday-em_http (~> 1.0)
       faraday-em_synchrony (~> 1.0)
@@ -370,6 +375,7 @@ DEPENDENCIES
   barnes
   commonmarker
   connection_pool
+  factory_bot_rails
   faraday
   faraday_middleware
   haml

app/jobs/create_broadcast_deliveries_job.rb

@@ -1,6 +1,125 @@
 class CreateBroadcastDeliveriesJob < ApplicationJob
   Recipient = Struct.new(:sponsorship, :email, :email_ccs, keyword_init: true)
 
+  module Filters
+    class Base
+      def initialize(broadcast:, params:)
+        @broadcast = broadcast
+        @params = params
+      end
+
+      attr_reader :broadcast, :params
+
+      def recipients
+        raise NotImplementedError
+      end
+
+      private def status_scope
+        case params[:status]
+        when 'all', nil
+          Sponsorship.all
+        when 'not_accepted'
+          Sponsorship.not_accepted
+        when 'pending'
+          Sponsorship.pending
+        when 'accepted'
+          Sponsorship.accepted
+        when 'active'
+          Sponsorship.active
+        when 'withdrawn'
+          Sponsorship.withdrawn
+        else
+          Sponsorship.none
+        end
+      end
+
+      private def plan_scope
+        if params[:plan_id].present?
+          plan = @broadcast.conference.plans.where(id: params[:plan_id]).first
+          if plan
+            return Sponsorship.where(plan:)
+          end
+        end
+        nil
+      end
+
+      private def locale_scope
+        if params[:locale].present?
+          Sponsorship.where(locale: params[:locale]) 
+        else
+          nil
+        end
+      end
+
+      private def exhibitor_scope
+        if params[:exhibitors].present?
+          Sponsorship.exhibitor 
+        else
+          nil
+        end
+      end
+
+      private def scope_sponsorships(scope)
+        [
+          status_scope,
+          plan_scope,
+          locale_scope,
+          exhibitor_scope,
+        ].inject(scope) do |r,i|
+          i ? r.merge(i) : r
+        end
+      end
+
+      private def sponsorships_to_recipients(scope)
+        scope.map do |sponsorship|
+          Recipient.new(
+            sponsorship: sponsorship,
+            email: sponsorship.contact.email,
+            email_ccs: sponsorship.contact.email_ccs,
+          )
+        end
+      end
+    end
+
+    class All < Base
+      def recipients
+        scope = scope_sponsorships(@broadcast.conference.sponsorships.includes(:contact))
+        sponsorships_to_recipients(scope)
+      end
+    end
+
+    class PastSponsors < Base
+      def recipients
+        conference = Conference.find_by!(id: params[:id])
+        scope = scope_sponsorships(conference.sponsorships.includes(:contact))
+        sponsorships_to_recipients(scope)
+      end
+    end
+
+    class Manual < Base
+      def recipients
+        scope = Sponsorship.where(id: [*params[:sponsorship_ids]])
+        if params[:exclude_current_sponsors].present?
+          scope = scope.where.not(organization_id: @broadcast.conference.sponsorships.pluck(:organization_id))
+        end
+        sponsorships_to_recipients(scope)
+      end
+    end
+
+    class Raw < Base
+      def recipients
+        [*params[:emails]].flatten.flat_map do |email_lines|
+          email_lines.to_s.each_line.map do |email|
+            Recipient.new(
+              email: email.chomp,
+              email_ccs: [],
+            )
+          end
+        end
+      end
+    end
+  end
+
   def perform(broadcast, recipient_filter_params)
     ApplicationRecord.transaction do
       @broadcast = broadcast
@@ -17,16 +136,16 @@ def perform(broadcast, recipient_filter_params)
         raise "Invalid state for CreateBroadcastDeliveriesJob (broadcast_id=#{broadcast.id}, state=#{broadcast.status})"
       end
 
-      existing_emails = broadcast.deliveries.pluck(:recipient)
+      existing_emails = broadcast.deliveries.pluck(:recipient).tally
       recipients = recipient_filter_params.flat_map do |filter|
         filter = filter.dup
-        kind = filter.delete('kind') || target.delete(:kind)
+        kind = filter.delete('kind')
 
         filter_recipients(kind, filter)
       end
 
       recipients.map do |recipient|
-        next if existing_emails.include?(recipient.email)
+        next if existing_emails[recipient.email]
         broadcast.deliveries.create!(
           status: :ready,
           sponsorship: recipient.sponsorship,
@@ -45,65 +164,13 @@ def perform(broadcast, recipient_filter_params)
   def filter_recipients(kind, params)
     case kind.to_s
     when 'all'
-      scope = @broadcast.conference.sponsorships.not_withdrawn.includes(:contact)
-      scope = scope.where(plan: @broadcast.conference.plans.find_by!(id: params[:plan_id])) if params[:plan_id].present?
-      scope = scope.where(locale: params[:locale]) if params[:locale].present?
-      scope = scope.exhibitor if params[:exhibitors].present?
-      case params[:status]
-      when 'all'
-        # do nothing
-      when 'not_accepted'
-        scope = scope.where(accepted_at: nil)
-      when 'accepted'
-        scope = scope.accepted
-      end
-
-      scope.map do |sponsorship|
-        Recipient.new(
-          sponsorship: sponsorship,
-          email: sponsorship.contact.email,
-          email_ccs: sponsorship.contact.email_ccs,
-        )
-      end
+      Filters::All.new(broadcast: @broadcast, params: params).recipients
     when 'past_sponsors'
-      conference = Conference.find_by!(id: params[:id])
-      scope = conference.sponsorships.not_withdrawn.includes(:contact)
-      scope = scope.where(locale: params[:locale]) if params[:locale].present?
-      scope = scope.where.not(organization_id: @broadcast.conference.sponsorships.pluck(:organization_id)) if params[:exclude_current_sponsors]
-      case params[:status]
-      when 'all'
-        # do nothing
-      when 'not_accepted'
-        scope = scope.where(accepted_at: nil)
-      when 'accepted'
-        scope = scope.accepted
-      end
-
-      scope.map do |sponsorship|
-        Recipient.new(
-          sponsorship: sponsorship,
-          email: sponsorship.contact.email,
-          email_ccs: sponsorship.contact.email_ccs,
-        )
-      end
+      Filters::PastSponsors.new(broadcast: @broadcast, params: params).recipients
     when 'raw'
-      [*params[:emails]].flatten.flat_map do |email_lines|
-        email_lines.to_s.each_line.map do |email|
-          Recipient.new(
-            email: email.chomp,
-            email_ccs: [],
-          )
-        end
-      end
+      Filters::Raw.new(broadcast: @broadcast, params: params).recipients
     when 'manual'
-      scope = Sponsorship.where(id: [*params[:sponsorship_ids]])
-      scope.map do |sponsorship|
-        Recipient.new(
-          sponsorship: sponsorship,
-          email: sponsorship.contact.email,
-          email_ccs: sponsorship.contact.email_ccs,
-        )
-      end
+      Filters::Manual.new(broadcast: @broadcast, params: params).recipients
     when 'none'
       []
     else

app/models/sponsorship.rb

@@ -47,12 +47,17 @@ def tito_booth_staff_discount_code
     @tito_booth_staff_discount_code ||= tito_discount_codes.where(kind: 'booth_staff').first
   end
 
-  scope :active, -> { where(withdrawn_at: nil).where.not(accepted_at: nil) }
+  scope :active, -> { accepted.not_withdrawn }
+  scope :pending, -> { not_accepted.not_withdrawn }
+
   scope :exhibitor, -> { where(booth_assigned: true) }
   scope :plan_determined, -> { where.not(plan_id: nil) }
+
   scope :withdrawn, -> { where.not(withdrawn_at: nil) }
   scope :not_withdrawn, -> { where(withdrawn_at: nil) }
   scope :accepted, -> { where.not(accepted_at: nil) }
+  scope :not_accepted, -> { where(accepted_at: nil) }
+
   scope :have_presence, -> { where(suspended: false).merge(Sponsorship.active).merge(Sponsorship.plan_determined) }
 
   scope :includes_contacts, -> { includes(:contact, :alternate_billing_contact) }

app/views/admin/broadcasts/_new_recipient_fields.html.haml

@@ -20,13 +20,22 @@
       .form-check
         %label.form-check-label
           = radio_button_tag "recipient_filter[status]", 'all', true, {class: 'form-check-input'}
-          All applications (where not withdrawn)
+          All
+        %label.form-check-label
+          = radio_button_tag "recipient_filter[status]", 'pending', false, {class: 'form-check-input'}
+          Pending
+        %label.form-check-label
+          = radio_button_tag "recipient_filter[status]", 'active', false, {class: 'form-check-input'}
+          Active
         %label.form-check-label
           = radio_button_tag "recipient_filter[status]", 'not_accepted', false, {class: 'form-check-input'}
-          Not accepted only
+          Not accepted (incl. withdrawn)
         %label.form-check-label
           = radio_button_tag "recipient_filter[status]", 'accepted', false, {class: 'form-check-input'}
-          Accepted only
+          Accepted (incl. withdrawn)
+        %label.form-check-label
+          = radio_button_tag "recipient_filter[status]", 'withdrawn', false, {class: 'form-check-input'}
+          Withdrawn
 
   %fieldset.broadcast_new_recipient_fields_kind__past_sponsors.d-none{disabled: true}
     .form-group
@@ -42,13 +51,24 @@
       .form-check
         %label.form-check-label
           = radio_button_tag "recipient_filter[status]", 'all', true, {class: 'form-check-input'}
-          All applications (where not withdrawn)
+          All
+        %label.form-check-label
+          = radio_button_tag "recipient_filter[status]", 'pending', false, {class: 'form-check-input'}
+          Pending
+        %label.form-check-label
+          = radio_button_tag "recipient_filter[status]", 'active', false, {class: 'form-check-input'}
+          Active
         %label.form-check-label
           = radio_button_tag "recipient_filter[status]", 'not_accepted', false, {class: 'form-check-input'}
-          Not accepted only
+          Not accepted (incl. withdrawn)
         %label.form-check-label
           = radio_button_tag "recipient_filter[status]", 'accepted', false, {class: 'form-check-input'}
-          Accepted only
+          Accepted (incl. withdrawn)
+        %label.form-check-label
+          = radio_button_tag "recipient_filter[status]", 'withdrawn', false, {class: 'form-check-input'}
+          Withdrawn
+
+
 
   %fieldset.broadcast_new_recipient_fields_kind__manual.d-none{disabled: true}
     .form-group

spec/factories/broadcasts.rb

@@ -0,0 +1,11 @@
+FactoryBot.define do
+  factory :broadcast do
+    conference
+    staff
+    status { :created }
+    sequence(:campaign) { |n| "Campaign #{n}" }
+    sequence(:description) { |n| "#{n}" }
+    sequence(:title) { |n| "Email #{n}" }
+    sequence(:body) { |n| "- Email #{n}" }
+  end
+end

spec/factories/conferences.rb

@@ -0,0 +1,15 @@
+FactoryBot.define do
+  factory :conference do
+    sequence(:name) { |n| "Conf #{n}" }
+    sequence(:slug) { |n| "conf#{n}" }
+    booth_capacity { 50 }
+    sequence(:contact_email_address) { |n| "info+#{n}@conf.test.invalid" }
+
+    trait :full do
+      after(:create) do |conference, context|
+        create(:form_description, conference:)
+        create(:plan, conference:)
+      end
+    end
+  end
+end

spec/factories/contacts.rb

@@ -0,0 +1,10 @@
+FactoryBot.define do
+  factory :contact do
+    #sponsorship
+    kind { "primary" }
+    sequence(:email) { |n|  "primary@#{n}.co.invalid" }
+    sequence(:address) { |n|  "#{n}-#{n}-#{n}" }
+    sequence(:organization) { |n| "Contoso" }
+    sequence(:name) { |n| "User #{n}" }
+  end
+end

spec/factories/form_descriptions.rb

@@ -0,0 +1,11 @@
+FactoryBot.define do
+  factory :form_description do
+    conference
+    locale { "en" }
+    head { "- Head #{locale} #{conference.id}" }
+    plan_help { "- Plan #{locale} #{conference.id}" }
+    booth_help { "- Booth #{locale} #{conference.id}" }
+    policy_help { "- Policy #{locale} #{conference.id}" }
+    ticket_help { "- Ticket #{locale} #{conference.id}" }
+  end
+end

spec/factories/plans.rb

@@ -0,0 +1,13 @@
+FactoryBot.define do
+  factory :plan do
+    conference
+    sequence(:name) { |n| "Plan #{n}" }
+    sequence(:rank) { |n| n }
+    summary { "na" }
+    capacity { 10 }
+    number_of_guests { 1 }
+    price_text { "na" }
+    words_limit { 15 }
+    auto_acceptance { true }
+  end
+end