Improve logic in Idv::UspsMail#mail_spammed?

**Why**: If the `max_mail_events` Figaro key is not set on the server,
or if it's set to zero, and if the user has not requested any letters,
`max_events?` will return `true`, which will then cause
`updated_within_last_month?` to throw a NoMethod error because it can't
call `update_at` on a nil object.

**How**:
- Return `false` early in `mail_spammed?` if the user has not requested
any letters.
- Add `max_mail_events` and `max_mail_events_window_in_days` to the list
of required Figaro keys.
- Set `max_mail_events` to 2 in the test environment to avoid creating
more entries in the DB than is necessary.
- Use `#size` instead of `#count` for performance reasons.

app/services/idv/usps_mail.rb

@@ -8,6 +8,7 @@ def initialize(current_user)
     end
 
     def mail_spammed?
+      return false if user_mail_events.empty?
       max_events? && updated_within_last_month?
     end
 
@@ -23,7 +24,7 @@ def user_mail_events
     end
 
     def max_events?
-      user_mail_events.count == MAX_MAIL_EVENTS
+      user_mail_events.size == MAX_MAIL_EVENTS
     end
 
     def updated_within_last_month?

config/application.yml.example

@@ -180,6 +180,7 @@ test:
   idp_sso_target_url: 'http://identityprovider.example.com/saml/auth'
   logins_per_ip_limit: '2'
   logins_per_ip_period: '60'
+  max_mail_events: '2'
   newrelic_license_key: 'xxx'
   otp_delivery_blocklist_bantime: '1'
   otp_delivery_blocklist_findtime: '1'

config/initializers/figaro.rb

@@ -11,6 +11,8 @@
   'idp_sso_target_url',
   'logins_per_ip_limit',
   'logins_per_ip_period',
+  'max_mail_events',
+  'max_mail_events_window_in_days',
   'min_password_score',
   'otp_delivery_blocklist_bantime',
   'otp_delivery_blocklist_findtime',

spec/controllers/verify/review_controller_spec.rb

@@ -188,48 +188,32 @@ def show
     context 'user has not requested too much mail' do
       before do
         idv_session.address_verification_mechanism = 'usps'
-        Event.create(event_type: :usps_mail_sent, user: user, updated_at: 2.months.ago)
-        Event.create(event_type: :usps_mail_sent, user: user, updated_at: 1.week.ago)
-        Event.create(event_type: :usps_mail_sent, user: user, updated_at: 1.day.ago)
-        Event.create(event_type: :usps_mail_sent, user: user, updated_at: 1.hour.ago)
+        usps_mail_service = instance_double(Idv::UspsMail)
+        allow(Idv::UspsMail).to receive(:new).with(user).and_return(usps_mail_service)
+        allow(usps_mail_service).to receive(:mail_spammed?).and_return(false)
       end
 
-      it 'does not display a success message' do
-        get :new
-
-        expect(flash.now[:success]).to eq(
-          t('idv.messages.mail_sent')
-        )
-      end
-
-      it 'displays a helpful error message' do
+      it 'displays a success message' do
         get :new
 
+        expect(flash.now[:success]).to eq t('idv.messages.mail_sent')
         expect(flash.now[:error]).to be_nil
       end
     end
 
     context 'user has requested too much mail' do
       before do
         idv_session.address_verification_mechanism = 'usps'
-        Event.create(event_type: :usps_mail_sent, user: user, updated_at: 2.weeks.ago)
-        Event.create(event_type: :usps_mail_sent, user: user, updated_at: 1.week.ago)
-        Event.create(event_type: :usps_mail_sent, user: user, updated_at: 1.day.ago)
-        Event.create(event_type: :usps_mail_sent, user: user, updated_at: 1.hour.ago)
-      end
-
-      it 'does not display a success message' do
-        get :new
-
-        expect(flash.now[:success]).to be_nil
+        usps_mail_service = instance_double(Idv::UspsMail)
+        allow(Idv::UspsMail).to receive(:new).with(user).and_return(usps_mail_service)
+        allow(usps_mail_service).to receive(:mail_spammed?).and_return(true)
       end
 
       it 'displays a helpful error message' do
         get :new
 
-        expect(flash.now[:error]).to eq(
-          t('idv.errors.mail_limit_reached')
-        )
+        expect(flash.now[:error]).to eq t('idv.errors.mail_limit_reached')
+        expect(flash.now[:success]).to be_nil
       end
     end
   end

spec/services/idv/usps_mail_spec.rb

@@ -6,26 +6,39 @@
 
   describe '#mail_spammed?' do
     context 'when no mail has been sent' do
-      it 'is never spammed' do
+      it 'returns false' do
+        expect(subject.mail_spammed?).to eq false
+      end
+    end
+
+    context 'when the amount of sent mail is lower than the allowed maximum' do
+      it 'returns false' do
+        Event.create(event_type: :usps_mail_sent, user: user)
+
         expect(subject.mail_spammed?).to eq false
       end
     end
 
     context 'when too much mail has been sent' do
-      it 'is spammed if all the updates have been within the last month' do
+      it 'returns true if the oldest event was within the last month' do
         Event.create(event_type: :usps_mail_sent, user: user, updated_at: 2.weeks.ago)
         Event.create(event_type: :usps_mail_sent, user: user, updated_at: 1.week.ago)
-        Event.create(event_type: :usps_mail_sent, user: user, updated_at: 1.day.ago)
-        Event.create(event_type: :usps_mail_sent, user: user, updated_at: 1.hour.ago)
 
         expect(subject.mail_spammed?).to eq true
       end
 
-      it 'is not spammed if the most distant update was more than a month ago' do
+      it 'returns false if the oldest event was more than a month ago' do
+        Event.create(event_type: :usps_mail_sent, user: user, updated_at: 2.weeks.ago)
         Event.create(event_type: :usps_mail_sent, user: user, updated_at: 2.months.ago)
-        Event.create(event_type: :usps_mail_sent, user: user, updated_at: 1.week.ago)
-        Event.create(event_type: :usps_mail_sent, user: user, updated_at: 1.day.ago)
-        Event.create(event_type: :usps_mail_sent, user: user, updated_at: 1.hour.ago)
+
+        expect(subject.mail_spammed?).to eq false
+      end
+    end
+
+    context 'when MAX_MAIL_EVENTS or MAIL_EVENTS_WINDOW_DAYS are zero' do
+      it 'returns false' do
+        stub_const 'Idv::UspsMail::MAX_MAIL_EVENTS', 0
+        stub_const 'Idv::UspsMail::MAIL_EVENTS_WINDOW_DAYS', 0
 
         expect(subject.mail_spammed?).to eq false
       end