Skip to content

Commit

Permalink
API-24186 Skippable benefits intake dimension checks (#12257)
Browse files Browse the repository at this point in the history
Since oversized PDFs are already resized by Data Dimensions upstream,
this change lets the Benefits Intake API accept documents larger than
the current limit of 22" wide/long in either of the following cases:

1. The `vba_documents_skip_dimension_check` Flipper flag is on, or
2. The metadata.json submitted with the document contains the key/value
   pair `{ "skipDimensionCheck": true }`
  • Loading branch information
caseywilliams authored Mar 28, 2023
1 parent e520d22 commit 992526d
Show file tree
Hide file tree
Showing 7 changed files with 137 additions and 27 deletions.
3 changes: 3 additions & 0 deletions config/features.yml
Original file line number Diff line number Diff line change
Expand Up @@ -751,6 +751,9 @@ features:
va_view_dependents_access:
actor_type: user
description: Allows us to gate the View/ Modify dependents content in a progressive rollout
vba_documents_skip_dimension_check:
actor_type: user
description: Allows Benefits Intake to accept any size of PDF, skipping the check for page dimensions
yellow_ribbon_mvp_enhancement:
actor_type: user
description: Enhances Yellow Ribbon MVP.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,11 @@ def download_and_process
# Validations
validate_parts(@upload, parts)
validate_metadata(parts[META_PART_NAME], submission_version: @upload.metadata['version'].to_i)
validate_documents(parts)

metadata = perfect_metadata(@upload, parts, timestamp)

pdf_validator_options = metadata['skipDimensionCheck'] ? { check_page_dimensions: false } : {}
validate_documents(parts, pdf_validator_options)

response = submit(metadata, parts)

process_response(response)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ def validate_body
Tempfile.create("vba-documents-validate-#{SecureRandom.hex}.pdf", binmode: true) do |tempfile|
tempfile << @request.body.read
tempfile.rewind

validator = PDFValidator::Validator.new(tempfile, PDF_VALIDATOR_OPTIONS)
options = Flipper.enabled?(:vba_documents_skip_dimension_check) ? { check_page_dimensions: false } : {}
validator = PDFValidator::Validator.new(tempfile, PDF_VALIDATOR_OPTIONS.merge(options))
result = validator.validate

unless result.valid_pdf?
Expand Down
18 changes: 12 additions & 6 deletions modules/vba_documents/lib/vba_documents/upload_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,14 @@ def validate_metadata(metadata_input, submission_version:)
raise VBADocuments::UploadError.new(code: 'DOC102', detail: 'Invalid JSON object')
end

def validate_documents(parts)
def validate_documents(parts, pdf_validator_options = {})
# Validate 'content' document
validate_document(parts[DOC_PART_NAME], DOC_PART_NAME)
validate_document(parts[DOC_PART_NAME], DOC_PART_NAME, pdf_validator_options)

# Validate attachments
attachment_names = parts.keys.select { |key| key.match(/attachment\d+/) }
attachment_names.each do |attachment_name|
validate_document(parts[attachment_name], attachment_name)
validate_document(parts[attachment_name], attachment_name, pdf_validator_options)
end
end

Expand Down Expand Up @@ -112,9 +112,15 @@ def validate_line_of_business(lob, submission_version)
end
end

def validate_document(file_path, part_name)
validator = PDFValidator::Validator.new(file_path, { check_encryption: false })
result = validator.validate
DEFAULT_PDF_VALIDATOR_OPTIONS = {
check_encryption: false # Owner passwords are allowed, user passwords are not
}.freeze

def validate_document(file_path, part_name, pdf_validator_options = {})
options = DEFAULT_PDF_VALIDATOR_OPTIONS.merge(pdf_validator_options)
options.merge!({ check_page_dimensions: false }) if Flipper.enabled?(:vba_documents_skip_dimension_check)

result = PDFValidator::Validator.new(file_path, options).validate

unless result.valid_pdf?
errors = result.errors
Expand Down
22 changes: 22 additions & 0 deletions modules/vba_documents/spec/lib/document_request_validator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,28 @@
expect(result.dig(:data, :attributes, :status)).to eq('valid')
end

describe 'given a document with large pages' do
let(:fixture_name) { '18x22.pdf' }

context 'when vba_documents_skip_dimension_check flag is off' do
before { Flipper.disable(:vba_documents_skip_dimension_check) }

it 'considers the PDF invalid' do
errors = result[:errors]
expect(errors.length).to eq(1)
expect(errors.first[:status]).to eq('422')
end
end

context 'when vba_documents_skip_dimension_check flag is on' do
before { Flipper.enable(:vba_documents_skip_dimension_check) }

it 'considers the PDF valid' do
expect(result.dig(:data, :attributes, :status)).to eq('valid')
end
end
end

describe 'given a PDF with an owner/permissions password' do
let(:fixture_name) { 'encrypted.pdf' }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,19 +118,44 @@ def invalidate_metadata(key, value = nil, delete_key = false)
expect(@attributes['uploaded_pdf']).to have_key('content')
end

it 'returns a UUID with status of error when an attachment is oversized' do
post SUBMIT_ENDPOINT,
params: {}.merge(valid_metadata).merge(valid_content).merge(invalid_attachment_oversized)
expect(response).to have_http_status(:bad_request)
json = JSON.parse(response.body)
@attributes = json['data']['attributes']
expect(@attributes).to have_key('guid')
expect(@attributes['status']).to eq('error')
uploaded_pdf = @attributes['uploaded_pdf']
expect(uploaded_pdf['total_documents']).to eq(3)
expect(uploaded_pdf['content']['dimensions']['oversized_pdf']).to eq(false)
expect(uploaded_pdf['content']['attachments'].first['dimensions']['oversized_pdf']).to eq(true)
expect(uploaded_pdf['content']['attachments'].last['dimensions']['oversized_pdf']).to eq(false)
describe 'when an attachment is oversized' do
let(:params) { {}.merge(valid_metadata).merge(valid_content).merge(invalid_attachment_oversized) }

context 'with the "vba_documents_skip_dimension_check" flag turned off' do
before { Flipper.disable :vba_documents_skip_dimension_check }

it 'returns a UUID with status of error' do
post SUBMIT_ENDPOINT, params: params
expect(response).to have_http_status(:bad_request)
json = JSON.parse(response.body)
@attributes = json['data']['attributes']
expect(@attributes).to have_key('guid')
expect(@attributes['status']).to eq('error')
uploaded_pdf = @attributes['uploaded_pdf']
expect(uploaded_pdf['total_documents']).to eq(3)
expect(uploaded_pdf['content']['dimensions']['oversized_pdf']).to eq(false)
expect(uploaded_pdf['content']['attachments'].first['dimensions']['oversized_pdf']).to eq(true)
expect(uploaded_pdf['content']['attachments'].last['dimensions']['oversized_pdf']).to eq(false)
end
end

context 'with the "vba_documents_skip_dimension_check" flag turned on' do
before { Flipper.enable :vba_documents_skip_dimension_check }

it 'allows the upload, returning a UUID with a status of uploaded and correct metadata' do
post SUBMIT_ENDPOINT, params: params
expect(response).to have_http_status(:ok)
json = JSON.parse(response.body)
@attributes = json['data']['attributes']
expect(@attributes).to have_key('guid')
expect(@attributes['status']).to eq('uploaded')
uploaded_pdf = @attributes['uploaded_pdf']
expect(uploaded_pdf['total_documents']).to eq(3)
expect(uploaded_pdf['content']['dimensions']['oversized_pdf']).to eq(false)
expect(uploaded_pdf['content']['attachments'].first['dimensions']['oversized_pdf']).to eq(true)
expect(uploaded_pdf['content']['attachments'].last['dimensions']['oversized_pdf']).to eq(false)
end
end
end

%i[dashes_slashes_first_last valid_metadata_space_in_name].each do |allowed|
Expand Down
60 changes: 56 additions & 4 deletions modules/vba_documents/spec/workers/upload_processor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -496,15 +496,67 @@
end

context 'with invalid sizes' do
%w[18x22 22x18].each do |invalid_size|
invalid_sizes = %w[18x22 22x18]

context 'when vba_documents_skip_dimension_check flag is off' do
before { Flipper.disable(:vba_documents_skip_dimension_check) }

it 'sets an error status for invalid size' do
invalid_sizes.each do |invalid_size|
allow(VBADocuments::MultipartParser).to receive(:parse) {
{ 'metadata' => valid_metadata, 'content' => get_fixture("#{invalid_size}.pdf") }
}
described_class.new.perform(upload.guid, test_caller)
updated = VBADocuments::UploadSubmission.find_by(guid: upload.guid)
expect(updated.status).to eq('error')
expect(updated.code).to eq('DOC108')
end
end

context 'when metadata.json contains skipDimensionCheck = true' do
let(:special_metadata) { JSON.parse(valid_metadata).merge({ 'skipDimensionCheck' => true }).to_json }
let(:content) { get_fixture("#{invalid_sizes.first}.pdf") }

before do
allow(CentralMail::Service).to receive(:new) { client_stub }
allow(faraday_response).to receive(:status).and_return(200)
allow(faraday_response).to receive(:body).and_return('')
allow(faraday_response).to receive(:success?).and_return(true)
allow(client_stub).to receive(:upload).and_return(faraday_response)
end

it 'allows the upload' do
allow(VBADocuments::MultipartParser).to receive(:parse) do
{ 'metadata' => special_metadata, 'content' => content }
end
described_class.new.perform(upload.guid, test_caller)
updated = VBADocuments::UploadSubmission.find_by(guid: upload.guid)
expect(updated.uploaded_pdf.dig('content', 'dimensions', 'oversized_pdf')).to eq(true)
expect(updated.status).to eq('received')
end
end
end

context 'when vba_documents_skip_dimension_check flag is on' do
let(:content) { get_fixture("#{invalid_sizes.first}.pdf") }

before do
Flipper.enable(:vba_documents_skip_dimension_check)
allow(CentralMail::Service).to receive(:new) { client_stub }
allow(faraday_response).to receive(:status).and_return(200)
allow(faraday_response).to receive(:body).and_return('')
allow(faraday_response).to receive(:success?).and_return(true)
allow(client_stub).to receive(:upload).and_return(faraday_response)
end

it 'allows the upload' do
allow(VBADocuments::MultipartParser).to receive(:parse) {
{ 'metadata' => valid_metadata, 'content' => get_fixture("#{invalid_size}.pdf") }
{ 'metadata' => valid_metadata, 'content' => content }
}
described_class.new.perform(upload.guid, test_caller)
updated = VBADocuments::UploadSubmission.find_by(guid: upload.guid)
expect(updated.status).to eq('error')
expect(updated.code).to eq('DOC108')
expect(updated.uploaded_pdf.dig('content', 'dimensions', 'oversized_pdf')).to eq(true)
expect(updated.status).to eq('received')
end
end
end
Expand Down

0 comments on commit 992526d

Please sign in to comment.