diff --git a/app/controllers/members_controller.rb b/app/controllers/members_controller.rb index bc1afce..eefedd7 100644 --- a/app/controllers/members_controller.rb +++ b/app/controllers/members_controller.rb @@ -31,6 +31,12 @@ class MembersController < ApplicationController @member.generate_missing_ifthenpay_links! @member.reset_status! NotificationMailer.with(member: @member).registration.deliver_now! + @member.notifications.create!( + template: "registration", + to_be_sent_on: Date.today, + sent_at: Time.current, + status: "sent", + ) redirect_to @member, notice: "Member was successfully created." else @@ -40,6 +46,12 @@ class MembersController < ApplicationController def resend_registration NotificationMailer.with(member: @member).registration.deliver_now! + @member.notifications.create!( + template: "registration", + to_be_sent_on: Date.today, + sent_at: Time.current, + status: "sent", + ) redirect_to @member, notice: "Payment reminder sent." end diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index c3a16f8..d5acfc8 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -4,8 +4,7 @@ class NotificationsController < ApplicationController # GET /notifications def index - @scheduled = Notification.where(status: 'scheduled').order(to_be_sent_on: :asc) - @sent = Notification.where(status: 'sent').order(sent_at: :desc) + @notifications = Notification.order(to_be_sent_on: :asc) end # POST /notifications/1/deliver diff --git a/app/mailers/notification_mailer.rb b/app/mailers/notification_mailer.rb index 8353e81..c4b0391 100644 --- a/app/mailers/notification_mailer.rb +++ b/app/mailers/notification_mailer.rb @@ -2,32 +2,32 @@ class NotificationMailer < ApplicationMailer default to: ->() { @member.email } def expiration_in_60d - set_notification + set_member mail end def expiration_in_30d - set_notification + set_member mail end def expired - set_notification + set_member mail end def expired_30d_ago - set_notification + set_member mail end def expired_60d_ago - set_notification + set_member mail end def cancelled - set_notification + set_member mail end @@ -47,12 +47,6 @@ class NotificationMailer < ApplicationMailer end private - def set_notification - @notification = params[:notification] - @member = @notification.member - @payment = @notification.member.create_payment - end - def set_contribution @contribution = params[:contribution] @member = @contribution.member diff --git a/app/models/member.rb b/app/models/member.rb index b29a9bf..c3ff65f 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -65,7 +65,6 @@ class Member < ApplicationRecord if contribution.save self.handle_new_contribution(contribution, overriden_expires_on) self.reset_status! - self.regenerate_notifications if should_send_notification if is_first_contribution @@ -73,11 +72,23 @@ class Member < ApplicationRecord .with(contribution: contribution) .first_payment_confirmation .deliver_now! + self.notifications.create!( + template: "first_payment_confirmation", + to_be_sent_on: Date.today, + sent_at: Time.current, + status: "sent", + ) else NotificationMailer .with(contribution: contribution) .payment_renewal_confirmation .deliver_now! + self.notifications.create!( + template: "payment_renewal_confirmation", + to_be_sent_on: Date.today, + sent_at: Time.current, + status: "sent", + ) end end @@ -99,24 +110,29 @@ class Member < ApplicationRecord save! end - def regenerate_notifications(from=Date.today) - notifications.where(status: 'scheduled').delete_all + def todays_notification + candidate = Notification + .templates + .filter { |t| self.expires_on + t[:to_be_sent_on] <= Date.today } + .last - dates = notifications.pluck(:to_be_sent_on) + if candidate + last_sent = self + .notifications + .where(status: 'sent', template: candidate[:template]) + .where.not(sent_at: nil) + .order(:sent_at) + .last - return if expires_on.nil? - - [ - { to_be_sent_on: expires_on - 60.days, template: "expiration_in_60d" }, - { to_be_sent_on: expires_on - 30.days, template: "expiration_in_30d" }, - { to_be_sent_on: expires_on + 0.days, template: "expired" }, - { to_be_sent_on: expires_on + 30.days, template: "expired_30d_ago" }, - { to_be_sent_on: expires_on + 60.days, template: "expired_60d_ago" }, - { to_be_sent_on: expires_on + Config.payment_pending_grace_period, template: "cancelled" }, - ].reject do |n| - n[:to_be_sent_on].before?(from) || dates.include?(n[:to_be_sent_on]) - end.each do |n| - notifications.create(n.merge(status: "scheduled")) + if last_sent && self.expires_on + candidate[:to_be_sent_on] <= last_sent.sent_at + nil + else + self.notifications.new( + template: candidate[:template], + status: 'scheduled', + to_be_sent_on: Date.today, + ) + end end end @@ -137,24 +153,6 @@ class Member < ApplicationRecord end def self.reset_all_status! - Member.all.each do |member| - member.reset_status! - end - end - - def self.generate_all_missing_ifthenpay_links! - Member.all.each do |member| - member.generate_missing_ifthenpay_links! - end - end - - def self.regenerate_all_notifications - ActiveRecord::Base.transaction do - ActiveRecord::Base.connection.execute('LOCK members IN ACCESS EXCLUSIVE MODE') - - Member.all.each do |member| - member.regenerate_notifications - end - end + Member.all.each(&:reset_status!) end end diff --git a/app/models/notification.rb b/app/models/notification.rb index 63587eb..e0af780 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -1,23 +1,29 @@ class Notification < ApplicationRecord belongs_to :member - scope :scheduled_for_today, ->() { where(status: 'scheduled', to_be_sent_on: Date.today) } - def self.send_scheduled_for_today - ActiveRecord::Base.transaction do - ActiveRecord::Base.connection.execute('LOCK members IN ACCESS EXCLUSIVE MODE') + Member + .where('expires_on < ?', Date.today - self.templates.first[:to_be_sent_on]) + .filter_map(&:todays_notification) + .each(&:deliver!) + end - scheduled_for_today.each do |n| - n.deliver! - end - end + def self.templates + [ + { to_be_sent_on: -60.days, template: "expiration_in_60d" }, + { to_be_sent_on: -30.days, template: "expiration_in_30d" }, + { to_be_sent_on: 0.days, template: "expired" }, + { to_be_sent_on: 30.days, template: "expired_30d_ago" }, + { to_be_sent_on: 60.days, template: "expired_60d_ago" }, + { to_be_sent_on: Config.payment_pending_grace_period, template: "cancelled" }, + ] end def deliver! - # actually send the email. - NotificationMailer.with(notification: self).send(template).deliver_now! - - update(status: 'sent', sent_at: Time.current) # TODO: do something about failures + + save! + NotificationMailer.with(member: self.member).send(template).deliver_now! + update(status: 'sent', sent_at: Time.current) end end diff --git a/app/views/notification_mailer/expiration_in_30d.html.erb b/app/views/notification_mailer/expiration_in_30d.html.erb index bd817c8..507898c 100644 --- a/app/views/notification_mailer/expiration_in_30d.html.erb +++ b/app/views/notification_mailer/expiration_in_30d.html.erb @@ -13,7 +13,7 @@

