[#2733/#8331] Prevent request classification notifications

app/mailers/application_mailer.rb

@@ -20,6 +20,9 @@ class ApplicationMailer < ActionMailer::Base
   # Site-wide access to configuration settings
   include ConfigHelper
 
+  # Check user can receive the emails by wrapping `mail_user`.
+  prepend CheckMailerAbility
+
   # This really should be the default - otherwise you lose any information
   # about the errors, and have to do error checking on return codes.
   self.raise_delivery_errors = true

app/mailers/concerns/check_mailer_ability.rb

@@ -0,0 +1,44 @@
+##
+# This module provides email sending ability control for mailers. It allows
+# mailers to check whether email delivery is permitted based on user
+# permissions or other criteria.
+#
+# Key functionality:
+# - Wraps mail sending with permission checking
+# - Extracts mailer name and action for permission lookup
+# - Builds ability instance with mailer context variables
+#
+module CheckMailerAbility
+  extend ActiveSupport::Concern
+
+  def mail_user(user, **headers)
+    @user = user
+
+    mail = super
+    mail.perform_deliveries = can_send?
+    mail
+  end
+
+  def can_send?
+    ability.can?(:receive, name)
+  end
+
+  def name
+    [mailer_name.underscore, action_name].join('#')
+  end
+
+  def ability
+    @ability ||= MailerAbility.new(@user, **variables)
+  end
+
+  private
+
+  def variables
+    instance_variables.inject({}) do |hash, var|
+      next hash if var.to_s.starts_with?('@_')
+
+      hash[var.to_s.delete('@').to_sym] = instance_variable_get(var)
+      hash
+    end
+  end
+end

app/mailers/mailer_ability.rb

@@ -0,0 +1,25 @@
+##
+# This ability model enforces permission rules for mailer functions.
+#
+class MailerAbility
+  include CanCan::Ability
+
+  attr_reader :user, :params
+
+  def initialize(user, **params)
+    @user = user
+    @params = params
+
+    can :receive, :all
+
+    cannot :receive, 'request_mailer#old_unclassified_updated' do
+      info_request.created_at <= 6.months.ago
+    end
+  end
+
+  private
+
+  def info_request
+    params[:info_request]
+  end
+end

doc/CHANGES.md

@@ -2,7 +2,9 @@
 
 ## Highlighted Features
 
-Add additional InfoRequest embargo scopes (Graeme Porteous)
+* Prevent request classification notifications from being set if request is
+  older than 6 months (Graeme Porteous)
+* Add additional InfoRequest embargo scopes (Graeme Porteous)
 
 # 0.45.3.1
 

spec/mailers/application_mailer_spec.rb

@@ -149,4 +149,35 @@ def create_multipart_method(method_name)
       remove_mail_methods(%w[simple theme_only core_only neither multipart])
     end
   end
+
+  context 'user mailer abilities' do
+    let(:mailer) do
+      mailer_klass = Class.new(ApplicationMailer) do
+        def my_message(user)
+          mail_user(user, subject: -> { 'My subject' }, body: 'My body')
+        end
+      end
+      mailer_klass.new
+    end
+
+    let(:user) { FactoryBot.create(:user) }
+    let(:mail) { mailer.my_message(user) }
+    let(:ability) { Object.new.extend(CanCan::Ability) }
+
+    before do
+      allow(mailer).to receive(:ability).and_return(ability)
+      allow(mailer).to receive(:action_name).and_return('my_message')
+
+      ability.can :receive, :all
+    end
+
+    it 'should perform deliveries by default' do
+      expect(mail.perform_deliveries).to eq true
+    end
+
+    it 'should be able to not perform deliveries' do
+      ability.cannot :receive, 'anonymous#my_message'
+      expect(mail.perform_deliveries).to eq false
+    end
+  end
 end

spec/mailers/mailer_ability_spec.rb

@@ -0,0 +1,147 @@
+require 'spec_helper'
+require "cancan/matchers"
+
+RSpec.describe MailerAbility do
+  let(:user) { FactoryBot.build(:user) }
+  let(:ability) { MailerAbility.new(user) }
+
+  describe 'EmbargoMailer#expiring_alert' do
+    let(:name) { 'embargo_mailer#expiring_alert' }
+    it { expect(ability).to be_able_to(:receive, name) }
+  end
+
+  describe 'EmbargoMailer#expired_alert' do
+    let(:name) { 'embargo_mailer#expired_alert' }
+    it { expect(ability).to be_able_to(:receive, name) }
+  end
+
+  describe 'SubscriptionMailer#payment_failed' do
+    let(:name) { 'subscription_mailer#payment_failed' }
+    it { expect(ability).to be_able_to(:receive, name) }
+  end
+
+  describe 'ContactMailer#user_message' do
+    let(:name) { 'contact_mailer#user_message' }
+    it { expect(ability).to be_able_to(:receive, name) }
+  end
+
+  describe 'ContactMailer#from_admin_message' do
+    let(:name) { 'contact_mailer#from_admin_message' }
+    it { expect(ability).to be_able_to(:receive, name) }
+  end
+
+  describe 'InfoRequestBatchMailer#batch_sent' do
+    let(:name) { 'info_request_batch_mailer#batch_sent' }
+    it { expect(ability).to be_able_to(:receive, name) }
+  end
+
+  describe 'NotificationMailer#daily_summary' do
+    let(:name) { 'notification_mailer#daily_summary' }
+    it { expect(ability).to be_able_to(:receive, name) }
+  end
+
+  describe 'NotificationMailer#response_notification' do
+    let(:name) { 'notification_mailer#response_notification' }
+    it { expect(ability).to be_able_to(:receive, name) }
+  end
+
+  describe 'NotificationMailer#embargo_expiring_notification' do
+    let(:name) { 'notification_mailer#embargo_expiring_notification' }
+    it { expect(ability).to be_able_to(:receive, name) }
+  end
+
+  describe 'NotificationMailer#expire_embargo_notification' do
+    let(:name) { 'notification_mailer#expire_embargo_notification' }
+    it { expect(ability).to be_able_to(:receive, name) }
+  end
+
+  describe 'NotificationMailer#overdue_notification' do
+    let(:name) { 'notification_mailer#overdue_notification' }
+    it { expect(ability).to be_able_to(:receive, name) }
+  end
+
+  describe 'NotificationMailer#very_overdue_notification' do
+    let(:name) { 'notification_mailer#very_overdue_notification' }
+    it { expect(ability).to be_able_to(:receive, name) }
+  end
+
+  describe 'RequestMailer#new_response' do
+    let(:name) { 'request_mailer#new_response' }
+    it { expect(ability).to be_able_to(:receive, name) }
+  end
+
+  describe 'RequestMailer#overdue_alert' do
+    let(:name) { 'request_mailer#overdue_alert' }
+    it { expect(ability).to be_able_to(:receive, name) }
+  end
+
+  describe 'RequestMailer#very_overdue_alert' do
+    let(:name) { 'request_mailer#very_overdue_alert' }
+    it { expect(ability).to be_able_to(:receive, name) }
+  end
+
+  describe 'RequestMailer#new_response_reminder_alert' do
+    let(:name) { 'request_mailer#new_response_reminder_alert' }
+    it { expect(ability).to be_able_to(:receive, name) }
+  end
+
+  describe 'RequestMailer#old_unclassified_updated' do
+    let(:name) { 'request_mailer#old_unclassified_updated' }
+    let(:ability) { MailerAbility.new(user, info_request: info_request) }
+
+    context 'when info request when sent less than 6 months ago' do
+      let(:info_request) { double(:InfoRequest, created_at: 6.months.ago + 1) }
+      it { expect(ability).to be_able_to(:receive, name) }
+    end
+
+    context 'when info request when sent more than 6 months ago' do
+      let(:info_request) { double(:InfoRequest, created_at: 6.months.ago) }
+      it { expect(ability).not_to be_able_to(:receive, name) }
+    end
+  end
+
+  describe 'RequestMailer#not_clarified_alert' do
+    let(:name) { 'request_mailer#not_clarified_alert' }
+    it { expect(ability).to be_able_to(:receive, name) }
+  end
+
+  describe 'RequestMailer#comment_on_alert' do
+    let(:name) { 'request_mailer#comment_on_alert' }
+    it { expect(ability).to be_able_to(:receive, name) }
+  end
+
+  describe 'RequestMailer#comment_on_alert_plural' do
+    let(:name) { 'request_mailer#comment_on_alert_plural' }
+    it { expect(ability).to be_able_to(:receive, name) }
+  end
+
+  describe 'SurveyMailer#survey_alert' do
+    let(:name) { 'survey_mailer#survey_alert' }
+    it { expect(ability).to be_able_to(:receive, name) }
+  end
+
+  describe 'TrackMailer#event_digest' do
+    let(:name) { 'track_mailer#event_digest' }
+    it { expect(ability).to be_able_to(:receive, name) }
+  end
+
+  describe 'UserMailer#confirm_login' do
+    let(:name) { 'user_mailer#confirm_login' }
+    it { expect(ability).to be_able_to(:receive, name) }
+  end
+
+  describe 'UserMailer#already_registered' do
+    let(:name) { 'user_mailer#already_registered' }
+    it { expect(ability).to be_able_to(:receive, name) }
+  end
+
+  describe 'UserMailer#changeemail_confirm' do
+    let(:name) { 'user_mailer#changeemail_confirm' }
+    it { expect(ability).to be_able_to(:receive, name) }
+  end
+
+  describe 'UserMailer#changeemail_already_used' do
+    let(:name) { 'user_mailer#changeemail_already_used' }
+    it { expect(ability).to be_able_to(:receive, name) }
+  end
+end

spec/mailers/previews/request_mailer_preview.rb

@@ -43,7 +43,8 @@ def info_request
       url_title: 'a_request',
       user: User.first,
       public_body: PublicBody.first,
-      described_state: 'successful'
+      described_state: 'successful',
+      created_at: Time.now
     )
   end
 

spec/mailers/request_mailer_spec.rb

@@ -542,6 +542,11 @@ def sent_alert_params(request, type)
       expect(mail.subject).to eq('Someone has updated the status of your request')
     end
 
+    it 'delivers the email' do
+      mail.deliver_now
+      expect(ActionMailer::Base.deliveries).to_not be_empty
+    end
+
     context "when the user does not use default locale" do
       before do
         info_request.user.locale = 'es'
@@ -554,6 +559,21 @@ def sent_alert_params(request, type)
       end
     end
 
+    context 'when the info request was created over 6 months ago' do
+      let(:info_request) do
+        FactoryBot.create(
+          :info_request,
+          user: user, title: "Test request", public_body: public_body,
+          url_title: "test_request", created_at: 6.months.ago
+        )
+      end
+
+      it 'does not deliver the email' do
+        mail.deliver_now
+        expect(ActionMailer::Base.deliveries).to be_empty
+      end
+    end
+
     it 'should tell them what status was picked' do
       expect(mail.body).to match(/"refused."/)
     end