Skip to content

Commit

Permalink
Removed ZSF monitoring and updated specs
Browse files Browse the repository at this point in the history
  • Loading branch information
danlim715 committed Dec 17, 2024
1 parent 8de4741 commit e32dcf4
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 51 deletions.
8 changes: 4 additions & 4 deletions app/controllers/v0/claim_documents_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ def create
# add the file after so that we have a form_id and guid for the uploader to use
@attachment.file = unlock_file(params['file'], params['password'])

if Flipper.enabled?(:document_upload_validation_enabled) && !stamped_pdf_valid?
if Flipper.enabled?(:document_upload_validation_enabled) && !stamped_pdf_valid? &&
%w[21P-527EZ 21P-530 21P-530V2].include?(form_id)
raise Common::Exceptions::ValidationErrors, @attachment
end

Expand Down Expand Up @@ -81,7 +82,6 @@ def unlock_file(file, file_password)
def stamped_pdf_valid?
extension = File.extname(@attachment&.file&.id)
allowed_types = PersistentAttachment::ALLOWED_DOCUMENT_TYPES

unless allowed_types.include?(extension)
raise Common::Exceptions::UnprocessableEntity.new(
detail: I18n.t('errors.messages.extension_allowlist_error', extension:, allowed_types:),
Expand All @@ -100,11 +100,11 @@ def stamped_pdf_valid?
end

def intake_service
@intake_service = BenefitsIntake::Service.new
@intake_service ||= BenefitsIntake::Service.new
end

def uploads_monitor
@uploads_monitor ||= ClaimDocuments::Monitor.new(intake_service)
@uploads_monitor ||= ClaimDocuments::Monitor.new('claim_documents_controller')
end
end
end
41 changes: 26 additions & 15 deletions lib/claim_documents/monitor.rb
Original file line number Diff line number Diff line change
@@ -1,36 +1,47 @@
# frozen_string_literal: true

require 'zero_silent_failures/monitor'
require 'logging/monitor'

module ClaimDocuments
##
# Monitor functions for Rails logging and StatsD
# @todo abstract, split logging for controller and sidekiq
#
class Monitor < ::ZeroSilentFailures::Monitor
class Monitor < ::Logging::Monitor
# statsd key for document uploads
DOCUMENT_STATS_KEY = 'api.document_upload'

def track_document_upload_attempt(form_id, current_user)
StatsD.increment("#{DOCUMENT_STATS_KEY}.attempt", tags: ["form_id: #{form_id}"])
Rails.logger.info("Creating PersistentAttachment FormID=#{form_id}",
{ user_account_uuid: current_user&.user_account_uuid,
statsd: "#{DOCUMENT_STATS_KEY}.attempt" })
additional_context = {
user_account_uuid: current_user&.user_account_uuid,
statsd: "#{DOCUMENT_STATS_KEY}.attempt",
tags: ["form_id:#{form_id}"]
}
track_request('info', "Creating PersistentAttachment FormID=#{form_id}", "#{DOCUMENT_STATS_KEY}.attempt",
**additional_context)
end

def track_document_upload_success(form_id, attachment_id, current_user)
StatsD.increment("#{DOCUMENT_STATS_KEY}.success", tags: ["form_id: #{form_id}"])
Rails.logger.info("Success creating PersistentAttachment FormID=#{form_id} AttachmentID=#{attachment_id}",
{ attachment_id:, user_account_uuid: current_user&.user_account_uuid,
statsd: "#{DOCUMENT_STATS_KEY}.success" })
additional_context = {
attachment_id:,
user_account_uuid: current_user&.user_account_uuid,
tags: ["form_id:#{form_id}"],
statsd: "#{DOCUMENT_STATS_KEY}.success"
}
track_request('info', "Success creating PersistentAttachment FormID=#{form_id} AttachmentID=#{attachment_id}",
"#{DOCUMENT_STATS_KEY}.success", **additional_context)
end

def track_document_upload_failed(form_id, attachment_id, current_user, e)
StatsD.increment("#{DOCUMENT_STATS_KEY}.failure", tags: ["form_id: #{form_id}"])
log_silent_failure({ form_id: form_id, attachment_id: attachment_id }, current_user&.user_account_uuid)
Rails.logger.error("Error creating PersistentAttachment FormID=#{form_id} AttachmentID=#{attachment_id} #{e}",
{ attachment_id:, user_account_uuid: current_user&.user_account_uuid,
statsd: "#{DOCUMENT_STATS_KEY}.failure", message: e&.message })
additional_context = {
attachment_id:,
user_account_uuid: current_user&.user_account_uuid,
tags: ["form_id:#{form_id}"],
statsd: "#{DOCUMENT_STATS_KEY}.failure",
message: e&.message
}
track_request('error', "Error creating PersistentAttachment FormID=#{form_id} AttachmentID=#{attachment_id} #{e}",
"#{DOCUMENT_STATS_KEY}.failure", **additional_context)
end
end
end
2 changes: 1 addition & 1 deletion spec/controllers/v0/claim_documents_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

before do
allow(controller).to receive_messages(current_user: user, uploads_monitor: monitor, intake_service:)
allow(PersistentAttachments::PensionBurial).to receive(:new).and_return(attachment)
allow(PersistentAttachment).to receive(:new).and_return(attachment)
allow(monitor).to receive_messages(track_document_upload_attempt: nil, track_document_upload_success: nil,
track_document_upload_failed: nil)
end
Expand Down
Binary file added spec/fixtures/files/tiny.pdf
Binary file not shown.
16 changes: 6 additions & 10 deletions spec/lib/claim_documents/monitor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@

describe '#track_document_upload_attempt' do
it 'logs an upload attempt' do
expect(StatsD).to receive(:increment).with("#{document_stats_key}.attempt", tags: ["form_id: #{form_id}"])
expect(StatsD).to receive(:increment).with("#{document_stats_key}.attempt", tags: ["form_id:#{form_id}"])
expect(Rails.logger).to receive(:info).with(
"Creating PersistentAttachment FormID=#{form_id}",
{ user_account_uuid: current_user.user_account_uuid, statsd: "#{document_stats_key}.attempt" }
hash_including(user_account_uuid: current_user.user_account_uuid, statsd: "#{document_stats_key}.attempt")
)

monitor.track_document_upload_attempt(form_id, current_user)
Expand All @@ -26,11 +26,10 @@

describe '#track_document_upload_success' do
it 'logs a successful upload' do
expect(StatsD).to receive(:increment).with("#{document_stats_key}.success", tags: ["form_id: #{form_id}"])
expect(StatsD).to receive(:increment).with("#{document_stats_key}.success", tags: ["form_id:#{form_id}"])
expect(Rails.logger).to receive(:info).with(
"Success creating PersistentAttachment FormID=#{form_id} AttachmentID=#{attachment_id}",
{ attachment_id: attachment_id, user_account_uuid: current_user.user_account_uuid,
statsd: "#{document_stats_key}.success" }
hash_including(user_account_uuid: current_user.user_account_uuid, statsd: "#{document_stats_key}.success")
)

monitor.track_document_upload_success(form_id, attachment_id, current_user)
Expand All @@ -39,13 +38,10 @@

describe '#track_document_upload_failed' do
it 'logs a failed upload' do
expect(StatsD).to receive(:increment).with("#{document_stats_key}.failure", tags: ["form_id: #{form_id}"])
expect(monitor).to receive(:log_silent_failure).with({ form_id: form_id, attachment_id: attachment_id },
current_user.user_account_uuid)
expect(StatsD).to receive(:increment).with("#{document_stats_key}.failure", tags: ["form_id:#{form_id}"])
expect(Rails.logger).to receive(:error).with(
"Error creating PersistentAttachment FormID=#{form_id} AttachmentID=#{attachment_id} #{error}",
{ attachment_id: attachment_id, user_account_uuid: current_user.user_account_uuid,
statsd: "#{document_stats_key}.failure", message: error.message }
hash_including(user_account_uuid: current_user.user_account_uuid, statsd: 'api.document_upload.failure')
)

monitor.track_document_upload_failed(form_id, attachment_id, current_user, error)
Expand Down
45 changes: 24 additions & 21 deletions spec/requests/v0/claim_documents_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,33 +60,36 @@
end

context 'with an invalid file' do
let(:file) do
fixture_file_upload('empty_file.txt', 'text/plain')
end
let(:file) { fixture_file_upload('tiny.pdf') }

it 'does not upload the file' do
params = { file:, form_id: '21P-527EZ' }
expect do
post('/v0/claim_attachments', params:)
end.not_to change(PersistentAttachment, :count)
expect(response).to have_http_status(:unprocessable_entity)
resp = JSON.parse(response.body)
expect(resp['errors'][0]['title']).to eq('Unprocessable Entity')
VCR.use_cassette('uploads/validate_document') do
params = { file:, form_id: '21P-527EZ' }
expect do
post('/v0/claim_attachments', params:)
end.not_to change(PersistentAttachment, :count)
expect(response).to have_http_status(:unprocessable_entity)
resp = JSON.parse(response.body)
expect(resp['errors'][0]['title']).to eq('File size must not be less than 1.0 KB')
end
end

it 'logs the error' do
expect(Rails.logger).to receive(:info).with('Creating PersistentAttachment FormID=21P-527EZ',
{ statsd: 'api.document_upload.attempt', user_account_uuid: nil })
expect(Rails.logger).not_to receive(:info).with(
/^Success creating PersistentAttachment FormID=21P-527EZ AttachmentID=\d+/
)
expect(Rails.logger).to receive(:error).with(
'Error creating PersistentAttachment FormID=21P-527EZ AttachmentID= Common::Exceptions::UnprocessableEntity',
instance_of(Hash)
)
VCR.use_cassette('uploads/validate_document') do
expect(Rails.logger).to receive(:info).with('Creating PersistentAttachment FormID=21P-527EZ',
hash_including(user_account_uuid: nil,
statsd: 'api.document_upload.attempt'))
expect(Rails.logger).not_to receive(:info).with(
/^Success creating PersistentAttachment FormID=21P-527EZ AttachmentID=\d+/
)
expect(Rails.logger).to receive(:error).with(
'Error creating PersistentAttachment FormID=21P-527EZ AttachmentID= Common::Exceptions::ValidationErrors',
instance_of(Hash)
)

params = { file:, form_id: '21P-527EZ' }
post('/v0/claim_attachments', params:)
params = { file:, form_id: '21P-527EZ' }
post('/v0/claim_attachments', params:)
end
end
end

Expand Down

0 comments on commit e32dcf4

Please sign in to comment.