Skip to content

Commit

Permalink
[10-10CG] Update LogEmailDiffJob to not use redis key (#19398)
Browse files Browse the repository at this point in the history
* add new in_progress_email_match_logs table

* separate index and create table migrations

* delete new class

* add flipper toggle for log_email_diff_job

* add class for new table

* update log_email_diff_job to use new table

* use .exists? for efficiency

* add codeowners for new model

* rename method to be clearer

* rename table and migrations

* update log model name and subsequent specs

* update codeowners file after rename

* stub flipper calls
  • Loading branch information
coope93 authored Nov 20, 2024
1 parent 2bffa79 commit 950ae18
Show file tree
Hide file tree
Showing 5 changed files with 182 additions and 47 deletions.
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ app/models/form526_job_status.rb @department-of-veterans-affairs/Disability-Expe
app/models/form526_submission.rb @department-of-veterans-affairs/Disability-Experience @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
app/models/form526_submission_remediation.rb @department-of-veterans-affairs/Disability-Experience @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
app/models/form_attachment.rb @department-of-veterans-affairs/benefits-decision-reviews-be @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
app/models/form_email_matches_profile_log.rb @department-of-veterans-affairs/vfs-10-10 @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
app/models/form_profile.rb @department-of-veterans-affairs/vfs-authenticated-experience-backend @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
app/models/form_profiles @department-of-veterans-affairs/my-education-benefits @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
app/models/form_profiles/va_21p527ez.rb @department-of-veterans-affairs/pension-and-burials @department-of-veterans-affairs/backend-review-group
Expand Down
4 changes: 4 additions & 0 deletions app/models/form_email_matches_profile_log.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# frozen_string_literal: true

class FormEmailMatchesProfileLog < ApplicationRecord
end
32 changes: 32 additions & 0 deletions app/sidekiq/hca/log_email_diff_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ class LogEmailDiffJob
sidekiq_options retry: false

def perform(in_progress_form_id, user_uuid)
if Flipper.enabled?(:hca_log_email_diff_in_progress_form)
log_email_difference(in_progress_form_id, user_uuid)
else
log_email_difference_redis(in_progress_form_id, user_uuid)
end
end

def log_email_difference_redis(in_progress_form_id, user_uuid)
redis_key = "HCA::LogEmailDiffJob:#{user_uuid}"
return if $redis.get(redis_key).present?

Expand All @@ -28,5 +36,29 @@ def perform(in_progress_form_id, user_uuid)
)
$redis.set(redis_key, 't')
end

def log_email_difference(in_progress_form_id, user_uuid)
return if FormEmailMatchesProfileLog.exists?(user_uuid:, in_progress_form_id:)

in_progress_form = InProgressForm.find_by(id: in_progress_form_id)
return if in_progress_form.nil?

parsed_form = JSON.parse(in_progress_form.form_data)
form_email = parsed_form['email']
email_confirmation = parsed_form['view:email_confirmation']

return if form_email.blank? || form_email != email_confirmation

user = User.find(user_uuid)
va_profile_email = user.va_profile_email

tag_text = va_profile_email&.downcase == form_email.downcase ? 'same' : 'different'

StatsD.increment(
"api.1010ez.in_progress_form_email.#{tag_text}"
)

FormEmailMatchesProfileLog.create(user_uuid:, in_progress_form_id:)
end
end
end
3 changes: 3 additions & 0 deletions config/features.yml
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,9 @@ features:
hca_log_form_attachment_create:
actor_type: user
description: Enable logging all successful-looking attachment creation calls to Sentry at info-level
hca_log_email_diff_in_progress_form:
actor_type: user
description: Enable using database instead of redis to log email differences between va_profile and in progress forms.
hca_retrieve_facilities_without_repopulating:
actor_type: user
description: Constrain facilities endpoint to only return existing facilities values - even if the table is empty, do not rerun the Job to populate it.
Expand Down
189 changes: 142 additions & 47 deletions spec/sidekiq/hca/log_email_diff_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,89 +6,184 @@
let!(:in_progress_form) { create(:in_progress_1010ez_form_with_email) }
let!(:user) { create(:user, :loa3) }

before do
in_progress_form.update!(user_uuid: user.uuid)
allow(User).to receive(:find).with(user.uuid).and_return(user)
end
describe 'hca_log_email_diff_in_progress_form enabled' do
before do
allow(Flipper).to receive(:enabled?).with(:hca_log_email_diff_in_progress_form).and_return(true)
in_progress_form.update!(user_uuid: user.uuid)
allow(User).to receive(:find).with(user.uuid).and_return(user)
end

def self.expect_does_nothing
it 'does nothing' do
expect(StatsD).not_to receive(:increment)
expect($redis).not_to receive(:set)
def self.expect_does_nothing
it 'does nothing' do
expect(StatsD).not_to receive(:increment)

subject
subject

