diff --git a/Gemfile.lock b/Gemfile.lock index 7887d5cba95..63a1e6c0f81 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -857,8 +857,7 @@ GEM hashie (>= 1.2.0, < 6.0) jwt (>= 1.5.6) retriable (3.1.2) - rexml (3.3.6) - strscan + rexml (3.3.7) rgeo (3.0.1) rgeo-activerecord (7.0.1) activerecord (>= 5.0) @@ -914,18 +913,17 @@ GEM actionpack (>= 5.2, < 8.0) railties (>= 5.2, < 8.0) rtesseract (3.1.3) - rubocop (1.65.1) + rubocop (1.66.1) json (~> 2.3) language_server-protocol (>= 3.17.0) parallel (~> 1.10) parser (>= 3.3.0.2) rainbow (>= 2.2.2, < 4.0) regexp_parser (>= 2.4, < 3.0) - rexml (>= 3.2.5, < 4.0) - rubocop-ast (>= 1.31.1, < 2.0) + rubocop-ast (>= 1.32.2, < 2.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 2.4.0, < 3.0) - rubocop-ast (1.32.1) + rubocop-ast (1.32.2) parser (>= 3.3.1.0) rubocop-capybara (2.21.0) rubocop (~> 1.41) @@ -1019,8 +1017,6 @@ GEM stringio (3.1.1) strong_migrations (2.0.0) activerecord (>= 6.1) - strscan (3.1.0) - strscan (3.1.0-java) super_diff (0.12.1) attr_extras (>= 6.2.4) diff-lcs diff --git a/config/features.yml b/config/features.yml index 3b7df234fa1..f8be1e58cef 100644 --- a/config/features.yml +++ b/config/features.yml @@ -72,6 +72,10 @@ features: hca_enrollment_status_override_enabled: actor_type: user description: Enables override of enrollment status for a user, to allow multiple submissions with same user. + hca_insurance_v2_enabled: + actor_type: user + description: Enables the the upgraded insurance section of the Health Care Application + enable_in_development: true hca_performance_alert_enabled: actor_type: user description: Enables alert notifying users of a potential issue with application performance. @@ -1345,10 +1349,6 @@ features: actor_type: user enable_in_development: true description: Toggle for routing appointment requests to the VetsAPI Gateway Service(VPG) instead of vaos-service. - va_online_scheduling_appointment_details_redesign: - actor_type: user - enable_in_development: true - description: Toggle for redesigning the appointment details page. va_online_scheduling_recent_locations_filter: actor_type: user enable_in_development: true @@ -1667,7 +1667,7 @@ features: description: >- A frontend-focused switch that toggles visibility of and access to the Travel Pay claim details page and entry point (features toggled together). Enabled - Entry point link and claim details page are viewable. - Disabled - Entry point link and claim details page are not viewable. + Disabled - Entry point link and claim details page are not viewable. travel_pay_submit_mileage_expense: actor_type: user enable_in_development: true diff --git a/modules/claims_api/app/clients/claims_api/bgs_client/definitions.rb b/modules/claims_api/app/clients/claims_api/bgs_client/definitions.rb index 8cb2541c3a4..1579cf8cdcb 100644 --- a/modules/claims_api/app/clients/claims_api/bgs_client/definitions.rb +++ b/modules/claims_api/app/clients/claims_api/bgs_client/definitions.rb @@ -211,6 +211,38 @@ module FindPersonBySSN end end + ## + # TrackedItemService + # + # Adding 'Bean' to the end to differentiate from the service + module TrackedItemServiceBean + DEFINITION = + Bean.new( + path: 'TrackedItemService', + namespaces: Namespaces.new( + target: 'http://services.mapd.benefits.vba.va.gov/', + data: nil + ) + ) + end + + module TrackedItemService + DEFINITION = + Service.new( + bean: TrackedItemServiceBean::DEFINITION, + path: 'TrackedItemService' + ) + + module FindTrackedItems + DEFINITION = + Action.new( + service: TrackedItemService::DEFINITION, + name: 'findTrackedItems', + key: 'BenefitClaim' + ) + end + end + ## # VdcBean # diff --git a/modules/claims_api/config/initializers/okcomputer.rb b/modules/claims_api/config/initializers/okcomputer.rb index 3bd1b57d290..a44cb242b1d 100644 --- a/modules/claims_api/config/initializers/okcomputer.rb +++ b/modules/claims_api/config/initializers/okcomputer.rb @@ -83,23 +83,6 @@ def name end end -class VbmsCheck < BaseCheck - def check - connection = Faraday::Connection.new - connection.options.timeout = 10 - response = connection.get("#{Settings.vbms.url}/vbms-efolder-svc/upload-v1/eFolderUploadService?wsdl") - response.status == 200 ? process_success : process_failure - rescue - process_failure - end - - protected - - def name - 'VBMS' - end -end - OkComputer::Registry.register 'mpi', MpiCheck.new OkComputer::Registry.register 'bgs-vet_record', BgsCheck.new('vet_record') OkComputer::Registry.register 'bgs-corporate_update', BgsCheck.new('corporate_update') @@ -119,7 +102,3 @@ def name FaradayBGSCheck.new('IntentToFileWebServiceBean/IntentToFileWebService') OkComputer::Registry.register 'localbgs-trackeditem', FaradayBGSCheck.new('TrackedItemService/TrackedItemService') - -OkComputer::Registry.register 'vbms', VbmsCheck.new - -OkComputer.make_optional %w[vbms bgs-vet_record bgs-corporate_update bgs-contention] diff --git a/modules/claims_api/lib/bgs_service/local_bgs.rb b/modules/claims_api/lib/bgs_service/local_bgs.rb index cbc014aceb9..a8d35e117b7 100644 --- a/modules/claims_api/lib/bgs_service/local_bgs.rb +++ b/modules/claims_api/lib/bgs_service/local_bgs.rb @@ -19,6 +19,7 @@ class LocalBGS IntentToFileWebServiceBean/IntentToFileWebService OrgWebServiceBean/OrgWebService PersonWebServiceBean/PersonWebService + TrackedItemService/TrackedItemService VDC/VeteranRepresentativeService VdcBean/ManageRepresentativeService VnpAtchmsWebServiceBean/VnpAtchmsService diff --git a/modules/claims_api/lib/claims_api/common/exceptions/lighthouse/bad_gateway.rb b/modules/claims_api/lib/claims_api/common/exceptions/lighthouse/bad_gateway.rb new file mode 100644 index 00000000000..4d45fb9edfd --- /dev/null +++ b/modules/claims_api/lib/claims_api/common/exceptions/lighthouse/bad_gateway.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module ClaimsApi + module Common + module Exceptions + module Lighthouse + class BadGateway < StandardError + def errors + errors_array = [] + errors_array << { + status: status_code.to_s, # LH standards want this be a string + title: 'Bad gateway', + detail: 'The server received an invalid or null response from an upstream server.' + } + errors_array + end + + def status_code + 502 + end + end + end + end + end +end diff --git a/modules/claims_api/lib/claims_api/v2/error/lighthouse_error_handler.rb b/modules/claims_api/lib/claims_api/v2/error/lighthouse_error_handler.rb index 656f3dfe2e3..4ce7e8939b3 100644 --- a/modules/claims_api/lib/claims_api/v2/error/lighthouse_error_handler.rb +++ b/modules/claims_api/lib/claims_api/v2/error/lighthouse_error_handler.rb @@ -8,6 +8,7 @@ require 'claims_api/common/exceptions/lighthouse/unprocessable_entity' require 'claims_api/common/exceptions/lighthouse/resource_not_found' require 'claims_api/common/exceptions/lighthouse/bad_request' +require 'claims_api/common/exceptions/lighthouse/bad_gateway' require 'claims_api/common/exceptions/lighthouse/timeout' module ClaimsApi @@ -28,6 +29,7 @@ def self.included(clazz) # rubocop:disable Metrics/MethodLength ::ClaimsApi::Common::Exceptions::Lighthouse::BadRequest, ::Common::Exceptions::BackendServiceException, ::ClaimsApi::Common::Exceptions::Lighthouse::Timeout, + ::ClaimsApi::Common::Exceptions::Lighthouse::BadGateway, ::ClaimsApi::Common::Exceptions::Lighthouse::BackendServiceException do |err| render_non_source_error(err) end diff --git a/modules/claims_api/lib/custom_error.rb b/modules/claims_api/lib/custom_error.rb index 6383a154453..b383ddcd1cb 100644 --- a/modules/claims_api/lib/custom_error.rb +++ b/modules/claims_api/lib/custom_error.rb @@ -1,8 +1,10 @@ # frozen_string_literal: true require 'claims_api/common/exceptions/lighthouse/backend_service_exception' +require 'claims_api/common/exceptions/lighthouse/bad_gateway' require 'claims_api/common/exceptions/lighthouse/timeout' require 'claims_api/v2/error/lighthouse_error_mapper' + module ClaimsApi class CustomError def initialize(error, detail = nil, async = true) # rubocop:disable Style/OptionalBooleanParameter @@ -29,6 +31,7 @@ def build_error when ::Common::Exceptions::BackendServiceException raise ::Common::Exceptions::Forbidden if @original_status == 403 + raise_bad_gateway_exception if @original_status == 504 raise_backend_exception if @original_status == 400 raise ::Common::Exceptions::Unauthorized if @original_status == 401 @@ -50,6 +53,11 @@ def raise_timeout_exception raise ::ClaimsApi::Common::Exceptions::Lighthouse::Timeout, error_details end + def raise_bad_gateway_exception + error_details = get_error_info if @original_body.present? + raise ::ClaimsApi::Common::Exceptions::Lighthouse::BadGateway, error_details + end + def get_error_info all_errors = [] diff --git a/modules/claims_api/spec/lib/claims_api/find_definition_spec.rb b/modules/claims_api/spec/lib/claims_api/find_definition_spec.rb index 624da476ca6..e344b5430b2 100644 --- a/modules/claims_api/spec/lib/claims_api/find_definition_spec.rb +++ b/modules/claims_api/spec/lib/claims_api/find_definition_spec.rb @@ -78,6 +78,21 @@ end end + # This one doesn't have `Bean` at the end + context 'TrackedItemService' do + let(:endpoint) { 'TrackedItemService/TrackedItemService' } + let(:action) { 'findTrackedItems' } + let(:key) { 'BenefitClaim' } + + it 'response with the correct attributes' do + result = subject.for_action(endpoint, action) + parsed_result = JSON.parse(result.to_json) + expect(parsed_result['service']['bean']['path']).to eq 'TrackedItemService' + expect(parsed_result['service']['path']).to eq 'TrackedItemService' + expect(parsed_result['service']['bean']['namespaces']['target']).to eq 'http://services.mapd.benefits.vba.va.gov/' + end + end + context 'VdcBean' do let(:endpoint) { 'VDC/ManageRepresentativeService' } let(:action) { 'readPOARequest' } @@ -249,6 +264,18 @@ end end + context 'TrackedItemService' do + let(:endpoint) { 'TrackedItemService/TrackedItemService' } + + it 'response with the correct namespace' do + result = subject.for_service(endpoint) + parsed_result = JSON.parse(result.to_json) + expect(parsed_result['bean']['path']).to eq 'TrackedItemService' + expect(parsed_result['path']).to eq 'TrackedItemService' + expect(parsed_result['bean']['namespaces']['target']).to eq 'http://services.mapd.benefits.vba.va.gov/' + end + end + context 'VdcBean' do let(:endpoint) { 'VDC/ManageRepresentativeService' } diff --git a/modules/claims_api/spec/lib/claims_api/v2/error/lighthouse_error_handler_spec.rb b/modules/claims_api/spec/lib/claims_api/v2/error/lighthouse_error_handler_spec.rb index b69e556211f..01de5c0142e 100644 --- a/modules/claims_api/spec/lib/claims_api/v2/error/lighthouse_error_handler_spec.rb +++ b/modules/claims_api/spec/lib/claims_api/v2/error/lighthouse_error_handler_spec.rb @@ -51,6 +51,10 @@ def raise_expired_token_signature def raise_timeout raise ClaimsApi::Common::Exceptions::Lighthouse::Timeout end + + def raise_bad_gateway + raise ClaimsApi::Common::Exceptions::Lighthouse::BadGateway + end end before do @@ -62,6 +66,7 @@ def raise_timeout get 'raise_invalid_token' => 'anonymous#raise_invalid_token' get 'raise_expired_token_signature' => 'anonymous#raise_expired_token_signature' get 'raise_timeout' => 'anonymous#raise_timeout' + get 'raise_bad_gateway' => 'anonymous#raise_bad_gateway' end end @@ -169,4 +174,21 @@ def raise_timeout expect(parsed_body['errors'][0]['status']).to be_a(String) end end + + it 'catches an external timeout and returns bad gateway' do + mock_ccg(scopes) do |auth_header| + request.headers.merge!(auth_header) + + get :raise_bad_gateway + + expect(response).to have_http_status(:bad_gateway) + parsed_body = JSON.parse(response.body) + expect(parsed_body['errors'][0]['title']).to eq('Bad gateway') + expect(parsed_body['errors'][0]['detail']).to eq( + 'The server received an invalid or null response from an upstream server.' + ) + expect(parsed_body['errors'][0]['status']).to eq('502') + expect(parsed_body['errors'][0]['status']).to be_a(String) + end + end end diff --git a/modules/claims_api/spec/requests/metadata_spec.rb b/modules/claims_api/spec/requests/metadata_spec.rb index adc35057982..b0f643d1ee6 100644 --- a/modules/claims_api/spec/requests/metadata_spec.rb +++ b/modules/claims_api/spec/requests/metadata_spec.rb @@ -50,25 +50,36 @@ expect(response).to have_http_status(:ok) end - required_upstream_services = %w[mpi] - optional_upstream_services = %w[vbms bgs-vet_record bgs-corporate_update bgs-contention - localbgs-healthcheck] - (required_upstream_services + optional_upstream_services).each do |upstream_service| - it "returns correct status when #{upstream_service} is not healthy" do - allow(MPI::Service).to receive(:service_is_up?).and_return(upstream_service != 'mpi') - allow_any_instance_of(BGS::Services).to receive(:vet_record) - .and_return(Struct.new(:healthy?).new(upstream_service != 'bgs-vet_record')) - allow_any_instance_of(BGS::Services).to receive(:corporate_update) - .and_return(Struct.new(:healthy?).new(upstream_service != 'bgs-corporate_update')) - allow_any_instance_of(BGS::Services).to receive(:contention) - .and_return(Struct.new(:healthy?).new(upstream_service != 'bgs-contention')) - allow_any_instance_of(ClaimsApi::LocalBGS).to receive(:healthcheck) - .and_return(200) - allow_any_instance_of(Faraday::Connection).to receive(:get) - .and_return(upstream_service == 'vbms' ? Struct.new(:status).new(500) : Struct.new(:status).new(200)) + it 'returns the correct status when MPI is not healthy' do + allow(MPI::Service).to receive(:service_is_up?).and_return(false) + get "/services/claims/#{version}/upstream_healthcheck" + result = JSON.parse(response.body) + expect(result['mpi']['success']).to eq(false) + end + + local_bgs_services = %i[claimant person org ebenefitsbenftclaim intenttofile trackeditem].freeze + local_bgs_methods = %i[find_poa_by_participant_id find_by_ssn find_poa_history_by_ptcpnt_id + find_benefit_claims_status_by_ptcpnt_id insert_intent_to_file find_tracked_items].freeze + local_bgs_services.each do |local_bgs_service| + it "returns the correct status when the local bgs #{local_bgs_service} is not healthy" do + local_bgs_methods.each do |local_bgs_method| + allow_any_instance_of(ClaimsApi::LocalBGS).to receive(local_bgs_method.to_sym) + .and_return(Struct.new(:healthy?).new(false)) + get "/services/claims/#{version}/upstream_healthcheck" + result = JSON.parse(response.body) + expect(result["localbgs-#{local_bgs_service}"]['success']).to eq(false) + end + end + end + + bgs_services = %i[vet_record corporate_update contention].freeze + bgs_services.each do |service| + it "returns the correct status when the BGS #{service} is not healthy" do + allow_any_instance_of(BGS::Services).to receive(service.to_sym) + .and_return(Struct.new(:healthy?).new(false)) get "/services/claims/#{version}/upstream_healthcheck" - expected_status = required_upstream_services.include?(upstream_service) ? :internal_server_error : :success - expect(response).to have_http_status(expected_status) + result = JSON.parse(response.body) + expect(result["bgs-#{service}"]['success']).to eq(false) end end end diff --git a/modules/claims_api/spec/sidekiq/claim_custom_error_spec.rb b/modules/claims_api/spec/sidekiq/claim_custom_error_spec.rb index 2ca90b5b1c1..0f8defaccac 100644 --- a/modules/claims_api/spec/sidekiq/claim_custom_error_spec.rb +++ b/modules/claims_api/spec/sidekiq/claim_custom_error_spec.rb @@ -164,4 +164,35 @@ end end end + + context 'an external service returns a 504' do + error_original_body = { + messages: [ + { + 'key' => 'timeout', + 'severity' => 'ERROR', + 'text' => 'external service timeout' + } + ] + } + + let(:external_timeout) do + Common::Exceptions::BackendServiceException.new( + backtrace.backtrace, + { status: 504, detail: 'timeout' }, + 504, + error_original_body + ) + end + + let(:external_timeout_submit) { ClaimsApi::CustomError.new(external_timeout) } + + it 'raises a 502' do + external_timeout_submit.build_error + rescue => e + expect(e.errors[0][:status]).to eq('502') + expect(e.errors[0][:title]).to eq('Bad gateway') + expect(e.errors[0][:detail]).to eq('The server received an invalid or null response from an upstream server.') + end + end end diff --git a/modules/travel_pay/app/controllers/travel_pay/pings_controller.rb b/modules/travel_pay/app/controllers/travel_pay/pings_controller.rb deleted file mode 100644 index 92ea3f59aa8..00000000000 --- a/modules/travel_pay/app/controllers/travel_pay/pings_controller.rb +++ /dev/null @@ -1,26 +0,0 @@ -# frozen_string_literal: true - -module TravelPay - class PingsController < ApplicationController - skip_before_action :authenticate, only: :ping - - def ping - btsss_ping_response = client.ping - - render json: { data: "Received ping from upstream server with status #{btsss_ping_response.status}." } - end - - def authorized_ping - btsss_authorized_ping_response = client.authorized_ping(@current_user) - render json: { - data: "Received authorized ping from upstream server with status #{btsss_authorized_ping_response.status}." - } - end - - private - - def client - TravelPay::Client.new - end - end -end diff --git a/modules/travel_pay/app/services/travel_pay/client.rb b/modules/travel_pay/app/services/travel_pay/client.rb index f92f5f24301..16b45ff8f32 100644 --- a/modules/travel_pay/app/services/travel_pay/client.rb +++ b/modules/travel_pay/app/services/travel_pay/client.rb @@ -43,29 +43,6 @@ def request_btsss_token(veis_token, sts_token) response.body['data']['accessToken'] end - ## - # HTTP GET call to the BTSSS 'ping' endpoint to test liveness - # - # @return [Faraday::Response] - # - def ping - veis_token = request_veis_token - request_ping(veis_token) - end - - ## - # HTTP GET call to the BTSSS 'authorized-ping' endpoint to test liveness - # - # @return [Faraday::Response] - # - def authorized_ping(current_user) - sts_token = request_sts_token(current_user) - veis_token = request_veis_token - btsss_token = request_btsss_token(veis_token, sts_token) - - request_authorized_ping(veis_token, btsss_token) - end - ## # HTTP GET call to the BTSSS 'claims' endpoint # API responds with travel pay claims including status diff --git a/modules/travel_pay/config/routes.rb b/modules/travel_pay/config/routes.rb index 0f62badc5ae..a142e0ba333 100644 --- a/modules/travel_pay/config/routes.rb +++ b/modules/travel_pay/config/routes.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true TravelPay::Engine.routes.draw do - get '/pings/ping', to: 'pings#ping' - get '/pings/authorized_ping', to: 'pings#authorized_ping' resources :claims end diff --git a/modules/travel_pay/spec/controllers/pings_controller_spec.rb b/modules/travel_pay/spec/controllers/pings_controller_spec.rb deleted file mode 100644 index d9d86cc9116..00000000000 --- a/modules/travel_pay/spec/controllers/pings_controller_spec.rb +++ /dev/null @@ -1,58 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe TravelPay::PingsController, type: :request do - let(:user) { build(:user) } - - before do - sign_in(user) - Flipper.disable :travel_pay_power_switch - end - - describe '#ping' do - context 'the feature switch is enabled' do - before do - Flipper.enable :travel_pay_power_switch - end - - it 'requests a token and sends a ping to BTSSS' do - VCR.use_cassette('travel_pay/ping') do - get '/travel_pay/pings/ping' - expect(response.body).to include('Received ping from upstream server with status 200') - end - end - end - - context 'the feature switch is disabled' do - it 'raises the proper error' do - get '/travel_pay/pings/ping' - expect(response).to have_http_status(:forbidden) - expect(response.body).to include('You do not have access to travel pay') - end - end - end - - describe '#authorized_ping' do - context 'the feature switch is enabled' do - before do - Flipper.enable :travel_pay_power_switch - end - - it 'requests a token and sends a ping to BTSSS' do - VCR.use_cassette('travel_pay/auth_ping', match_requests_on: %i[method path]) do - get '/travel_pay/pings/authorized_ping', headers: { 'Authorization' => 'Bearer vagov_token' } - expect(response.body).to include('Received authorized ping from upstream server with status 200') - end - end - end - - context 'the feature switch is disabled' do - it 'raises the proper error' do - get '/travel_pay/pings/authorized_ping', headers: { 'Authorization' => 'Bearer vagov_token' } - expect(response).to have_http_status(:forbidden) - expect(response.body).to include('You do not have access to travel pay') - end - end - end -end diff --git a/modules/travel_pay/spec/services/client_spec.rb b/modules/travel_pay/spec/services/client_spec.rb index 9983bf59768..f10ba2c6962 100644 --- a/modules/travel_pay/spec/services/client_spec.rb +++ b/modules/travel_pay/spec/services/client_spec.rb @@ -74,28 +74,6 @@ end end - context 'ping' do - before do - allow_any_instance_of(TravelPay::Client) - .to receive(:request_veis_token) - .and_return('veis_token') - end - - it 'receives response from ping endpoint' do - @stubs.get('/api/v1/Sample/ping') do - [ - 200, - { 'Content-Type': 'application/json' } - ] - end - client = TravelPay::Client.new - response = client.ping - - expect(response).to be_success - @stubs.verify_stubbed_calls - end - end - context '/claims' do before do allow_any_instance_of(TravelPay::Client) @@ -159,35 +137,6 @@ end end - context 'authorized_ping' do - before do - allow_any_instance_of(TravelPay::Client) - .to receive(:request_veis_token) - .and_return('veis_token') - allow_any_instance_of(TravelPay::Client) - .to receive(:request_sts_token) - .and_return('sts_token') - allow_any_instance_of(TravelPay::Client) - .to receive(:request_btsss_token) - .with('veis_token', 'sts_token') - .and_return('btsss_token') - end - - it 'receives response from authorized-ping endpoint' do - @stubs.get('/api/v1/Sample/authorized-ping') do - [ - 200, - { 'Content-Type': 'application/json' } - ] - end - client = TravelPay::Client.new - response = client.authorized_ping(user) - - expect(response).to be_success - @stubs.verify_stubbed_calls - end - end - context 'request_sts_token' do let(:assertion) do { diff --git a/modules/va_forms/app/sidekiq/va_forms/form_builder.rb b/modules/va_forms/app/sidekiq/va_forms/form_builder.rb index 24a1245654e..2b2fecc9070 100644 --- a/modules/va_forms/app/sidekiq/va_forms/form_builder.rb +++ b/modules/va_forms/app/sidekiq/va_forms/form_builder.rb @@ -33,7 +33,7 @@ class FormFetchError < StandardError; end VAForms::Slack::Messenger.new( { class: job_class.to_s, - message: "URL for form_name: #{form_name}, row_id: #{row_id} no longer returns a valid PDF or web page.", + message: "URL for form #{form_name} no longer returns a valid PDF or web page.", form_url: url } ).notify! @@ -89,16 +89,16 @@ def build_and_save_form(form_data) # and sends Slack notifications; if response is unsuccessful, raises an error def check_form_validity(form, attrs, url) response = fetch_form(url) - if response.success? - attrs[:valid_pdf] = true - attrs[:sha256] = Digest::SHA256.hexdigest(response.body) + if response.success? || response.status == 404 # 404s are non-recoverable and should not be retried + attrs[:valid_pdf] = response.success? + attrs[:sha256] = response.success? ? Digest::SHA256.hexdigest(response.body) : nil attrs[:url] = url send_slack_notifications(form, attrs, response.headers['Content-Type']) attrs else - raise FormFetchError, 'The form could not be fetched from the url provided.' + raise FormFetchError, "The form could not be fetched from the url provided. Response code: #{response.status}" end end @@ -116,7 +116,13 @@ def fetch_form(url) # Given a +existing_form+ instance of +VAForms::Form+, +updated_attributes+ for that form, and the +content_type+ # returned by the form URL, sends appropriate Slack notifications def send_slack_notifications(existing_form, updated_attrs, content_type) - if existing_form.url != updated_attrs[:url] + if existing_form.valid_pdf && !updated_attrs[:valid_pdf] + # If the PDF was valid but is no longer valid, notify + notify_slack( + "URL for form #{updated_attrs[:form_name]} no longer returns a valid PDF or web page.", + form_url: updated_attrs[:url] + ) + elsif existing_form.url != updated_attrs[:url] # If the URL has changed, we notify regardless of content notify_slack( "Form #{updated_attrs[:form_name]} has been updated.", diff --git a/modules/va_forms/spec/sidekiq/form_builder_spec.rb b/modules/va_forms/spec/sidekiq/form_builder_spec.rb index 838393b2b4a..4ee13faa26d 100644 --- a/modules/va_forms/spec/sidekiq/form_builder_spec.rb +++ b/modules/va_forms/spec/sidekiq/form_builder_spec.rb @@ -11,13 +11,14 @@ let(:slack_messenger) { instance_double(VAForms::Slack::Messenger) } let(:default_form_data) { JSON.parse(File.read(VAForms::Engine.root.join('spec', 'fixtures', 'gql_form.json'))) } - let(:bad_url_form_data) { JSON.parse(File.read(VAForms::Engine.root.join('spec', 'fixtures', 'gql_form_invalid_url.json'))) } + let(:invalid_url_form_data) { JSON.parse(File.read(VAForms::Engine.root.join('spec', 'fixtures', 'gql_form_invalid_url.json'))) } let(:deleted_form_data) { JSON.parse(File.read(VAForms::Engine.root.join('spec', 'fixtures', 'gql_form_deleted.json'))) } let(:valid_pdf_cassette) { 'va_forms/valid_pdf' } let(:not_found_pdf_cassette) { 'va_forms/pdf_not_found' } + let(:server_error_pdf_cassette) { 'va_forms/pdf_internal_server_error' } - let(:form_fetch_error_message) { 'The form could not be fetched from the url provided.' } + let(:form_fetch_error_message) { 'The form could not be fetched from the url provided. Response code: 500' } before do Sidekiq::Job.clear_all @@ -47,7 +48,7 @@ form.reload end - context 'when the form url returns a valid body' do + context 'when the form url returns a successful response' do it 'correctly updates attributes based on the new form data' do expect(result).to have_attributes( form_name: '21-0966', @@ -76,10 +77,69 @@ end context 'when the form url returns a 404' do - let(:form_data) { bad_url_form_data } + let(:form_data) { invalid_url_form_data } + let(:invalid_form_url) { 'https://www.vba.va.gov/pubs/forms/not_a_valid_url.pdf' } + let(:result) do + form = VAForms::Form.create!(url:, form_name:, sha256:, title:, valid_pdf:, row_id:) + with_settings(Settings.va_forms.slack, enabled: enable_notifications) do + VCR.use_cassette(not_found_pdf_cassette) do + form_builder.perform(form_data) + end + end + form.reload + end + + it 'marks the form as invalid' do + expect(result.valid_pdf).to be(false) + end + + it 'updates the form url' do + expect(result.url).to eql(invalid_form_url) + end + + it 'clears the sha256' do + expect(result.sha256).to be_nil + end + + it 'correctly updates the remaining attributes based on the form data' do + expect(result).to have_attributes( + form_name: '21-0966', + row_id: 5382, + title: 'Intent to File a Claim for Compensation and/or Pension, or Survivors Pension and/or DIC', + first_issued_on: Date.new(2019, 11, 7), + last_revision_on: Date.new(2018, 8, 22), + pages: 1, + ranking: nil, + tags: '21-0966', + language: 'en', + related_forms: ['10-10d'], + benefit_categories: [{ 'name' => 'Pension', 'description' => 'VA pension benefits' }], + va_form_administration: 'Veterans Benefits Administration', + form_type: 'benefit', + form_usage: 'Someusagehtml', + form_tool_intro: 'some intro text', + form_tool_url: 'https://www.va.gov/education/apply-for-education-benefits/application/1995/introduction', + form_details_url: 'https://www.va.gov/find-forms/about-form-21-0966', + deleted_at: nil + ) + end + + it 'notifies slack that the form url no longer returns a valid form' do + result + expect(VAForms::Slack::Messenger).to have_received(:new).with( + { + class: described_class.to_s, + message: "URL for form #{form_name} no longer returns a valid PDF or web page.", + form_url: invalid_form_url + } + ) + expect(slack_messenger).to have_received(:notify!) + end + end + context 'when the form url returns a 500' do it 'raises an error' do - VCR.use_cassette(not_found_pdf_cassette) do + VCR.use_cassette(server_error_pdf_cassette) do expect { form_builder.perform(form_data) } .to raise_error(described_class::FormFetchError, form_fetch_error_message) end @@ -227,7 +287,7 @@ let(:expected_notify) do { class: described_class.to_s, - message: "URL for form_name: #{form_name}, row_id: #{row_id} no longer returns a valid PDF or web page.", + message: "URL for form #{form_name} no longer returns a valid PDF or web page.", form_url: url } end diff --git a/spec/support/vcr_cassettes/va_forms/pdf_internal_server_error.yml b/spec/support/vcr_cassettes/va_forms/pdf_internal_server_error.yml new file mode 100644 index 00000000000..4bb7777cf1a --- /dev/null +++ b/spec/support/vcr_cassettes/va_forms/pdf_internal_server_error.yml @@ -0,0 +1,84 @@ +--- +http_interactions: + - request: + method: get + uri: http://www.vba.va.gov/pubs/forms/VBA-21-0966-ARE.pdf + body: + encoding: US-ASCII + string: '' + headers: + User-Agent: + - Faraday v0.17.6 + Accept-Encoding: + - gzip;q=1.0,deflate;q=0.6,identity;q=0.3 + Accept: + - "*/*" + response: + status: + code: 302 + message: Moved Temporarily + headers: + Location: + - https://www.vba.va.gov/pubs/forms/VBA-21-0966-ARE.pdf + Server: + - BigIP + Connection: + - Keep-Alive + Content-Length: + - '0' + body: + encoding: UTF-8 + string: '' + recorded_at: Fri, 07 Jul 2023 20:12:46 GMT + - request: + method: get + uri: https://www.vba.va.gov/pubs/forms/VBA-21-0966-ARE.pdf + body: + encoding: US-ASCII + string: '' + headers: + User-Agent: + - Faraday v0.17.6 + Accept-Encoding: + - gzip;q=1.0,deflate;q=0.6,identity;q=0.3 + Accept: + - "*/*" + response: + status: + code: 500 + message: Internal Server Error + headers: + Content-Type: + - text/html + Server: + - Microsoft-IIS/8.5 + X-Powered-By: + - ASP.NET + Strict-Transport-Security: + - max-age=31536000; includeSubDomains + Date: + - Fri, 07 Jul 2023 20:12:46 GMT + Content-Length: + - '1245' + Set-Cookie: + - BIGipServerwww.vba.va.gov_http=444976650.20480.0000; path=/; Httponly; Secure + X-Frame-Options: + - SAMEORIGIN + X-Xss-Protection: + - 1; mode=block + Referrer-Policy: + - '' + body: + encoding: UTF-8 + string: "\r\n\r\n\r\n\r\n500 - Internal Server Error\r\n\r\n\r\n\r\n

Server Error

\r\n
\r\n
\r\n

500 - Internal Server Error

\r\n
\r\n
\r\n\r\n\r\n" + recorded_at: Fri, 07 Jul 2023 20:12:46 GMT +recorded_with: VCR 6.2.0