Para estender a tua inscrição por mais um ano, pedimos que faças o pagamento - das quotas até <%= @notification.member.expires_on %>. + das quotas até <%= @member.expires_on %>.

<%= render partial: "payment", locals: { payment: @payment } %> diff --git a/app/views/notification_mailer/expiration_in_30d.text.erb b/app/views/notification_mailer/expiration_in_30d.text.erb index 69a08f2..b5ed5e5 100644 --- a/app/views/notification_mailer/expiration_in_30d.text.erb +++ b/app/views/notification_mailer/expiration_in_30d.text.erb @@ -8,7 +8,7 @@ nossas actividades, e gostaríamos de continuar a contar com a tua participação. Para estender a tua inscrição por mais um ano, pedimos que faças o pagamento -das quotas até <%= @notification.member.expires_on %>. +das quotas até <%= @member.expires_on %>. <%= render partial: "payment", locals: { payment: @payment } %> diff --git a/app/views/notification_mailer/expiration_in_60d.html.erb b/app/views/notification_mailer/expiration_in_60d.html.erb index c92d920..5d082d8 100644 --- a/app/views/notification_mailer/expiration_in_60d.html.erb +++ b/app/views/notification_mailer/expiration_in_60d.html.erb @@ -13,7 +13,7 @@

Para estender a tua inscrição por mais um ano, pedimos que faças o pagamento - das quotas até <%= @notification.member.expires_on %>. + das quotas até <%= @member.expires_on %>.