expect(FormEmailMatchesProfileLog).not_to receive(:create).with(
user_uuid: user.uuid, in_progress_form_id: in_progress_form.id
)
end
end
end

def self.expect_email_tag(tag)
it "logs that email is #{tag}" do
expect do
subject
end.to trigger_statsd_increment("api.1010ez.in_progress_form_email.#{tag}")
def self.expect_email_tag(tag)
it "logs that email is #{tag}" do
expect do
subject
end.to trigger_statsd_increment("api.1010ez.in_progress_form_email.#{tag}")

expect($redis.get("HCA::LogEmailDiffJob:#{user.uuid}")).to eq('t')
expect(InProgressForm.where(user_uuid: user.uuid, id: in_progress_form.id)).to exist
end
end
end

describe '#perform' do
subject { described_class.new.perform(in_progress_form.id, user.uuid) }
describe '#perform' do
subject { described_class.new.perform(in_progress_form.id, user.uuid) }

context 'when the form has been deleted' do
before do
in_progress_form.destroy!
context 'when the form has been deleted' do
before do
in_progress_form.destroy!
end

expect_does_nothing
end

expect_does_nothing
end
context 'when form email is present' do
context 'when email confirmation is different' do
before do
in_progress_form.update!(
form_data: JSON.parse(in_progress_form.form_data).except('view:email_confirmation').to_json
)
end

context 'when form email is present' do
context 'when email confirmation is different' do
expect_does_nothing
end

context 'when va profile email is different' do
expect_email_tag('different')
end

context 'when va profile is the same' do
before do
allow(user).to receive(:va_profile_email).and_return('Email@email.com')
end

expect_email_tag('same')

context 'when FormEmailMatchesProfileLog already exists' do
before do
FormEmailMatchesProfileLog.create(user_uuid: user.uuid, in_progress_form_id: in_progress_form.id)
end

expect_does_nothing
end
end

context 'when va profile email is blank' do
before do
expect(user).to receive(:va_profile_email).and_return(nil)
end

expect_email_tag('different')
end
end

context 'when form email is blank' do
before do
in_progress_form.update!(
form_data: JSON.parse(in_progress_form.form_data).except('view:email_confirmation').to_json
form_data: JSON.parse(in_progress_form.form_data).except('email').to_json
)
end

expect_does_nothing
end
end
end

describe 'hca_log_email_diff_in_progress_form disabled' do
before do
allow(Flipper).to receive(:enabled?).with(:hca_log_email_diff_in_progress_form).and_return(false)
in_progress_form.update!(user_uuid: user.uuid)
allow(User).to receive(:find).with(user.uuid).and_return(user)
end

def self.expect_does_nothing
it 'does nothing' do
expect(StatsD).not_to receive(:increment)
expect($redis).not_to receive(:set)

subject
end
end

def self.expect_email_tag(tag)
it "logs that email is #{tag}" do
expect do
subject
end.to trigger_statsd_increment("api.1010ez.in_progress_form_email.#{tag}")

context 'when va profile email is different' do
expect_email_tag('different')
expect($redis.get("HCA::LogEmailDiffJob:#{user.uuid}")).to eq('t')
end
end

describe '#perform' do
subject { described_class.new.perform(in_progress_form.id, user.uuid) }

context 'when va profile is the same' do
context 'when the form has been deleted' do
before do
allow(user).to receive(:va_profile_email).and_return('Email@email.com')
in_progress_form.destroy!
end

expect_email_tag('same')
expect_does_nothing
end

context 'when redis key for the user is already set' do
context 'when form email is present' do
context 'when email confirmation is different' do
before do
$redis.set("HCA::LogEmailDiffJob:#{user.uuid}", 't')
in_progress_form.update!(
form_data: JSON.parse(in_progress_form.form_data).except('view:email_confirmation').to_json
)
end

expect_does_nothing
end
end

context 'when va profile email is blank' do
before do
expect(user).to receive(:va_profile_email).and_return(nil)
context 'when va profile email is different' do
expect_email_tag('different')
end

expect_email_tag('different')
end
end
context 'when va profile is the same' do
before do
allow(user).to receive(:va_profile_email).and_return('Email@email.com')
end

context 'when form email is blank' do
before do
in_progress_form.update!(
form_data: JSON.parse(in_progress_form.form_data).except('email').to_json
)
expect_email_tag('same')

context 'when redis key for the user is already set' do
before do
$redis.set("HCA::LogEmailDiffJob:#{user.uuid}", 't')
end

expect_does_nothing
end
end

context 'when va profile email is blank' do
before do
expect(user).to receive(:va_profile_email).and_return(nil)
end

expect_email_tag('different')
end
end

expect_does_nothing
context 'when form email is blank' do
before do
in_progress_form.update!(
form_data: JSON.parse(in_progress_form.form_data).except('email').to_json
)
end

expect_does_nothing
end
end
end
end

0 comments on commit 950ae18

Please sign in to comment.