diff --git a/app/jobs/batch_email_notification_job.rb b/app/jobs/batch_email_notification_job.rb index 7164bb452..67de2a21a 100644 --- a/app/jobs/batch_email_notification_job.rb +++ b/app/jobs/batch_email_notification_job.rb @@ -25,7 +25,7 @@ def perform end end - user.update(last_emailed_at: Time.current) + user.last_emailed_at = Time.current end BatchEmailNotificationJob.set(wait_until: Date.tomorrow.midnight).perform_later end diff --git a/app/models/sushi.rb b/app/models/sushi.rb index 7df08af90..1267c6f81 100644 --- a/app/models/sushi.rb +++ b/app/models/sushi.rb @@ -204,7 +204,9 @@ def coerce_dates(params = {}) raise Sushi::Error::UsageNotReadyForRequestedDatesError.new(data: "Unable to complete the request because the end_date of #{params[:end_date]} is for a month that has incomplete data. That month's data ends on #{latest_date.iso8601}.") # rubocop:enable Metrics/MethodLength end + # rubocop:disable Lint/ShadowedException rescue ActionController::ParameterMissing, KeyError => e + # rubocop:enable Lint/ShadowedException raise Sushi::Error::InsufficientInformationToProcessRequestError.new(data: e.message) end end diff --git a/app/models/user.rb b/app/models/user.rb index a4a623c98..1d402c369 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -2,6 +2,8 @@ # rubocop:disable Metrics/ClassLength class User < ApplicationRecord + has_one :user_batch_email, dependent: :destroy + # Includes lib/rolify from the rolify gem rolify # Connects this user object to Hydra behaviors. @@ -189,5 +191,13 @@ def statistics_for(start_date: (Time.zone.now - 1.month).beginning_of_month, end total_work_views: } end + + def last_emailed_at + UserBatchEmail.find_or_create_by(user: self).last_emailed_at + end + + def last_emailed_at=(value) + UserBatchEmail.find_or_create_by(user: self).update(last_emailed_at: value) + end end # rubocop:enable Metrics/ClassLength diff --git a/app/models/user_batch_email.rb b/app/models/user_batch_email.rb new file mode 100644 index 000000000..c71863a26 --- /dev/null +++ b/app/models/user_batch_email.rb @@ -0,0 +1,6 @@ +# frozen_string_literal:true + +class UserBatchEmail < ApplicationRecord + self.table_name = 'user_batch_emails' + belongs_to :user, class_name: '::User' +end diff --git a/db/migrate/20240820195641_remove_last_emailed_at_from_users.rb b/db/migrate/20240820195641_remove_last_emailed_at_from_users.rb new file mode 100644 index 000000000..1534368c7 --- /dev/null +++ b/db/migrate/20240820195641_remove_last_emailed_at_from_users.rb @@ -0,0 +1,5 @@ +class RemoveLastEmailedAtFromUsers < ActiveRecord::Migration[6.1] + def change + remove_column :users, :last_emailed_at, :datetime if column_exists?(:users, :last_emailed_at) + end +end diff --git a/db/migrate/20240820200440_create_user_batch_emails.rb b/db/migrate/20240820200440_create_user_batch_emails.rb new file mode 100644 index 000000000..fe5dd9cba --- /dev/null +++ b/db/migrate/20240820200440_create_user_batch_emails.rb @@ -0,0 +1,10 @@ +class CreateUserBatchEmails < ActiveRecord::Migration[6.1] + def change + create_table :user_batch_emails do |t| + t.references :user, unique: true, index: true + t.datetime :last_emailed_at + + t.timestamps + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 488e8ac6a..cdced2bad 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2024_08_06_161142) do +ActiveRecord::Schema.define(version: 2024_08_20_200440) do # These are extensions that must be enabled in order to support this database enable_extension "hstore" @@ -856,6 +856,14 @@ t.index ["user_id"], name: "index_uploaded_files_on_user_id" end + create_table "user_batch_emails", force: :cascade do |t| + t.bigint "user_id" + t.datetime "last_emailed_at" + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + t.index ["user_id"], name: "index_user_batch_emails_on_user_id" + end + create_table "user_stats", id: :serial, force: :cascade do |t| t.integer "user_id" t.datetime "date" @@ -917,7 +925,6 @@ t.string "provider" t.string "uid" t.string "batch_email_frequency", default: "never" - t.datetime "last_emailed_at" t.string "api_key" t.index ["api_key"], name: "index_users_on_api_key" t.index ["email"], name: "index_users_on_email", unique: true diff --git a/spec/jobs/batch_email_notification_job_spec.rb b/spec/jobs/batch_email_notification_job_spec.rb index 8e859c41e..db2ae6cf6 100644 --- a/spec/jobs/batch_email_notification_job_spec.rb +++ b/spec/jobs/batch_email_notification_job_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true RSpec.describe BatchEmailNotificationJob do + let(:subject) { BatchEmailNotificationJob.perform_now } let(:account) { create(:account_with_public_schema) } let(:receipt) { FactoryBot.create(:mailboxer_receipt, receiver: user) } let!(:message) { receipt.notification } @@ -17,62 +18,70 @@ end describe '#perform' do - let(:frequency) { 'daily' } - - it 'marks the message as delivered' do - expect { BatchEmailNotificationJob.perform_now }.to change { message.receipts.first.is_delivered }.from(false).to(true) + before do + UserBatchEmail.find_or_create_by(user: user).update(last_emailed_at: last_emailed) end - it 're-enqueues the job' do - expect { BatchEmailNotificationJob.perform_now }.to have_enqueued_job(BatchEmailNotificationJob) + context 'basic job behavior' do + let(:frequency) { 'daily' } + let(:last_emailed) { nil } + + it 'marks the message as delivered' do + expect { subject }.to change { message.receipts.first.is_delivered }.from(false).to(true) + end + + it 're-enqueues the job' do + expect { subject }.to have_enqueued_job(BatchEmailNotificationJob) + end end context 'when the user has a daily frequency' do let(:frequency) { 'daily' } + let(:last_emailed) { 1.day.ago } it 'sends email to users with batch_email_frequency set' do - expect { BatchEmailNotificationJob.perform_now }.to change { ActionMailer::Base.deliveries.count }.by(1) + expect { subject }.to change { ActionMailer::Base.deliveries.count }.by(1) end end context 'when the user has a weekly frequency' do let(:frequency) { 'weekly' } - let(:user) { FactoryBot.create(:user, batch_email_frequency: frequency, last_emailed_at:) } + let(:user) { FactoryBot.create(:user, batch_email_frequency: frequency) } context 'when the user was last emailed less than a week ago' do - let(:last_emailed_at) { 5.days.ago } + let(:last_emailed) { 5.days.ago } it 'does not send an email to users with batch_email_frequency set' do - expect { BatchEmailNotificationJob.perform_now }.to_not change { ActionMailer::Base.deliveries.count } + expect { subject }.to_not change { ActionMailer::Base.deliveries.count } end end context 'when the user was last emailed more than a week ago' do - let(:last_emailed_at) { 8.days.ago } + let(:last_emailed) { 8.days.ago } it 'sends email to users with batch_email_frequency set' do - expect { BatchEmailNotificationJob.perform_now }.to change { ActionMailer::Base.deliveries.count }.by(1) + expect { subject }.to change { ActionMailer::Base.deliveries.count }.by(1) end end end context 'when the user has a monthly frequency' do let(:frequency) { 'monthly' } - let(:user) { FactoryBot.create(:user, batch_email_frequency: frequency, last_emailed_at:) } + let(:user) { FactoryBot.create(:user, batch_email_frequency: frequency) } context 'when the user was last emailed less than a month ago' do - let(:last_emailed_at) { 20.days.ago } + let(:last_emailed) { 20.days.ago } it 'does not send an email to users with batch_email_frequency set' do - expect { BatchEmailNotificationJob.perform_now(account) }.to_not change { ActionMailer::Base.deliveries.count } + expect { subject }.to_not change { ActionMailer::Base.deliveries.count } end end context 'when the user was last emailed more than a month ago' do - let(:last_emailed_at) { 40.days.ago } + let(:last_emailed) { 40.days.ago } it 'sends email to users with batch_email_frequency set' do - expect { BatchEmailNotificationJob.perform_now }.to change { ActionMailer::Base.deliveries.count }.by(1) + expect { subject }.to change { ActionMailer::Base.deliveries.count }.by(1) end end end