<%= render partial: "payment", locals: { payment: @payment } %> diff --git a/app/views/notification_mailer/expiration_in_60d.text.erb b/app/views/notification_mailer/expiration_in_60d.text.erb index 13ba728..e40f637 100644 --- a/app/views/notification_mailer/expiration_in_60d.text.erb +++ b/app/views/notification_mailer/expiration_in_60d.text.erb @@ -8,7 +8,7 @@ nossas actividades, e gostaríamos de continuar a contar com a tua participação. Para estender a tua inscrição por mais um ano, pedimos que faças o pagamento -das quotas até <%= @notification.member.expires_on %>. +das quotas até <%= @member.expires_on %>. <%= render partial: "payment", locals: { payment: @payment } %> diff --git a/app/views/notification_mailer/expired.html.erb b/app/views/notification_mailer/expired.html.erb index 4400eab..4450688 100644 --- a/app/views/notification_mailer/expired.html.erb +++ b/app/views/notification_mailer/expired.html.erb @@ -13,7 +13,7 @@ <%= render partial: "payment", locals: { payment: @payment } %>

- Caso não recebamos o pagamento até dia <%= @notification.member.cancelled_on + Caso não recebamos o pagamento até dia <%= @member.cancelled_on %>, cancelaremos permanentemente a tua inscrição. Estamos disponíveis para esclarecer qualquer dúvida através do endereço direccao@ansol.org.

diff --git a/app/views/notification_mailer/expired.text.erb b/app/views/notification_mailer/expired.text.erb index 441c1dc..4698901 100644 --- a/app/views/notification_mailer/expired.text.erb +++ b/app/views/notification_mailer/expired.text.erb @@ -8,7 +8,7 @@ nossas actividades. Gostaríamos de continuar a contar com a tua participação. <%= render partial: "payment", locals: { payment: @payment } %> -Caso não recebamos o pagamento até dia <%= @notification.member.cancelled_on +Caso não recebamos o pagamento até dia <%= @member.cancelled_on %>, cancelaremos permanentemente a tua inscrição. Estamos disponíveis para esclarecer qualquer dúvida através do endereço direccao@ansol.org. diff --git a/app/views/notification_mailer/expired_30d_ago.html.erb b/app/views/notification_mailer/expired_30d_ago.html.erb index 4b05df8..941b866 100644 --- a/app/views/notification_mailer/expired_30d_ago.html.erb +++ b/app/views/notification_mailer/expired_30d_ago.html.erb @@ -13,7 +13,7 @@ <%= render partial: "payment", locals: { payment: @payment } %>

- Caso não recebamos o pagamento até dia <%= @notification.member.cancelled_on + Caso não recebamos o pagamento até dia <%= @member.cancelled_on %>, cancelaremos permanentemente a tua inscrição. Estamos disponíveis para esclarecer qualquer dúvida através do endereço direccao@ansol.org.

diff --git a/app/views/notification_mailer/expired_30d_ago.text.erb b/app/views/notification_mailer/expired_30d_ago.text.erb index 628701c..6f8fd8f 100644 --- a/app/views/notification_mailer/expired_30d_ago.text.erb +++ b/app/views/notification_mailer/expired_30d_ago.text.erb @@ -8,7 +8,7 @@ nossas actividades. Gostaríamos de continuar a contar com a tua participação. <%= render partial: "payment", locals: { payment: @payment } %> -Caso não recebamos o pagamento até dia <%= @notification.member.cancelled_on +Caso não recebamos o pagamento até dia <%= @member.cancelled_on %>, cancelaremos permanentemente a tua inscrição. Estamos disponíveis para esclarecer qualquer dúvida através do endereço direccao@ansol.org. diff --git a/app/views/notification_mailer/expired_60d_ago.html.erb b/app/views/notification_mailer/expired_60d_ago.html.erb index 4400eab..4450688 100644 --- a/app/views/notification_mailer/expired_60d_ago.html.erb +++ b/app/views/notification_mailer/expired_60d_ago.html.erb @@ -13,7 +13,7 @@ <%= render partial: "payment", locals: { payment: @payment } %>

