From dc004caf71c30e9b4debebc8b52722a7d37b62aa Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Wed, 11 Mar 2026 06:49:07 -0400 Subject: [PATCH] Convert attempt IP from EmailDomainBlock history tracking to string before recording (#38137) --- app/models/email_domain_block.rb | 2 +- app/models/trends/history.rb | 4 ++-- spec/models/email_domain_block_spec.rb | 10 +++++++--- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/app/models/email_domain_block.rb b/app/models/email_domain_block.rb index 583d2e6c1b..44d6bc6987 100644 --- a/app/models/email_domain_block.rb +++ b/app/models/email_domain_block.rb @@ -59,7 +59,7 @@ class EmailDomainBlock < ApplicationRecord def blocking?(allow_with_approval: false) blocks = EmailDomainBlock.where(domain: domains_with_variants, allow_with_approval: allow_with_approval).by_domain_length - blocks.each { |block| block.history.add(@attempt_ip) } if @attempt_ip.present? + blocks.each { |block| block.history.add(@attempt_ip.to_s) } if @attempt_ip.present? blocks.any? end diff --git a/app/models/trends/history.rb b/app/models/trends/history.rb index 21331f00dc..9e4d173475 100644 --- a/app/models/trends/history.rb +++ b/app/models/trends/history.rb @@ -40,11 +40,11 @@ class Trends::History with_redis { |redis| redis.get(key_for(:uses)).to_i } end - def add(account_id) + def add(value) with_redis do |redis| redis.pipelined do |pipeline| pipeline.incrby(key_for(:uses), 1) - pipeline.pfadd(key_for(:accounts), account_id) + pipeline.pfadd(key_for(:accounts), value) pipeline.expire(key_for(:uses), EXPIRE_AFTER) pipeline.expire(key_for(:accounts), EXPIRE_AFTER) end diff --git a/spec/models/email_domain_block_spec.rb b/spec/models/email_domain_block_spec.rb index c3662b2d6c..5dbc4a5aff 100644 --- a/spec/models/email_domain_block_spec.rb +++ b/spec/models/email_domain_block_spec.rb @@ -56,16 +56,20 @@ RSpec.describe EmailDomainBlock do end describe '.requires_approval?' do - subject { described_class.requires_approval?(input) } + subject { described_class.requires_approval?(input, attempt_ip: IPAddr.new('100.100.100.100')) } let(:input) { nil } context 'with a matching block requiring approval' do - before { Fabricate :email_domain_block, domain: input, allow_with_approval: true } + let!(:email_domain_block) { Fabricate :email_domain_block, domain: input, allow_with_approval: true } let(:input) { 'host.example' } - it { is_expected.to be true } + it 'returns true and records attempt' do + expect do + expect(subject).to be(true) + end.to change { email_domain_block.history.get(Date.current).accounts }.by(1) + end end context 'with a matching block not requiring approval' do