Commit c140dcd8cac306c327a830131f7041b2eb94972b
Committed by
Antonio Terceiro
1 parent
3cab8c3f
Exists in
staging
and in
42 other branches
Send mailing in batches
(ActionItem1712)
Showing
5 changed files
with
42 additions
and
41 deletions
Show diff stats
app/models/environment_mailing.rb
| 1 | 1 | class EnvironmentMailing < Mailing |
| 2 | 2 | |
| 3 | - def recipient(offset=0) | |
| 4 | - source.people.first(:order => :id, :offset => offset) | |
| 3 | + def recipients(offset=0, limit=100) | |
| 4 | + source.people.all(:order => :id, :offset => offset, :limit => limit, :joins => "LEFT OUTER JOIN mailing_sents m ON (m.mailing_id = #{id} AND m.person_id = profiles.id)", :order => :id, :conditions => { "m.person_id" => nil }) | |
| 5 | 5 | end |
| 6 | 6 | |
| 7 | 7 | def each_recipient |
| 8 | 8 | offset = 0 |
| 9 | - while person = recipient(offset) | |
| 10 | - unless self.already_sent_mailing_to?(person) | |
| 11 | - yield person | |
| 9 | + limit = 100 | |
| 10 | + while !(people = recipients(offset, limit)).empty? | |
| 11 | + people.each do |person| | |
| 12 | + yield person | |
| 12 | 13 | end |
| 13 | - offset = offset + 1 | |
| 14 | + offset = offset + limit | |
| 14 | 15 | end |
| 15 | 16 | end |
| 16 | 17 | ... | ... |
app/models/mailing.rb
| ... | ... | @@ -28,13 +28,7 @@ class Mailing < ActiveRecord::Base |
| 28 | 28 | '' |
| 29 | 29 | end |
| 30 | 30 | |
| 31 | - def already_sent_mailing_to?(recipient) | |
| 32 | - self.mailing_sents.find_by_person_id(recipient.id) | |
| 33 | - end | |
| 34 | - | |
| 35 | 31 | def deliver |
| 36 | - offset = 0 | |
| 37 | - people = [] | |
| 38 | 32 | each_recipient do |recipient| |
| 39 | 33 | Mailing::Sender.deliver_mail(self, recipient.email) |
| 40 | 34 | self.mailing_sents.create(:person => recipient) | ... | ... |
app/models/organization_mailing.rb
| ... | ... | @@ -4,17 +4,18 @@ class OrganizationMailing < Mailing |
| 4 | 4 | "#{person.name} <#{source.environment.contact_email}>" |
| 5 | 5 | end |
| 6 | 6 | |
| 7 | - def recipient(offset=0) | |
| 8 | - source.members.first(:order => :id, :offset => offset) | |
| 7 | + def recipients(offset=0, limit=100) | |
| 8 | + source.members.all(:order => :id, :offset => offset, :limit => limit, :joins => "LEFT OUTER JOIN mailing_sents m ON (m.mailing_id = #{id} AND m.person_id = profiles.id)", :order => :id, :conditions => { "m.person_id" => nil }) | |
| 9 | 9 | end |
| 10 | 10 | |
| 11 | 11 | def each_recipient |
| 12 | 12 | offset = 0 |
| 13 | - while person = recipient(offset) | |
| 14 | - unless self.already_sent_mailing_to?(person) | |
| 15 | - yield person | |
| 13 | + limit = 50 | |
| 14 | + while !(people = recipients(offset, limit)).empty? | |
| 15 | + people.each do |person| | |
| 16 | + yield person | |
| 16 | 17 | end |
| 17 | - offset = offset + 1 | |
| 18 | + offset = offset + limit | |
| 18 | 19 | end |
| 19 | 20 | end |
| 20 | 21 | ... | ... |
test/unit/environment_mailing_test.rb
| ... | ... | @@ -7,10 +7,10 @@ class EnvironmentMailingTest < ActiveSupport::TestCase |
| 7 | 7 | ActionMailer::Base.perform_deliveries = true |
| 8 | 8 | ActionMailer::Base.deliveries = [] |
| 9 | 9 | @environment = fast_create(Environment, :name => 'Network') |
| 10 | - create_user('user_one', :environment_id => @environment.id) | |
| 11 | - create_user('user_two', :environment_id => @environment.id) | |
| 10 | + @person_1 = create_user('user_one', :environment_id => @environment.id).person | |
| 11 | + @person_2 = create_user('user_two', :environment_id => @environment.id).person | |
| 12 | 12 | end |
| 13 | - attr_reader :environment | |
| 13 | + attr_reader :environment, :person_1, :person_2 | |
| 14 | 14 | |
| 15 | 15 | |
| 16 | 16 | should 'require source_id' do |
| ... | ... | @@ -45,53 +45,53 @@ class EnvironmentMailingTest < ActiveSupport::TestCase |
| 45 | 45 | end |
| 46 | 46 | |
| 47 | 47 | should 'display name and email on generate_from' do |
| 48 | - person = Person['user_one'] | |
| 49 | - mailing = EnvironmentMailing.new(:source => environment, :person => person) | |
| 48 | + mailing = EnvironmentMailing.new(:source => environment, :person => person_1) | |
| 50 | 49 | assert_equal "#{environment.name} <#{environment.contact_email}>", mailing.generate_from |
| 51 | 50 | end |
| 52 | 51 | |
| 53 | 52 | should 'deliver mailing to each recipient after create' do |
| 54 | - person = Person['user_one'] | |
| 55 | - mailing = EnvironmentMailing.create(:source => environment, :subject => 'Hello', :body => 'We have some news', :person => person) | |
| 53 | + mailing = EnvironmentMailing.create(:source => environment, :subject => 'Hello', :body => 'We have some news', :person => person_1) | |
| 56 | 54 | process_delayed_job_queue |
| 57 | 55 | assert_equal 2, ActionMailer::Base.deliveries.count |
| 58 | 56 | end |
| 59 | 57 | |
| 60 | 58 | should 'create mailing sent to each recipient after delivering mailing' do |
| 61 | - person = Person['user_one'] | |
| 62 | - mailing = EnvironmentMailing.create(:source => environment, :subject => 'Hello', :body => 'We have some news', :person => person) | |
| 59 | + mailing = EnvironmentMailing.create(:source => environment, :subject => 'Hello', :body => 'We have some news', :person => person_1) | |
| 63 | 60 | assert_difference MailingSent, :count, 2 do |
| 64 | 61 | process_delayed_job_queue |
| 65 | 62 | end |
| 66 | 63 | end |
| 67 | 64 | |
| 68 | 65 | should 'change locale according to the mailing locale' do |
| 69 | - person = Person['user_one'] | |
| 70 | - mailing = EnvironmentMailing.create(:source => environment, :subject => 'Hello', :body => 'We have some news', :locale => 'pt', :person => person) | |
| 66 | + mailing = EnvironmentMailing.create(:source => environment, :subject => 'Hello', :body => 'We have some news', :locale => 'pt', :person => person_1) | |
| 71 | 67 | Noosfero.expects(:with_locale).with('pt') |
| 72 | 68 | process_delayed_job_queue |
| 73 | 69 | end |
| 74 | 70 | |
| 75 | - should 'return recipient' do | |
| 76 | - mailing = EnvironmentMailing.new(:source => environment) | |
| 77 | - assert_equal Person['user_one'], mailing.recipient | |
| 71 | + should 'return recipients' do | |
| 72 | + mailing = EnvironmentMailing.create(:source => environment, :subject => 'Hello', :body => 'We have some news', :locale => 'pt', :person => person_1) | |
| 73 | + assert_equal [person_1, person_2], mailing.recipients | |
| 74 | + end | |
| 75 | + | |
| 76 | + should 'return recipients according to limit' do | |
| 77 | + mailing = EnvironmentMailing.create(:source => environment, :subject => 'Hello', :body => 'We have some news', :locale => 'pt', :person => person_1) | |
| 78 | + assert_equal [person_1], mailing.recipients(0, 1) | |
| 78 | 79 | end |
| 79 | 80 | |
| 80 | 81 | should 'return true if already sent mailing to a recipient' do |
| 81 | - person = Person['user_one'] | |
| 82 | - mailing = EnvironmentMailing.create(:source => environment, :subject => 'Hello', :body => 'We have some news', :person => person) | |
| 82 | + mailing = EnvironmentMailing.create(:source => environment, :subject => 'Hello', :body => 'We have some news', :person => person_1) | |
| 83 | 83 | process_delayed_job_queue |
| 84 | 84 | |
| 85 | - assert mailing.already_sent_mailing_to?(person) | |
| 85 | + assert mailing.mailing_sents.find_by_person_id(person_1.id) | |
| 86 | 86 | end |
| 87 | 87 | |
| 88 | 88 | should 'return false if did not sent mailing to a recipient' do |
| 89 | 89 | recipient = fast_create(Person) |
| 90 | 90 | person = Person['user_one'] |
| 91 | - mailing = EnvironmentMailing.create(:source => environment, :subject => 'Hello', :body => 'We have some news', :person => person) | |
| 91 | + mailing = EnvironmentMailing.create(:source => environment, :subject => 'Hello', :body => 'We have some news', :person => person_1) | |
| 92 | 92 | process_delayed_job_queue |
| 93 | 93 | |
| 94 | - assert !mailing.already_sent_mailing_to?(recipient) | |
| 94 | + assert !mailing.mailing_sents.find_by_person_id(recipient.id) | |
| 95 | 95 | end |
| 96 | 96 | |
| 97 | 97 | end | ... | ... |
test/unit/organization_mailing_test.rb
| ... | ... | @@ -87,15 +87,20 @@ class OrganizationMailingTest < ActiveSupport::TestCase |
| 87 | 87 | end |
| 88 | 88 | |
| 89 | 89 | should 'return recipient' do |
| 90 | - mailing = OrganizationMailing.new(:source => community) | |
| 91 | - assert_equal Person['user_one'], mailing.recipient | |
| 90 | + mailing = OrganizationMailing.create(:source => community, :subject => 'Hello', :body => 'We have some news', :person => person) | |
| 91 | + assert_equal [Person['user_one'], Person['user_two']], mailing.recipients | |
| 92 | + end | |
| 93 | + | |
| 94 | + should 'return recipients according to limit' do | |
| 95 | + mailing = OrganizationMailing.create(:source => community, :subject => 'Hello', :body => 'We have some news', :person => person) | |
| 96 | + assert_equal [Person['user_one']], mailing.recipients(0, 1) | |
| 92 | 97 | end |
| 93 | 98 | |
| 94 | 99 | should 'return true if already sent mailing to a recipient' do |
| 95 | 100 | member = Person['user_one'] |
| 96 | 101 | mailing = OrganizationMailing.create(:source => community, :subject => 'Hello', :body => 'We have some news', :person => person) |
| 97 | 102 | process_delayed_job_queue |
| 98 | - assert mailing.already_sent_mailing_to?(member) | |
| 103 | + assert mailing.mailing_sents.find_by_person_id(member.id) | |
| 99 | 104 | end |
| 100 | 105 | |
| 101 | 106 | should 'return false if did not sent mailing to a recipient' do |
| ... | ... | @@ -103,7 +108,7 @@ class OrganizationMailingTest < ActiveSupport::TestCase |
| 103 | 108 | mailing = OrganizationMailing.create(:source => community, :subject => 'Hello', :body => 'We have some news', :person => person) |
| 104 | 109 | process_delayed_job_queue |
| 105 | 110 | |
| 106 | - assert !mailing.already_sent_mailing_to?(recipient) | |
| 111 | + assert !mailing.mailing_sents.find_by_person_id(recipient.id) | |
| 107 | 112 | end |
| 108 | 113 | |
| 109 | 114 | protected | ... | ... |