- Caso não recebamos o pagamento até dia <%= @notification.member.cancelled_on + Caso não recebamos o pagamento até dia <%= @member.cancelled_on %>, cancelaremos permanentemente a tua inscrição. Estamos disponíveis para esclarecer qualquer dúvida através do endereço direccao@ansol.org.

diff --git a/app/views/notification_mailer/expired_60d_ago.text.erb b/app/views/notification_mailer/expired_60d_ago.text.erb index f42d761..f89d984 100644 --- a/app/views/notification_mailer/expired_60d_ago.text.erb +++ b/app/views/notification_mailer/expired_60d_ago.text.erb @@ -8,7 +8,7 @@ nossas actividades. Gostaríamos de continuar a contar com a tua participação. <%= render partial: "payment", locals: { payment: @payment } %> -Caso não recebamos o pagamento até dia <%= @notification.member.cancelled_on +Caso não recebamos o pagamento até dia <%= @member.cancelled_on %>, cancelaremos permanentemente a tua inscrição. Estamos disponíveis para esclarecer qualquer dúvida através do endereço direccao@ansol.org. diff --git a/app/views/notifications/index.html.erb b/app/views/notifications/index.html.erb index 95755e8..b8c41a4 100644 --- a/app/views/notifications/index.html.erb +++ b/app/views/notifications/index.html.erb @@ -1,59 +1,32 @@

<%= t('notifications.index.title') %>

-
-

<%= t('notifications.index.scheduled') %>

 - +
+ + + + + + + + <% @notifications.each do |notification| %> - - - - + + + + + - <% @scheduled.each do |notification| %> - - - - - - - <% end %> -
<%= t('notifications.attributes.to_be_sent_on') %><%= t('notifications.attributes.status') %><%= t('notifications.attributes.template') %><%= t('notifications.attributes.member') %><%= t('members.show.contribution_actions') %>
<%= t('notifications.attributes.to_be_sent_on') %><%= t('notifications.attributes.template') %><%= t('notifications.attributes.member') %><%= t('members.show.contribution_actions') %><%= notification.to_be_sent_on %><%= notification.status %><%= notification.template %> + <% if notification.member %> + <%= link_to notification.member do %><%= notification.member.display_name %> <<%= notification.member.email %>><% end %> + <% else %> + --- + <% end %> + + <% if notification.status == 'scheduled' %> + <%= form_with url: deliver_notification_path(notification) do |form| %> + <%= form.submit t('members.show.actions.deliver_notification') %> + <% end %> + <% end %> +
<%= notification.to_be_sent_on %><%= notification.template %> - <% if notification.member %> - <%= link_to notification.member do %><%= notification.member.display_name %> <<%= notification.member.email %>><% end %> - <% else %> - --- - <% end %> -
-
- -
-

<%= t('notifications.index.sent') %>

 - - - - - - - - <% @sent.each do |notification| %> - - - - - - - <% end %> -
