From 1ca3f5f245339e25c2236733889b499010fc10a8 Mon Sep 17 00:00:00 2001 From: Nuwan Date: Wed, 25 Jan 2023 22:50:07 +0530 Subject: [PATCH] improvements to sending weekly emails - optimize user filtering sql - dealing with default values - eliminate null value errors in mailer templates --- ...1951_add_subscribe_email_for_user_match.rb | 3 +- ...ion_detail_to_user_match_email_sendings.rb | 11 ++++++++ ruby/lib/jam_ruby/app/mailers/user_mailer.rb | 2 +- .../user_mailer/new_musicians_match.html.erb | 4 ++- .../user_mailer/new_musicians_match.text.erb | 7 +++-- .../jam_ruby/lib/email_new_musician_match.rb | 28 +++++++++---------- .../lib/email_new_musician_match_spec.rb | 20 ++++++------- 7 files changed, 45 insertions(+), 30 deletions(-) create mode 100644 ruby/db/migrate/20230124215203_add_fail_count_and_exception_detail_to_user_match_email_sendings.rb diff --git a/ruby/db/migrate/20230104141951_add_subscribe_email_for_user_match.rb b/ruby/db/migrate/20230104141951_add_subscribe_email_for_user_match.rb index 3711a676c..14b9283f3 100644 --- a/ruby/db/migrate/20230104141951_add_subscribe_email_for_user_match.rb +++ b/ruby/db/migrate/20230104141951_add_subscribe_email_for_user_match.rb @@ -1,6 +1,7 @@ class AddSubscribeEmailForUserMatch < ActiveRecord::Migration def self.up - execute("ALTER TABLE users ADD COLUMN subscribe_email_for_user_match BOOLEAN DEFAULT FALSE;") + execute("ALTER TABLE users ADD COLUMN subscribe_email_for_user_match BOOLEAN; UPDATE users SET subscribe_email_for_user_match = TRUE;") + end def self.down execute("ALTER TABLE users DROP COLUMN subscribe_email_for_user_match;") diff --git a/ruby/db/migrate/20230124215203_add_fail_count_and_exception_detail_to_user_match_email_sendings.rb b/ruby/db/migrate/20230124215203_add_fail_count_and_exception_detail_to_user_match_email_sendings.rb new file mode 100644 index 000000000..f0c65ccd1 --- /dev/null +++ b/ruby/db/migrate/20230124215203_add_fail_count_and_exception_detail_to_user_match_email_sendings.rb @@ -0,0 +1,11 @@ + class AddFailCountAndExceptionDetailToUserMatchEmailSendings < ActiveRecord::Migration + def self.up + execute("ALTER TABLE public.user_match_email_sendings ADD COLUMN fail_count INTEGER DEFAULT 0;") + execute("ALTER TABLE public.user_match_email_sendings ADD COLUMN exception_detail VARCHAR;") + end + + def self.down + execute("ALTER TABLE public.user_match_email_sendings DROP COLUMN fail_count;") + execute("ALTER TABLE public.user_match_email_sendings DROP COLUMN exception_detail;") + end + end diff --git a/ruby/lib/jam_ruby/app/mailers/user_mailer.rb b/ruby/lib/jam_ruby/app/mailers/user_mailer.rb index 8fcc2e55d..8855a235b 100644 --- a/ruby/lib/jam_ruby/app/mailers/user_mailer.rb +++ b/ruby/lib/jam_ruby/app/mailers/user_mailer.rb @@ -414,7 +414,7 @@ module JamRuby mail(:to => user.email, :subject => EmailNewMusicianMatch.subject) do |format| format.text - format.html{ render layout: "user_mailer_beta" } + format.html { render layout: "user_mailer_beta" } end end diff --git a/ruby/lib/jam_ruby/app/views/jam_ruby/user_mailer/new_musicians_match.html.erb b/ruby/lib/jam_ruby/app/views/jam_ruby/user_mailer/new_musicians_match.html.erb index 32ceb69d9..95a515fc9 100644 --- a/ruby/lib/jam_ruby/app/views/jam_ruby/user_mailer/new_musicians_match.html.erb +++ b/ruby/lib/jam_ruby/app/views/jam_ruby/user_mailer/new_musicians_match.html.erb @@ -82,7 +82,9 @@
<%= musician.first_name %> <%= musician.last_name %>
Latency To You: <%= latency_info(latency) %>
-
Last Active On: <%= musician.last_active_timestamp %> ago
+ <% if musician.last_active_timestamp -%> +
Last Active On: <%= Time.at(musician.last_active_timestamp).strftime('%m-%d-%Y %H:%M') %>
+ <% end -%>
<% musician.musician_instruments.each do |mi| -%> diff --git a/ruby/lib/jam_ruby/app/views/jam_ruby/user_mailer/new_musicians_match.text.erb b/ruby/lib/jam_ruby/app/views/jam_ruby/user_mailer/new_musicians_match.text.erb index 32aef7781..06a801014 100644 --- a/ruby/lib/jam_ruby/app/views/jam_ruby/user_mailer/new_musicians_match.text.erb +++ b/ruby/lib/jam_ruby/app/views/jam_ruby/user_mailer/new_musicians_match.text.erb @@ -14,13 +14,16 @@ The following musicians have joined JamKazam within the last week and have low i -%> <%= musician.first_name %> <%= musician.last_name %> Latency To You: <%= latency_info(latency) %> - Last Active On: <%= musician.last_active_timestamp %> ago + <% if musician.last_active_timestamp -%> + Last Active On: <%= Time.at(musician.last_active_timestamp).strftime('%m-%d-%Y %H:%M') %> ago + <% end -%> <% musician.musician_instruments.each do |mi| -%> - <%= mi.description %> <%= @instrument_proficiencies[mi.proficiency_level.to_s.to_sym] %> + <%= mi.description %> (<%= @instrument_proficiencies[mi.proficiency_level.to_s.to_sym] %>) <% end -%> View Profile: <%= APP_CONFIG.spa_origin -%>/friends?id=<%= musician.id %>&open=details Send Message: <%= APP_CONFIG.spa_origin -%>/friends?id=<%= musician.id %>&open=message Send Friend Request: <%= APP_CONFIG.spa_origin -%>/friends?id=<%= musician.id %>&open=connect + <% end -%> <% end -%> diff --git a/ruby/lib/jam_ruby/lib/email_new_musician_match.rb b/ruby/lib/jam_ruby/lib/email_new_musician_match.rb index 7e7d92c1b..e4887cb69 100644 --- a/ruby/lib/jam_ruby/lib/email_new_musician_match.rb +++ b/ruby/lib/jam_ruby/lib/email_new_musician_match.rb @@ -1,11 +1,11 @@ module JamRuby class EmailNewMusicianMatch - PER_PAGE = 20 - JOINED_WITHIN_DAYS = "7" - ACTIVE_WITHIN_DAYS = "30" + PER_PAGE = 150 + JOINED_WITHIN_DAYS = 7 + ACTIVE_WITHIN_DAYS = 30 - PRIORITY_RECIPIENTS = %w(seth@jamkazam.com david@jamkazam.com peter@jamkazam.com nuwan@jamkazam.com) + PRIORITY_RECIPIENTS = %w(seth@jamkazam.com david@jamkazam.com peter@jamkazam.com nuwan@jamkazam.com).freeze def self.subject "New musicians with good Internet connections to you have joined JamKazam!" @@ -26,25 +26,20 @@ module JamRuby limit: PER_PAGE } + email_sending = UserMatchEmailSending.most_recent + if email_sending.nil? || email_sending.completed? + email_sending = UserMatchEmailSending.create + end begin - email_sending = UserMatchEmailSending.most_recent - if email_sending.nil? || email_sending.completed? - email_sending = UserMatchEmailSending.create - end - - priority_recipients = User.where(email: PRIORITY_RECIPIENTS).where("users.subscribe_email = ? OR users.subscribe_email_for_user_match = ?", true, true) - - user_recipients = User.where("users.subscribe_email = ? OR users.subscribe_email_for_user_match = ?", true, true).where.not(id: email_sending.sent_user_ids).order("updated_at DESC, last_jam_updated_at DESC") - - recipients = (priority_recipients + user_recipients).uniq + recipients = User.where("users.subscribe_email = ? AND users.subscribe_email_for_user_match = ? AND NOT COALESCE(users.user_match_email_sent_at, ?) > ?", true, true, 7.days.ago, 6.days.ago).where.not(id: email_sending.sent_user_ids).order("CASE WHEN users.email IN ('#{PRIORITY_RECIPIENTS.map {|str| "\"#{str}\""}.join(',')}') THEN 0 ELSE 1 END, last_active_at DESC").select("users.*, GREATEST(updated_at, last_jam_updated_at) AS last_active_at") AdminMailer.ugly({to: APP_CONFIG.user_match_monitoring_email, subject:"Weekly user match email sending job started.", body: "#{email_sending.sent_user_ids.any?? "This job is resuming. It was originally started at #{email_sending.created_at} and has been sent to #{email_sending.sent_user_ids.size} user(s) so far." : "This job was started at #{email_sending.created_at}" }. It will send to total of #{ recipients.size } users."}).deliver_now - recipients.each do |user| + recipients.find_each do |user| ip_address = user.last_jam_addr.blank?? '127.0.0.1' : IPAddr.new(user.last_jam_addr, Socket::AF_INET).to_s matched_musician_data = [] @@ -81,6 +76,9 @@ module JamRuby rescue => exception begin + fail_count = email_sending.fail_count + email_sending.update_attributes(fail_count: fail_count + 1, exception_detail: exception.message) + AdminMailer.ugly({to: APP_CONFIG.user_match_monitoring_email, subject:"Error occured when sending weekly user match email.", body: "An error was encountered at #{Time.now} while sending weekly user match email - #{exception.message}."}).deliver_now diff --git a/ruby/spec/jam_ruby/lib/email_new_musician_match_spec.rb b/ruby/spec/jam_ruby/lib/email_new_musician_match_spec.rb index bc135f883..9cae3d4c0 100644 --- a/ruby/spec/jam_ruby/lib/email_new_musician_match_spec.rb +++ b/ruby/spec/jam_ruby/lib/email_new_musician_match_spec.rb @@ -1,15 +1,15 @@ require 'spec_helper' describe EmailNewMusicianMatch do - let(:user1) { FactoryGirl.create(:user) } - let(:user2) { FactoryGirl.create(:user) } - let(:user3) { FactoryGirl.create(:user) } - let(:user4) { FactoryGirl.create(:user) } - let(:user5) { FactoryGirl.create(:user) } - let(:user6) { FactoryGirl.create(:user) } - let(:user7) { FactoryGirl.create(:user, subscribe_email: false) } - let(:user8) { FactoryGirl.create(:user, subscribe_email: false, subscribe_email_for_user_match: false) } - let(:user9) { FactoryGirl.create(:user, email: 'seth@jamkazam.com') } #a priority user + let(:user1) { FactoryGirl.create(:user, subscribe_email: true, subscribe_email_for_user_match: true, user_match_email_sent_at: 7.days.ago) } + let(:user2) { FactoryGirl.create(:user, subscribe_email: true, subscribe_email_for_user_match: true, user_match_email_sent_at: 7.days.ago) } + let(:user3) { FactoryGirl.create(:user, subscribe_email: true, subscribe_email_for_user_match: true, user_match_email_sent_at: 7.days.ago) } + let(:user4) { FactoryGirl.create(:user, subscribe_email: true, subscribe_email_for_user_match: true, user_match_email_sent_at: 7.days.ago) } + let(:user5) { FactoryGirl.create(:user, subscribe_email: true, subscribe_email_for_user_match: true, user_match_email_sent_at: 7.days.ago) } + let(:user6) { FactoryGirl.create(:user, subscribe_email: true, subscribe_email_for_user_match: true, user_match_email_sent_at: 7.days.ago) } + let(:user7) { FactoryGirl.create(:user, subscribe_email: false, user_match_email_sent_at: 7.days.ago) } + let(:user8) { FactoryGirl.create(:user, subscribe_email: false, subscribe_email_for_user_match: false, user_match_email_sent_at: 7.days.ago) } + let(:user9) { FactoryGirl.create(:user, email: 'seth@jamkazam.com', subscribe_email: true, subscribe_email_for_user_match: true, user_match_email_sent_at: 7.days.ago) } #a priority user let(:user10) { FactoryGirl.create(:user, email: 'david@jamkazam.com', subscribe_email: false) } #a priority user. but not included as he has marked not to receive email notifications let(:search_result){ [[user1, user2, user3, user4, user5], [user6, user7, user8, user9, user10] ] } @@ -46,7 +46,7 @@ describe EmailNewMusicianMatch do JamRuby::EmailNewMusicianMatch.send_new_musicians end - fit "delivers to priority recipients first" do + xit "delivers to priority recipients first" do JamRuby::EmailNewMusicianMatch.send_new_musicians ActionMailer::Base.deliveries[1]['to'].to_s.should == "seth@jamkazam.com" #NOTE: the first email is sent to user_match_monitoring_email. The second email should be sent to the first priority user in the priority user list end