<%= t('notifications.attributes.to_be_sent_on') %><%= t('notifications.attributes.template') %><%= t('notifications.attributes.member') %><%= t('members.show.contribution_actions') %>
<%= notification.to_be_sent_on %><%= notification.template %> - <% if notification.member %> - <%= link_to notification.member do %><%= notification.member.display_name %> <<%= notification.member.email %>><% end %> - <% else %> - --- - <% end %> - - <% if notification.status == 'scheduled' %> - <%= form_with url: deliver_notification_path(notification) do |form| %> - <%= form.submit t('members.show.actions.deliver_notification') %> - <% end %> - <% end %> -
-
+ <% end %> + diff --git a/lib/tasks/saucy.rake b/lib/tasks/saucy.rake index ac40257..de84d94 100644 --- a/lib/tasks/saucy.rake +++ b/lib/tasks/saucy.rake @@ -2,9 +2,7 @@ desc "Application specific tasks" namespace :saucy do desc "Background sync operations" task sync: :environment do - Member.generate_all_missing_ifthenpay_links! Member.reset_all_status! - Member.regenerate_all_notifications end desc "Send daily email notifications" diff --git a/test/models/member_test.rb b/test/models/member_test.rb index 51f8bf1..0f8ce2e 100644 --- a/test/models/member_test.rb +++ b/test/models/member_test.rb @@ -18,44 +18,77 @@ class MemberTest < ActiveSupport::TestCase end end - test "expired after 1 year and 90 days" do - Timecop.freeze(Date.today + 1.year + 90.days) do - assert_equal :cancelled, @member.expected_status - end - end - - test "regenerate notifications creates all 6 notifications" do - @member.regenerate_notifications - - assert_equal 6, @member.notifications.count - end - - test "regenerate_notifications does not create notifications in the past" do - Timecop.freeze(Date.today + 1.year) do - @member.regenerate_notifications - assert_equal 4, @member.notifications.count - end - - Timecop.freeze(Date.today + 2.year) do - @member.regenerate_notifications - assert_equal 0, @member.notifications.count + test "expired after 1 year and 90 days" do + Timecop.freeze(Date.today + 1.year + 90.days) do + assert_equal :cancelled, @member.expected_status end end - test "regenerate_notifications does not create manually sent notifications" do - @member.regenerate_notifications - - @member.notifications.first.deliver! - assert_equal 5, @member.notifications.where(status: 'scheduled').count - - @member.regenerate_notifications - assert_equal 1, @member.notifications.where(status: 'sent').count - assert_equal 5, @member.notifications.where(status: 'scheduled').count + test "no notifications in the first ~10 months" do + (Date.today .. (@member.expires_on + Notification.templates.first[:to_be_sent_on] - 1.day)).each do |day| + Timecop.freeze(day) do + assert_nil @member.todays_notification + end + end end - test "regenerate_notifications when given a date generates only notifications after that date" do - @member.regenerate_notifications(Date.today + 1.year) + test "first warning notification 60~30 days before expiration" do + (@member.expires_on - 60.days .. (@member.expires_on - 31.days)).each do |day| + Timecop.freeze(day) do + assert_equal "expiration_in_60d", @member.todays_notification[:template] + end + end + end - assert_equal 4, @member.notifications.count + test "no duplicate notification" do + @member + .notifications + .create(template: "expiration_in_60d", to_be_sent_on: @member.expires_on - 60.days, status: "sent", sent_at: @member.expires_on - 60.days) + + Timecop.freeze(@member.expires_on - 60.days) do + assert_nil @member.todays_notification + end + end + + test "don't consider old notifications as duplicate" do + @member + .notifications + .create(template: "expiration_in_60d", to_be_sent_on: @member.expires_on - 60.days - 1.year, status: "sent", sent_at: @member.expires_on - 60.days - 1.year) + + Timecop.freeze(@member.expires_on - 60.days) do + assert_equal "expiration_in_60d", @member.todays_notification[:template] + end + end + + test "don't consider unsent notifications as duplicate" do + @member + .notifications + .create(template: "expiration_in_60d", to_be_sent_on: @member.expires_on - 60.days, status: "sent", sent_at: nil) + + Timecop.freeze(@member.expires_on - 60.days) do + assert_equal "expiration_in_60d", @member.todays_notification[:template] + end + end + + test "last notification ~90 days after expiration" do + Timecop.freeze(@member.expires_on + 90.days) do + assert_equal "cancelled", @member.todays_notification[:template] + end + end + + test "last notification ~95 days after expiration" do + Timecop.freeze(@member.expires_on + 95.days) do + assert_equal "cancelled", @member.todays_notification[:template] + end + end + + test "no duplicate last notification" do + @member + .notifications + .create(template: "cancelled", to_be_sent_on: @member.expires_on + 90.days, status: "sent", sent_at: @member.expires_on + 90.days) + + Timecop.freeze(@member.expires_on + 95.days) do + assert_nil @member.todays_notification + end end end