From 386a1abdae1fd4945ee17414b59a5b804a790211 Mon Sep 17 00:00:00 2001 From: Matthew Thornton <99351305+ThorntonMatthew@users.noreply.github.com> Date: Fri, 30 Aug 2024 20:12:34 -0400 Subject: [PATCH] Feature Tracking PR for APPEALS-50887 (#22244) * Remove transaction_wrapper * Remove extra newline * APPEALS-51045: Remove ReceiveNotificationJob (#22207) Co-authored-by: nhansen3 * MattT/APPEALS-51115: Initialize FIFO SQS Queues Locally (#22182) * Set local env to utilize FIFO queues where appropriate FIFO queues are configured with the same attributes as our queues in higher environments. * Resolve failing test * Remove defunct test * Update make commands --------- Co-authored-by: Matthew Thornton * APPEALS-51847: Institute Database Migration for Establishing 'sms_status_reason' and 'email_status_reason' Columns (#22208) * adding and running migration * adding comments to migration and re running db:migrate * pushing up original scheam * committing schema * small edit to migration and deleting unnecessary columns from schema --------- Co-authored-by: nhansen3 Co-authored-by: Matthew Thornton <99351305+ThorntonMatthew@users.noreply.github.com> * removing job and all mentions of job (#22212) Co-authored-by: nhansen3 Co-authored-by: Matthew Thornton <99351305+ThorntonMatthew@users.noreply.github.com> * MattT/APPEALS-51059: Alter Select Quarterly Statuses (#22239) * Alter status messages * Save more info in fake response * Swap keys * Add missing param * Test fixes. Also, setting appeal_docketed to false whenever appeals are decided or cancelled * Fix fasterer issue --------- Co-authored-by: Matthew Thornton * MattT/APPEALS-51101: Rewrite ProcessNotificationStatusUpdateJob to Consume Messages from an SQS Queue (#22247) * Update fake response body * Try enabling localstack in CI * Init SqsService and alter most/all of the ProcessNotificationStatusUpdatesJob * Create SQS queues in test env * Change port formatting * Mount docker socket * Try localstack hostname * Wait for localstack in CI * Add more yarddoc comments * Log errors and number of messages processed * Add custom error classes * Add yarddoc comments to SqsService class * Create SqsService spec file * Redo much of ProcessNotificationStatusUpdatesJob's spec file * Disable line length checks for yarddoc comments --------- Co-authored-by: Matthew Thornton * APPEALS-51087: Rewrite Api::V1::VaNotifyController#notification_update Controller Action to No Longer Utilize Redis (#22295) * initial push * rubocop'n * fixing tests? * rubocop'n * addressing most of matt's comments * removing yard doc files and keeping comment * rubocop --------- Co-authored-by: nhansen3 * Remove extra attribute * Fix incorrect log counts * Fix log statement * Fix typo * APPEALS-53603: Modify logic to always show recipient information if present (#22414) * initial commit * making rspec tests happy * linting * adding tests back and refactoring * small char error --------- Co-authored-by: nhansen3 * Sync decided appeals with states table (#22434) * APPEALS-52882 Sync decided appeals decision_mailed status * APPEALS-52882 Fix lint issues * APPEALS-52882 Fix codeclimate issues * APPEALS-52882 Address PR feedback * APPEALS-52882 Address PR feedback * Jcohen/APPEALS-52861 (#22450) * APPEALS-52861 Bug fix implemented. * APPEALS-52861 cleaned up some code through aliasing a common method name between appeal_types, eliminating a check that we do in the AppealDecisionMailed module. * APPEALS-52861 fixed method name error. * APPEALS-52861 fixed some tests. * APPEALS-52861 fixed More tests. * APPEALS-52861 fixed More tests. * APPEALS-52861 feature branch changes merged in with working branch and method name changes extracted. * APPEALS-52861 feature branch changes merged in with working branch and method name changes extracted, included case statement. * APPEALS-52861 moved case statement to another file to be the boolean check for constested status on an appeal. --------- Co-authored-by: Jonathan Cohen Co-authored-by: Marc Steele <71673522+msteele96@users.noreply.github.com> * APPEALS-52892: For all legacy appeal states/notifications check status in VACOLS (#22455) * initial commit * addressing comments * small change * making tests happy * fixing spec tests * rubocop'n * reverting --------- Co-authored-by: nhansen3 Co-authored-by: Marc Steele <71673522+msteele96@users.noreply.github.com> * APPEALS-55159: Parameter Requirements Are Too Stringent Given What We Will Expect from VA Notify (#22561) * APPEALS-55159 * updating tests * APPEALS-55159 Refactor permitted and required params * APPEALS-55159 Fix linting issue --------- Co-authored-by: nhansen3 Co-authored-by: msteele Co-authored-by: Marc Steele <71673522+msteele96@users.noreply.github.com> * Adding rake task to sync decided appeals (#22492) * APPEALS-52872 Add rake task to sync decided appeals * APPEALS-52872 Remove byebug * APPEALS-52872 Fixing lint errors * APPEALS-52872 Addressing performance issues * APPEALS-52872 Addressing performance issues * APPEALS-52872 Fix ENV variable update * APPEALS-52872 Parallelizing AppealState updates * APPEALS-52872 Remove byebug --------- Co-authored-by: Marc Steele <71673522+msteele96@users.noreply.github.com> * Prevent Prodtest from contacting VA Notify Staging (#22629) * APPEALS-54918 Update VA Notify fake service and initializer * APPEALS-54918 Add CASEFLOW_BASE_URL env var in demo/dev * APPEALS-54918 Add seed file for API Key and added to main seed execution * APPEALS-54918 Add missing argument to method definition * APPEALS-54918 Update "to" params and fix linting issues * APPEALS-54918 Prevent post requests in test env * Rename ApiKeys seed file * Fix linting error on require statement * Add argument for call within sync_review_job.rb and minor refactor * Inherit seed from base, align with AMA for seed file placement --------- Co-authored-by: Matthew Thornton Co-authored-by: noahhansen-gov <166541737+noahhansen-gov@users.noreply.github.com> Co-authored-by: nhansen3 Co-authored-by: prernadevbah <132498915+prernadevbah@users.noreply.github.com> Co-authored-by: msteele Co-authored-by: Jonathan Cohen <121630615+JCohDev@users.noreply.github.com> Co-authored-by: Jonathan Cohen Co-authored-by: Marc Steele <71673522+msteele96@users.noreply.github.com> --- .github/workflows/workflow.yml | 12 + Makefile.example | 4 +- .../api/v1/va_notify_controller.rb | 67 +++-- app/helpers/sync_decided_appeals_helper.rb | 76 +++++ app/jobs/nightly_syncs_job.rb | 12 + app/jobs/process_decision_document_job.rb | 4 +- ...process_notification_status_updates_job.rb | 211 ++++++++++++-- app/jobs/receive_notification_job.rb | 73 ----- app/jobs/send_notification_job.rb | 24 +- app/jobs/sync_reviews_job.rb | 2 +- app/jobs/va_notify_status_update_job.rb | 187 ------------ app/models/appeal_state.rb | 7 +- app/models/decision_document.rb | 17 +- .../va_notify/appeal_decision_mailed.rb | 9 +- .../va_notify/appellant_notification.rb | 11 + app/services/sqs_service.rb | 79 +++++ .../components/NotificationTableColumns.jsx | 2 +- client/constants/QUARTERLY_STATUSES.json | 4 +- client/constants/VA_NOTIFY_CONSTANTS.json | 3 + client/test/data/notifications.js | 4 +- config/environments/demo.rb | 2 + config/environments/development.rb | 9 +- config/environments/test.rb | 6 + config/initializers/message_queues.rb | 30 ++ config/initializers/scheduled_jobs.rb | 1 - config/initializers/shoryuken.rb | 19 +- config/initializers/va_notify.rb | 7 +- ...d_sm_sand_email_status_to_notifications.rb | 6 + db/schema.rb | 2 + db/seeds/api_keys.rb | 9 +- docker-compose-m1.yml | 5 +- docker-compose.yml | 5 +- lib/caseflow/error.rb | 4 + lib/fakes/va_notify_service.rb | 42 ++- lib/tasks/appeal_state_synchronizer.rake | 8 + .../api/v1/va_notify_controller_spec.rb | 222 +++++++++++++- .../sync_decided_appeals_helper_spec.rb | 75 +++++ .../hearings/receive_notification_job_spec.rb | 182 ------------ spec/jobs/nightly_syncs_job_spec.rb | 77 +++++ .../notification_initialization_job_spec.rb | 4 + .../process_decision_document_job_spec.rb | 4 +- ...ss_notification_status_updates_job_spec.rb | 196 ++++++++----- spec/jobs/quarterly_notifications_job_spec.rb | 66 ++--- spec/jobs/send_notification_job_spec.rb | 12 +- spec/jobs/sync_reviews_job_spec.rb | 2 +- spec/jobs/va_notify_status_update_job_spec.rb | 271 ------------------ spec/models/appellant_notification_spec.rb | 42 ++- spec/models/decision_document_spec.rb | 3 +- spec/models/tasks/bva_dispatch_task_spec.rb | 6 +- spec/services/sqs_service_spec.rb | 136 +++++++++ spec/workflows/ihp_tasks_factory_spec.rb | 2 +- 51 files changed, 1280 insertions(+), 983 deletions(-) create mode 100644 app/helpers/sync_decided_appeals_helper.rb delete mode 100644 app/jobs/receive_notification_job.rb delete mode 100644 app/jobs/va_notify_status_update_job.rb create mode 100644 app/services/sqs_service.rb create mode 100644 client/constants/VA_NOTIFY_CONSTANTS.json create mode 100644 config/initializers/message_queues.rb create mode 100644 db/migrate/20240717145856_add_sm_sand_email_status_to_notifications.rb create mode 100644 spec/helpers/sync_decided_appeals_helper_spec.rb delete mode 100644 spec/jobs/hearings/receive_notification_job_spec.rb delete mode 100644 spec/jobs/va_notify_status_update_job_spec.rb create mode 100644 spec/services/sqs_service_spec.rb diff --git a/.github/workflows/workflow.yml b/.github/workflows/workflow.yml index c33f56c9b6a..a691dc2a520 100644 --- a/.github/workflows/workflow.yml +++ b/.github/workflows/workflow.yml @@ -45,6 +45,15 @@ jobs: ports: - 1521:1521 + localstack: + image: localstack/localstack:0.14.5 + ports: + - 4566:4566 + env: + SERVICES: "sqs" + volumes: + - /var/run/docker.sock:/var/run/docker.sock + strategy: fail-fast: false matrix: @@ -188,6 +197,9 @@ jobs: - name: "Wait for database" run: dockerize -wait tcp://postgres:5432 -timeout 1m + - name: "Wait for localstack" + run: dockerize -wait tcp://localstack:4566 -timeout 30s + - name: "Wait for FACOLS" run: ./ci-bin/capture-log "bundle exec rake local:vacols:wait_for_connection" diff --git a/Makefile.example b/Makefile.example index e27b6efcb99..22486d99be1 100644 --- a/Makefile.example +++ b/Makefile.example @@ -287,7 +287,7 @@ one-test: ## run the rspec test passed in bundle exec rspec $(RUN_ARGS) run-all-queues: ## start shoryuken with all queues - bundle exec shoryuken -q caseflow_development_send_notifications caseflow_development_high_priority caseflow_development_low_priority -R + bundle exec shoryuken -q caseflow_development_send_notifications.fifo caseflow_development_high_priority caseflow_development_low_priority -R run-low-priority: ## start shoryuken with just the low priority queue bundle exec shoryuken -q caseflow_development_low_priority -R @@ -296,7 +296,7 @@ run-high-priority: ## start shoryuken with just the high priority queue bundle exec shoryuken -q caseflow_development_high_priority -R run-send-notifications: ## start shoryuken with just the send_notification queue - bundle exec shoryuken -q caseflow_development_send_notifications -R + bundle exec shoryuken -q caseflow_development_send_notifications.fifo -R jest: ## Run jest tests cd client && yarn jest diff --git a/app/controllers/api/v1/va_notify_controller.rb b/app/controllers/api/v1/va_notify_controller.rb index d47a946fd06..9ddb119e58e 100644 --- a/app/controllers/api/v1/va_notify_controller.rb +++ b/app/controllers/api/v1/va_notify_controller.rb @@ -1,46 +1,63 @@ # frozen_string_literal: true class Api::V1::VaNotifyController < Api::ApplicationController - # Purpose: POST request to VA Notify API to update status for a Notification entry + # Purpose: POST request to VA Notify API to update status for a Notification entry. # # Params: Params content can be found at https://vajira.max.gov/browse/APPEALS-21021 # # Response: Update corresponding Notification status def notifications_update - send "#{required_params[:notification_type]}_update" + send_sqs_message + render json: { + message: "#{params['notification_type']} Notification successfully updated: ID #{params['id']}" + } + rescue StandardError => error + log_error(error, params["id"], params["notification_type"]) + render json: { error: error.message }, status: :bad_request end private - # Purpose: Finds and updates notification if type is email - # - # Params: Params content can be found at https://vajira.max.gov/browse/APPEALS-21021 - # - # Response: Update corresponding email Notification status - def email_update - redis.set("email_update:#{required_params[:id]}:#{required_params[:status]}", 0) - - render json: { message: "Email notification successfully updated: ID #{required_params[:id]}" } + def va_notify_params + params.permit(:id, :notification_type, :status, :status_reason, :to) end - # Purpose: Finds and updates notification if type is SMS - # - # Params: Params content can be found at https://vajira.max.gov/browse/APPEALS-21021 - # - # Response: Update corresponding SMS Notification status - def sms_update - redis.set("sms_update:#{required_params[:id]}:#{required_params[:status]}", 0) - - render json: { message: "SMS notification successfully updated: ID #{required_params[:id]}" } + def build_message_body + id_param, notification_type_param, status_param = va_notify_params.require([:id, :notification_type, :status]) + + { + external_id: id_param, + notification_type: notification_type_param, + recipient: va_notify_params[:to], + status: status_param, + status_reason: va_notify_params[:status_reason] + } + rescue StandardError => error + raise error end - def required_params - id_param, notification_type_param, status_param = params.require([:id, :notification_type, :status]) + def build_sqs_message + message_body = build_message_body.to_json + + { + queue_url: SqsService.find_queue_url_by_name(name: "receive_notifications"), + message_body: message_body, + message_deduplication_id: Digest::SHA256.hexdigest(message_body), + message_group_id: Constants.VA_NOTIFY_CONSTANTS.message_group_id + } + rescue StandardError => error + raise error + end - { id: id_param, notification_type: notification_type_param, status: status_param } + def send_sqs_message + sqs = SqsService.sqs_client + sqs.send_message(build_sqs_message) end - def redis - @redis ||= Redis.new(url: Rails.application.secrets.redis_url_cache) + def log_error(error, external_id, notification_type) + Rails.logger.error("#{error.message}\n#{error.backtrace.join("\n")}\n \ + external_id: #{external_id}\n \ + notification_type: #{notification_type}") + Raven.capture_exception(error) end end diff --git a/app/helpers/sync_decided_appeals_helper.rb b/app/helpers/sync_decided_appeals_helper.rb new file mode 100644 index 00000000000..45adcc0e36e --- /dev/null +++ b/app/helpers/sync_decided_appeals_helper.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +## +# Helper to sync the decided appeals and their decision_mailed status + +module SyncDecidedAppealsHelper + VACOLS_BATCH_PROCESS_LIMIT = ENV["VACOLS_QUERY_BATCH_SIZE"] || 800 + + # Syncs the decision_mailed status of Legacy Appeals with a decision made + def sync_decided_appeals + begin + # Join query to retrieve Legacy AppealState ids and corresponding vacols_id + appeal_state_ids = AppealState.legacy.where(decision_mailed: false) + .joins(:legacy_appeal).preload(:legacy_appeal) + .pluck(:id, :vacols_id) + + appeal_state_ids_hash = appeal_state_ids.to_h + + vacols_decision_dates = get_decision_dates(appeal_state_ids_hash.values).to_h + + ActiveSupport::Dependencies.interlock.permit_concurrent_loads do + Parallel.each(appeal_state_ids_hash, in_threads: 4) do |appeal_state_hash| + appeal_state_id = appeal_state_hash[0] + vacols_id = appeal_state_hash[1] + # If there is a decision date on the VACOLS record, + # update the decision_mailed status on the AppealState to true + if vacols_decision_dates[vacols_id].present? + AppealState.find(appeal_state_id).decision_mailed_appeal_state_update_action! + end + end + end + rescue StandardError => error + Rails.logger.error("#{error.class}: #{error.message}\n#{error.backtrace}") + + # Re-raising the error so it can be caught in the NightlySyncsJob report + raise error + end + end + + # Method to retrieve the decision dates from VACOLS in batches + # params: vacols_ids + # Returns: Hash containing the key, value pair of vacols_id, decision_date + def get_decision_dates(vacols_ids) + begin + decision_dates = {} + + # Query VACOLS in batches + vacols_ids.in_groups_of(VACOLS_BATCH_PROCESS_LIMIT.to_i) do |vacols_id| + VACOLS::Case.where(bfkey: vacols_id).each do |vacols_record| + decision_dates[vacols_record[:bfkey]] = vacols_record[:bfddec] + end + end + + decision_dates + rescue ActiveRecord::RecordNotFound + [] + end + end + + def get_vacols_ids(legacy_appeal_states) + begin + vacols_ids = {} + + legacy_appeal_states.each do |appeal_state| + legacy_appeal = LegacyAppeal.find(appeal_state.appeal_id) + + # Find the VACOLS record associated with the LegacyAppeal + vacols_ids << { appeal_state.id.to_s => (legacy_appeal[:vacols_id]).to_s } + end + + vacols_ids + rescue ActiveRecord::RecordNotFound + {} + end + end +end diff --git a/app/jobs/nightly_syncs_job.rb b/app/jobs/nightly_syncs_job.rb index ef835251d04..664a1fa0ce3 100644 --- a/app/jobs/nightly_syncs_job.rb +++ b/app/jobs/nightly_syncs_job.rb @@ -6,6 +6,7 @@ class NightlySyncsJob < CaseflowJob queue_with_priority :low_priority application_attr :queue # arbitrary + include SyncDecidedAppealsHelper def perform RequestStore.store[:current_user] = User.system_user @@ -16,6 +17,7 @@ def perform sync_vacols_users sync_decision_review_tasks sync_bgs_attorneys + sync_all_decided_appeals slack_service.send_notification(@slack_report.join("\n"), self.class.name) if @slack_report.any? end @@ -84,6 +86,14 @@ def sync_bgs_attorneys @slack_report << "*Fatal error in sync_bgs_attorneys:* #{error}" end + def sync_all_decided_appeals + begin + sync_decided_appeals + rescue StandardError => error + @slack_report << "*Fatal error in sync_decided_appeals* #{error}" + end + end + def dangling_legacy_appeals reporter = LegacyAppealsWithNoVacolsCase.new reporter.call @@ -105,5 +115,7 @@ def sync_hearing_states state.scheduled_in_error_appeal_state_update_action! end end + rescue StandardError => error + @slack_report << "*Fatal error in sync_hearing_states* #{error}" end end diff --git a/app/jobs/process_decision_document_job.rb b/app/jobs/process_decision_document_job.rb index e2d0e9f0cee..8f392bab152 100644 --- a/app/jobs/process_decision_document_job.rb +++ b/app/jobs/process_decision_document_job.rb @@ -4,10 +4,10 @@ class ProcessDecisionDocumentJob < CaseflowJob queue_with_priority :low_priority application_attr :intake - def perform(decision_document_id, mail_package = nil) + def perform(decision_document_id, contested, mail_package = nil) RequestStore.store[:application] = "idt" RequestStore.store[:current_user] = User.system_user - DecisionDocument.find(decision_document_id).process!(mail_package) + DecisionDocument.find(decision_document_id).process!(contested, mail_package) end end diff --git a/app/jobs/process_notification_status_updates_job.rb b/app/jobs/process_notification_status_updates_job.rb index 32bb0e443cb..7c8571c8c6a 100644 --- a/app/jobs/process_notification_status_updates_job.rb +++ b/app/jobs/process_notification_status_updates_job.rb @@ -1,45 +1,204 @@ # frozen_string_literal: true +# rubocop:disable Layout/LineLength +# A job that pulls messages from the 'receive_notifications' FIFO SQS queue +# that represent status updates for VA Notify notifications and persists +# the information in our notifications table. +# +# The messages are queued by {Api::V1::VaNotifyController#notifications_update} which is +# an endpoint where VA Notify sends information to us about notifications we've requested +# that they send via their +# {https://github.com/department-of-veterans-affairs/notification-api/blob/1b758dddf2d2c12d73415e4ee508cf6b0e101343/app/celery/service_callback_tasks.py#L29 send_delivery_status_to_service} callback. +# +# This information includes: +# - The latest status pertaining to the notification's delivery (ex: success or temporary-failure) +# - The status reason (extra context around the status, if available) +# - The recipient's email or phone number +# - Caseflow simply provides VA Notify with the intended recipient's participant ID with each initial notification request, and it does not know of the destination of a message until they inform us. +# +# @see https://github.com/department-of-veterans-affairs/caseflow/wiki/VA-Notify +# @see https://github.com/department-of-veterans-affairs/caseflow/wiki/Status-Webhook-API +# rubocop:enable Layout/LineLength class ProcessNotificationStatusUpdatesJob < CaseflowJob + include Hearings::EnsureCurrentUserIsSet + queue_with_priority :low_priority + MESSAGE_GROUP_ID = "VANotifyStatusUpdate" # Used to only process messages queued by the status update webhook + PROCESSING_LIMIT = 5000 # How many updates to perform per job execution + + # Consumes messages from the 'receive_notifications' FIFO SQS queue whose 'MessageGroupId' + # attribute matches MESSAGE_GROUP_ID, and then persists data contained within those messages + # about VA Notify notifications to our 'notifications' table. def perform - RequestStore[:current_user] = User.system_user + ensure_current_user_is_set - redis = Redis.new(url: Rails.application.secrets.redis_url_cache) + begin + number_of_messages_processed = 0 - processed_count = 0 + number_of_messages_processed += process_batch_of_messages while number_of_messages_processed < PROCESSING_LIMIT + rescue Caseflow::Error::SqsQueueExhaustionError + Rails.logger.info("ProcessNotificationStatusUpdatesJob is exiting early due to the queue being empty.") + rescue StandardError => error + log_error(error) + raise error + ensure + Rails.logger.info("#{number_of_messages_processed} messages have been processed by this execution.") + end + end - # prefer scan so we only load a single record into memory, - # dumping the whole list could cause performance issues when job runs - redis.scan_each(match: "*_update:*") do |key| - break if processed_count >= 1000 + private - begin - raw_notification_type, uuid, status = key.split(":") + # Returns the SQS URL of the 'receive_notifications' FIFO SQS queue for the + # current environment using a substring. + # + # @return [String] + # The URL of the queue that messages will be pulled from. + def recv_queue_url + @recv_queue_url ||= SqsService.find_queue_url_by_name(name: "receive_notifications", check_fifo: true) + end - notification_type = extract_notification_type(raw_notification_type) + # Pulls in up to 10 messages from the 'receive_notifications' FIFO SQS queue + # and consume the data in order to persist VA Notify status updates to the + # the notifications table. + # + # @see https://github.com/department-of-veterans-affairs/caseflow/blob/master/app/controllers/api/v1/va_notify_controller.rb + # + # @return [Integer] + # The number of messages that were attempted to be processed in a batch. + def process_batch_of_messages + response = SqsService.sqs_client.receive_message( + { + queue_url: recv_queue_url, + max_number_of_messages: 10, + attribute_names: ["MessageGroupId"] + } + ) - fail InvalidNotificationStatusFormat if [notification_type, uuid, status].any?(&:nil?) + # Exit loop early if there does not seem to be any more messages. + fail Caseflow::Error::SqsQueueExhaustionError if response.messages.empty? - rows_updated = Notification.select(Arel.star).where( - Notification.arel_table["#{notification_type}_notification_external_id".to_sym].eq(uuid) - ).update_all("#{notification_type}_notification_status" => status) + filtered_messages = filter_messages_by_group_id(response.messages) - fail StandardError, "No notification matches UUID #{uuid}" if rows_updated.zero? - rescue StandardError => error - log_error(error) - ensure - # cleanup keys - do first so we don't reporcess any failed keys - redis.del key - processed_count += 1 - end - end + batch_status_updates(filtered_messages) + SqsService.batch_delete_messages(queue_url: recv_queue_url, messages: filtered_messages) + + # Return the number of messages attempted to be processed + filtered_messages.size end - private + # Sorts pending status update messages by notification type and performs up to two + # separate UPDATE queries to persist data to the corresponding notifications + # table records. + # + # @param messages [Array] A collection of AWS SQS messages. + # + # @return [Boolean] + # True/False depending on if the final totals could be logged. + def batch_status_updates(messages) + parsed_bodies = messages.map { |msg| JSON.parse(msg.body) } + + email_rows_update_count = update_email_statuses(filter_body_by_notification_type(parsed_bodies, "email")) + sms_rows_update_count = update_sms_statuses(filter_body_by_notification_type(parsed_bodies, "sms")) + + Rails.logger.info( + "Email statuses updated: #{email_rows_update_count} - SMS statuses updated: #{sms_rows_update_count}" + ) + end + + # Filters messages bodies by notification_type. + # + # @param bodies [Array>] A collection of the bodies of messages that have been + # parsed into hashes. + # @param notification_type [String] The type of notification to filter for. 'email' and 'sms' + # are the two valid types at the time of writing this comment. + # + # @return [Array>] + # Messages bodies whose notification_type matches the desired one. + def filter_body_by_notification_type(bodies, notification_type) + bodies.filter { _1["notification_type"] == notification_type } + end + + # Performs updates to any email notifications in the current batch of messages + # being processed. Statuses, status reasons, and recipient informations are items that are updated. + # + # @param status_update_list [Array>] A collection of the bodies of messages that have been + # parsed into hashes. These represent VA Notify status updates. + # + # @return [Integer] + # The number of rows that have been updated. + def update_email_statuses(status_update_list) + return 0 if status_update_list.empty? + + query = <<-SQL + UPDATE notifications AS n SET + email_notification_status = new.n_status, + recipient_email = new.recipient, + email_status_reason = new.status_reason + FROM ( VALUES + #{build_values_mapping(status_update_list)} + ) AS new(external_id, n_status, status_reason, recipient) + WHERE new.external_id = n.email_notification_external_id + SQL + + ActiveRecord::Base.connection.update(query) + end + + # Performs updates to any SMS notifications in the current batch of messages + # being processed. Statuses, status reasons, and recipient informations are items that are updated. + # + # @param status_update_list [Array>] A collection of the bodies of messages that have been + # parsed into hashes. These represent VA Notify status updates. + # + # @return [Integer] + # The number of rows that have been updated. + def update_sms_statuses(status_update_list) + return 0 if status_update_list.empty? + + query = <<-SQL + UPDATE notifications AS n SET + sms_notification_status = new.n_status, + recipient_phone_number = new.recipient, + sms_status_reason = new.status_reason + FROM ( VALUES + #{build_values_mapping(status_update_list)} + ) AS new(external_id, n_status, status_reason, recipient) + WHERE new.external_id = n.sms_notification_external_id + SQL + + ActiveRecord::Base.connection.update(query) + end + + # Builds a comma-delimited list of VALUES expressions to represent the data to be used + # in updated notification statuses, status reasons, and recipient information. + # + # @param status_update_list [Array>] A collection of the bodies of messages that have been + # parsed into hashes. These represent VA Notify status updates. + # + # @return [String] + # A sanitized SQL string consisting of VALUE expressions. + def build_values_mapping(status_update_list) + values = status_update_list.map do |status_update| + external_id = status_update["external_id"] + status = status_update["status"] + status_reason = status_update["status_reason"] + recipient = status_update["recipient"] + + "('#{external_id}', '#{status}', '#{status_reason}', '#{recipient}')" + end + + ActiveRecord::Base.sanitize_sql(values.join(",")) + end - def extract_notification_type(raw_notification_type) - raw_notification_type.split("_").first + # Filters out SQS messages whose MessageGroupId isn't the one utilized by our VA Notify webhooks + # so that they're not accidentally processed. + # + # @param messages [Array] A collection of messages to be filtered. + # + # @return [Array] + # Messages whose MessageGroupId matches the one this job expect. Messages with + # a different MessageGroupId will be ignored. + def filter_messages_by_group_id(messages) + messages.filter { _1.attributes["MessageGroupId"] == MESSAGE_GROUP_ID } end end diff --git a/app/jobs/receive_notification_job.rb b/app/jobs/receive_notification_job.rb deleted file mode 100644 index 7294ce043af..00000000000 --- a/app/jobs/receive_notification_job.rb +++ /dev/null @@ -1,73 +0,0 @@ -# frozen_string_literal: true - -class ReceiveNotificationJob < CaseflowJob - queue_as ApplicationController.dependencies_faked? ? :receive_notifications : :"receive_notifications.fifo" - application_attr :hearing_schedule - - def perform(message) - if !message.nil? - message_attributes = message[:message_attributes] - if !message_attributes.nil? - # load reference value to obtain notification id for record lookup - notification_id = message_attributes[:reference][:string_value] - - # load intersecting fields that may change in our database - email_address = message_attributes[:email_address][:string_value] - phone_number = message_attributes[:phone_number][:string_value] - status = message_attributes[:status][:string_value] - type = message_attributes[:type][:string_value] - - # load record - audit_record = Notification.find_by(id: notification_id) - - compare_notification_audit_record(audit_record, email_address, phone_number, status, type) - - else - log_error("message_attributes was nil on the ReceiveNotificationListenerJob message. Exiting Job.") - end - else - log_error("There was no message passed into the ReceiveNotificationListener. Exiting job.") - end - end - - private - - # Purpose: Method to be called with an error need to be logged to the rails logger - # - # Params: error_message (Expecting a string) - Message to be logged to the logger - # - # Response: None - def log_error(error_message) - Rails.logger.error(error_message) - end - - # Purpose: Method to compare audit record from database with record in message - # - # Params: - # - audit_record - audit record to compare with message - # - email_address - email of recipient - # - phone_number = phone number of recipient - # - status - status of notification - # - type - sms or email, used to update email/text notification status - # - # Returns: Updated model from update_audit_record - def compare_notification_audit_record(audit_record, email_address, phone_number, status, type) # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity - status = status.capitalize - - if !email_address.nil? && audit_record.recipient_email != email_address - audit_record.update!(recipient_email: email_address) - end - - if !phone_number.nil? && audit_record.recipient_phone_number != phone_number - audit_record.update!(recipient_phone_number: phone_number) - end - - if type == "email" && !status.nil? && status != audit_record.email_notification_status - audit_record.update!(email_notification_status: status) - elsif type == "sms" && !status.nil? && status != audit_record.sms_notification_status - audit_record.update!(sms_notification_status: status) - end - - audit_record - end -end diff --git a/app/jobs/send_notification_job.rb b/app/jobs/send_notification_job.rb index 4438014e8ec..9bce677e847 100644 --- a/app/jobs/send_notification_job.rb +++ b/app/jobs/send_notification_job.rb @@ -43,12 +43,11 @@ class SendNotificationJobError < StandardError; end class << self def queue_name_suffix - ApplicationController.dependencies_faked? ? :send_notifications : :"send_notifications.fifo" + :"send_notifications.fifo" end end # Must receive JSON string as argument - def perform(message_json) ensure_current_user_is_set @@ -57,18 +56,12 @@ def perform(message_json) @message = validate_message(JSON.parse(message_json, object_class: OpenStruct)) - transaction_wrapper do + ActiveRecord::Base.transaction do @notification_audit = find_or_create_notification_audit update_notification_statuses send_to_va_notify if message_status_valid? end rescue StandardError => error - if Rails.deploy_env?(:prodtest) && error.in?(DISCARD_ERRORS) - transaction_wrapper do - @notification_audit = find_or_create_notification_audit - end - end - log_error(error) raise error end @@ -76,19 +69,6 @@ def perform(message_json) private - # Conditionally wraps database operations in a transaction block depending on whether - # the current environment is ProdTest. The choice to not have ProdTest queries utilize - # a transction is due to how unlikely it will be for us to have an operation VA Notify - # integration in that environment due to this environment having production-replicated - # data and us not wanting to inadvertently transmit messages to actual recipients. - # - # The lack of a transaction block will prevent rollbacks on the records created in the - # notifications table and allow for observations around notification accuracy to be - # more easily obtained. - def transaction_wrapper - ActiveRecord::Base.transaction { yield } - end - def event_type message.template_name end diff --git a/app/jobs/sync_reviews_job.rb b/app/jobs/sync_reviews_job.rb index e7be4761efb..92a26c5aa81 100644 --- a/app/jobs/sync_reviews_job.rb +++ b/app/jobs/sync_reviews_job.rb @@ -58,7 +58,7 @@ def perform_decision_rating_issues_syncs(limit) def reprocess_decision_documents(limit) DecisionDocument.requires_processing.limit(limit).each do |decision_document| - ProcessDecisionDocumentJob.perform_later(decision_document.id) + ProcessDecisionDocumentJob.perform_later(decision_document.id, decision_document.for_contested_claim?) end end end diff --git a/app/jobs/va_notify_status_update_job.rb b/app/jobs/va_notify_status_update_job.rb deleted file mode 100644 index 0072204ab5d..00000000000 --- a/app/jobs/va_notify_status_update_job.rb +++ /dev/null @@ -1,187 +0,0 @@ -# frozen_string_literal: true - -class VANotifyStatusUpdateJob < CaseflowJob - queue_with_priority :low_priority - application_attr :hearing_schedule - - QUERY_LIMIT = ENV["VA_NOTIFY_STATUS_UPDATE_BATCH_LIMIT"] - VALID_NOTIFICATION_STATUSES = %w[Success temporary-failure technical-failure sending created].freeze - - # Description: Jobs main perform method that will find all notification records that do not have - # status updates from VA Notify and calls VA Notify API to get the latest status - # - # Params: None - # - # Retuns: None - def perform # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity - notifications_not_processed.each do |notification| - sms_external_id = notification.sms_notification_external_id - email_external_id = notification.email_notification_external_id - case notification.notification_type - when "Email" - if !email_external_id.nil? - update_attributes = get_current_status(email_external_id, "Email") - update_notification_audit_record(notification, update_attributes) - else - log_error("Notification Record " + notification.id.to_s + "With Email type does not have an external id.") - update_notification_audit_record(notification, "email_notification_status" => "No External Id") - end - when "SMS" - if !sms_external_id.nil? - update_attributes = get_current_status(sms_external_id, "SMS") - update_notification_audit_record(notification, update_attributes) - else - log_error("Notification Record " + notification.id.to_s + "With SMS type does not have an external id.") - update_notification_audit_record(notification, "sms_notification_status" => "No External Id") - end - when "Email and SMS" - if !email_external_id.nil? - update_attributes = get_current_status(email_external_id, "Email") - update_notification_audit_record(notification, update_attributes) - else - log_error("Notification Record " + notification.id.to_s + "With Email and SMS type does not have an \ - email external id.") - update_notification_audit_record(notification, "email_notification_status" => "No External Id") - end - if !sms_external_id.nil? - update_attributes = get_current_status(sms_external_id, "SMS") - update_notification_audit_record(notification, update_attributes) - else - log_error("Notification Record " + notification.id.to_s + "With Email and SMS type does not have a \ - SMS external id.") - update_notification_audit_record(notification, "sms_notification_status" => "No External Id") - end - end - notification.save! - end - end - - private - - # Description: Method that applies a query limit to the list of notification records that - # will get the status checked for. - # them from VA Notiufy - # - # Params: None - # - # Retuns: Lits of Notification records that has QUERY_LIMIT or less records - def notifications_not_processed - if !QUERY_LIMIT.nil? && QUERY_LIMIT.is_a?(String) - find_notifications_not_processed.first(QUERY_LIMIT.to_i) - else - log_info("VANotifyStatusJob can not read the VA_NOTIFY_STATUS_UPDATE_BATCH_LIMIT environment variable.\ - Defaulting to 650.") - find_notifications_not_processed.first(650) - end - end - - # Description: Method to query the Notification database for Notififcation - # records that have not been updated with a VA Notify Status - # - # Params: None - # - # Retuns: Lits of Notification Active Record associations meeting the where condition - def find_notifications_not_processed - Notification.select(Arel.star).where( - Arel::Nodes::Group.new( - email_status_check.or( - sms_status_check.or( - email_and_sms_status_check - ) - ) - ) - ) - .where(created_at: 4.days.ago..Time.zone.now) - .order(created_at: :desc) - end - - def email_status_check - Notification.arel_table[:notification_type].eq("Email").and( - generate_valid_status_check(:email_notification_status) - ) - end - - def sms_status_check - Notification.arel_table[:notification_type].eq("SMS").and( - generate_valid_status_check(:sms_notification_status) - ) - end - - def email_and_sms_status_check - Notification.arel_table[:notification_type].eq("Email and SMS").and( - generate_valid_status_check(:email_notification_status).or( - generate_valid_status_check(:sms_notification_status) - ) - ) - end - - def generate_valid_status_check(col_name_sym) - Notification.arel_table[col_name_sym].in(VALID_NOTIFICATION_STATUSES) - end - - # Description: Method to be called when an error message need to be logged - # - # Params: Error message to be logged - # - # Retuns: None - def log_error(message) - Rails.logger.error(message) - end - - # Description: Method to be called when an info message need to be logged - # - # Params: Info message to be logged - # - # Retuns: None - def log_info(message) - Rails.logger.info(message) - end - - # Description: Method that will get the VA Notify Status for the notification based on notification type - # - # - # Params: - # notification_id - The external id that VA Notify assigned to each notification. Can be for Email or SMS - # type - Type of notification to get status for - # values - Email, SMS or Email and SMS - # - # Retuns: Return a hash of attributes that need to be updated on the notification record - def get_current_status(notification_id, type) - begin - response = VANotifyService.get_status(notification_id) - if type == "Email" - { "email_notification_status" => response.body["status"], "recipient_email" => response.body["email_address"] } - elsif type == "SMS" - { "sms_notification_status" => response.body["status"], "recipient_phone_number" => - response.body["phone_number"] } - else - message = "Type neither email nor sms" - log_error("VA Notify API returned error for notificiation " + notification_id + " with type " + type) - Raven.capture_exception(type, extra: { error_uuid: error_uuid, message: message }) - end - rescue Caseflow::Error::VANotifyApiError => error - log_error( - "VA Notify API returned error for notification " + notification_id + " with error #{error}" - ) - Raven.capture_exception(error, extra: { error_uuid: error_uuid }) - nil - end - end - - # Description: Method that will update the notification record values - # - # Params: - # notification_audit_record - Notification Record to be updated - # to_update - Hash containing the column names and values to be updated - # - # Retuns: Lits of Notification records that has QUERY_LIMIT or less records - def update_notification_audit_record(notification_audit_record, to_update) - to_update&.each do |key, value| - notification_audit_record[key] = value - end - end -end - -def error_uuid - @error_uuid ||= SecureRandom.uuid -end diff --git a/app/models/appeal_state.rb b/app/models/appeal_state.rb index 6acd80c9c8f..c71700f39b0 100644 --- a/app/models/appeal_state.rb +++ b/app/models/appeal_state.rb @@ -443,9 +443,14 @@ def update_appeal_state_action!(status_to_update) if status_to_update == :appeal_cancelled existing_statuses.merge!({ privacy_act_complete: false, - privacy_act_pending: false + privacy_act_pending: false, + appeal_docketed: false }) end + + if status_to_update == :decision_mailed + existing_statuses[:appeal_docketed] = false + end end) end end diff --git a/app/models/decision_document.rb b/app/models/decision_document.rb index ec6496e8321..13d7a0ec93b 100644 --- a/app/models/decision_document.rb +++ b/app/models/decision_document.rb @@ -61,12 +61,16 @@ def submit_for_processing!(delay: processing_delay) super if not_processed_or_decision_date_not_in_the_future? - ProcessDecisionDocumentJob.perform_later(id, mail_package) + # Below we're grabbing the boolean value at this point in time. + # This will act as a point of truth that wont be affected by the + # async behavior of the outcode function due to triggering jobs. + + ProcessDecisionDocumentJob.perform_later(id, for_contested_claim?, mail_package) end end # rubocop:disable Metrics/CyclomaticComplexity - def process!(mail_package) + def process!(_contested, mail_package) return if processed? fail NotYetSubmitted unless submitted_and_ready? @@ -118,6 +122,15 @@ def all_contention_records(epe) contention_records(epe) end + def for_contested_claim? + case appeal_type + when "Appeal" + appeal.contested_claim? + when "LegacyAppeal" + appeal.contested_claim + end + end + private attr_reader :mail_package diff --git a/app/models/prepend/va_notify/appeal_decision_mailed.rb b/app/models/prepend/va_notify/appeal_decision_mailed.rb index 708af3e4955..9618ed5af0d 100644 --- a/app/models/prepend/va_notify/appeal_decision_mailed.rb +++ b/app/models/prepend/va_notify/appeal_decision_mailed.rb @@ -12,16 +12,11 @@ module AppealDecisionMailed # Params: none # # Response: returns true if successfully processed, returns false if not successfully processed (will not notify) - def process!(mail_package = nil) + def process!(contested, mail_package = nil) super_return_value = super if processed? appeal.appeal_state.decision_mailed_appeal_state_update_action! - case appeal_type - when "Appeal" - template = appeal.contested_claim? ? CONTESTED_CLAIM : NON_CONTESTED_CLAIM - when "LegacyAppeal" - template = appeal.contested_claim ? CONTESTED_CLAIM : NON_CONTESTED_CLAIM - end + template = contested ? CONTESTED_CLAIM : NON_CONTESTED_CLAIM AppellantNotification.notify_appellant(appeal, template) end super_return_value diff --git a/app/models/prepend/va_notify/appellant_notification.rb b/app/models/prepend/va_notify/appellant_notification.rb index 77e4df6ec8b..f2e9d49526e 100644 --- a/app/models/prepend/va_notify/appellant_notification.rb +++ b/app/models/prepend/va_notify/appellant_notification.rb @@ -23,10 +23,21 @@ def status end end + class InactiveAppealError < StandardError + def initialize(appeal_id, message = "The appeal status is inactive") + super(message + " for appeal with id #{appeal_id}") + end + + def status + "Inactive" + end + end + class NoAppealError < StandardError; end def self.handle_errors(appeal) fail NoAppealError if appeal.nil? + fail InactiveAppealError, appeal.external_id if !appeal.active? message_attributes = {} message_attributes[:appeal_type] = appeal.class.to_s diff --git a/app/services/sqs_service.rb b/app/services/sqs_service.rb new file mode 100644 index 00000000000..9f56544df56 --- /dev/null +++ b/app/services/sqs_service.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +# A service class to aid in interacting with Caseflow's SQS queues. +class SqsService + class << self + # Intializes an SQS client, or returns a cached version if one has already been initialized. + # + # @return [Aws::SQS::Client] + # An SQS Client + def sqs_client + @sqs_client ||= initialize_sqs_client + end + + # Locates the URL for a SQS queue based on a provided substring. + # + # @param name [String] A substring of the queue's name being searched for. + # @param check_fifo [Boolean] Whether or not the queue being searched for should be for a FIFO queue. + # + # @return [String] The full URL of the SQS queue whose name contains the substring provided. + def find_queue_url_by_name(name:, check_fifo: false) + url = sqs_client.list_queues.queue_urls.find { _1.include?(name) && _1.include?(ENV["DEPLOY_ENV"]) } + + fail Caseflow::Error::SqsQueueNotFoundError, "The #{name} SQS queue is missing in this environment." unless url + + # Optional validation check + if check_fifo && !url.include?(".fifo") + fail Caseflow::Error::SqsUnexpectedQueueTypeError, "No FIFO queue with name #{name} could be located." + end + + url + end + + # Removes the messages provided from a specified queue. + # + # @param queue_url [String] The URL of the SQS queue that the messages will be deleted from. + # @param messages [Array] Messages to be deleted. + def batch_delete_messages(queue_url:, messages:) + messages.in_groups_of(10, false).flat_map do |msg_batch| + sqs_client.delete_message_batch({ + queue_url: queue_url, + entries: process_entries_for_batch_delete(msg_batch) + }) + end + end + + private + + # Intializes an SQS client. Takes into account SQS endpoint overrides and applies them + # to the instantiated client object. + # + # @return [Aws::SQS::Client] + # An SQS Client + def initialize_sqs_client + sqs_client = Aws::SQS::Client.new + + # Allow for overriding the endpoint requests are sent to via the Rails config. + if Rails.application.config.sqs_endpoint + sqs_client.config[:endpoint] = URI(Rails.application.config.sqs_endpoint) + end + + sqs_client + end + + # Prepares a batch of messages to be in the format needed for the SQS SDK's delete_message_batch method. + # + # @param unprocessed_entries [Array] Messages to be deleted. + # + # @return [Array] An array where each entry is a hash that contains a unique (per batch) + # id and a message's receipt handle. + def process_entries_for_batch_delete(unprocessed_entries) + unprocessed_entries.map.with_index do |msg, index| + { + id: "message_#{index}", + receipt_handle: msg.receipt_handle + } + end + end + end +end diff --git a/client/app/queue/components/NotificationTableColumns.jsx b/client/app/queue/components/NotificationTableColumns.jsx index 592e8c775af..e23aa966832 100644 --- a/client/app/queue/components/NotificationTableColumns.jsx +++ b/client/app/queue/components/NotificationTableColumns.jsx @@ -81,7 +81,7 @@ export const recipientInformationColumn = (notifications) => { tableData: notifications, valueName: 'Recipient Information', // eslint-disable-next-line no-negated-condition - valueFunction: (notification) => notification.status !== 'delivered' ? '—' : notification.recipient_information + valueFunction: (notification) => notification.recipient_information ?? '—' }; }; diff --git a/client/constants/QUARTERLY_STATUSES.json b/client/constants/QUARTERLY_STATUSES.json index e6df95484e1..c6891b266e8 100644 --- a/client/constants/QUARTERLY_STATUSES.json +++ b/client/constants/QUARTERLY_STATUSES.json @@ -4,7 +4,7 @@ "hearing_scheduled": "Hearing Scheduled", "privacy_pending": "Privacy Act Pending", "ihp_pending": "VSO IHP Pending", - "hearing_to_be_rescheduled": "Hearing to be Rescheduled", - "hearing_to_be_rescheduled_privacy_pending": "Hearing to be Rescheduled / Privacy Act Pending", + "hearing_to_be_rescheduled": "docketed", + "hearing_to_be_rescheduled_privacy_pending": "Privacy Act Pending", "appeal_docketed": "docketed" } diff --git a/client/constants/VA_NOTIFY_CONSTANTS.json b/client/constants/VA_NOTIFY_CONSTANTS.json new file mode 100644 index 00000000000..5c30dead852 --- /dev/null +++ b/client/constants/VA_NOTIFY_CONSTANTS.json @@ -0,0 +1,3 @@ +{ + "message_group_id": "VANotifyStatusUpdate" +} diff --git a/client/test/data/notifications.js b/client/test/data/notifications.js index b31dd46ccf6..af53219e4c0 100644 --- a/client/test/data/notifications.js +++ b/client/test/data/notifications.js @@ -118,9 +118,9 @@ export const notifications = [ notification_type: 'Email and SMS', event_date: '2022-10-27', event_type: 'Appeal decision mailed (Non-contested claims)', - recipient_email: 'test@caseflow.com', + recipient_email: null, recipient_phone_number: '2468012345', - email_notification_status: 'sent', + email_notification_status: 'temporary-failure', sms_notification_status: 'delivered', notification_content: 'string' } diff --git a/config/environments/demo.rb b/config/environments/demo.rb index 2000da40c1b..474107fc1eb 100644 --- a/config/environments/demo.rb +++ b/config/environments/demo.rb @@ -85,6 +85,8 @@ # eFolder Express URL for demo environment used as a mock link ENV["EFOLDER_EXPRESS_URL"] ||= "http://localhost:4000" + ENV["CASEFLOW_BASE_URL"] ||= "https://www.demo.appeals.va.gov" + # BatchProcess ENVs # priority_ep_sync ENV["BATCH_PROCESS_JOB_DURATION"] ||= "1" # Number of hours the job will run for diff --git a/config/environments/development.rb b/config/environments/development.rb index c56b036c14a..e615bb3fafc 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -98,10 +98,10 @@ # Set to true to get the documents from efolder running locally on port 4000. config.use_efolder_locally = false - # set to true to create queues and override the sqs endpiont + # set to true to create queues and override the sqs endpoint config.sqs_create_queues = true - config.sqs_endpoint = ENV.has_key?('DOCKERIZED') ? 'http://localstack:4576' : 'http://localhost:4576' + config.sqs_endpoint = ENV.has_key?('DOCKERIZED') ? 'http://localstack:4566' : 'http://localhost:4566' # since we mock aws using localstack, provide dummy creds to the aws gem ENV["AWS_ACCESS_KEY_ID"] ||= "dummykeyid" @@ -128,12 +128,17 @@ # One time Appeal States migration for Legacy & AMA Appeal Batch Sizes ENV["STATE_MIGRATION_JOB_BATCH_SIZE"] ||= "1000" + # Syncing decided appeals in select batch sizes + ENV["VACOLS_QUERY_BATCH_SIZE"] ||= "800" + # Travel Board Sync Batch Size ENV["TRAVEL_BOARD_HEARING_SYNC_BATCH_LIMIT"] ||= "250" # Time in seconds before the sync lock expires LOCK_TIMEOUT = ENV["SYNC_LOCK_MAX_DURATION"] ||= "60" + ENV["CASEFLOW_BASE_URL"] ||= "http://localhost:3000" + # Notifications page eFolder link ENV["CLAIM_EVIDENCE_EFOLDER_BASE_URL"] ||= "https://vefs-claimevidence-ui-uat.stage.bip.va.gov" diff --git a/config/environments/test.rb b/config/environments/test.rb index 83e3a687dd0..01f2cd1db43 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -128,6 +128,9 @@ # One time Appeal States migration for Legacy & AMA Appeal Batch Sizes ENV["STATE_MIGRATION_JOB_BATCH_SIZE"] ||= "1000" + # Syncing decided appeals in select batch sizes + ENV["VACOLS_QUERY_BATCH_SIZE"] ||= "800" + # Travel Board Sync Batch Size ENV["TRAVEL_BOARD_HEARING_SYNC_BATCH_LIMIT"] ||= "250" @@ -149,4 +152,7 @@ # Dynatrace variables ENV["STATSD_ENV"] = "test" + + config.sqs_create_queues = true + config.sqs_endpoint = ENV["CI"] ? 'http://localstack:4566' : 'http://localhost:4566' end diff --git a/config/initializers/message_queues.rb b/config/initializers/message_queues.rb new file mode 100644 index 00000000000..c8832581647 --- /dev/null +++ b/config/initializers/message_queues.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +# Initializes SQS message queues not for intended for use with +# asynchronous jobs. +# +# This will primarily be utilized in our development and demo environments. + +QUEUE_PREFIX = "caseflow_#{ENV['DEPLOY_ENV']}_" + +MESSAGE_QUEUES = [ + { + name: "receive_notifications.fifo", + attributes: { + "FifoQueue" => "true", + "FifoThroughputLimit" => "perQueue" + } + } +].freeze + +if Rails.application.config.sqs_create_queues + sqs_client = Aws::SQS::Client.new + sqs_client.config[:endpoint] = URI(Rails.application.config.sqs_endpoint) + + MESSAGE_QUEUES.each do |queue_info| + sqs_client.create_queue({ + queue_name: "#{QUEUE_PREFIX}#{queue_info[:name]}".to_sym, + attributes: queue_info[:attributes] + }) + end +end diff --git a/config/initializers/scheduled_jobs.rb b/config/initializers/scheduled_jobs.rb index 1f706fa375a..73293371f1d 100644 --- a/config/initializers/scheduled_jobs.rb +++ b/config/initializers/scheduled_jobs.rb @@ -39,7 +39,6 @@ "update_appellant_representation_job" => UpdateAppellantRepresentationJob, "update_cached_appeals_attributes_job" => UpdateCachedAppealsAttributesJob, "warm_bgs_caches_job" => WarmBgsCachesJob, - "va_notify_status_update_job" => VANotifyStatusUpdateJob, "poll_docketed_legacy_appeals_job" => PollDocketedLegacyAppealsJob, "retrieve_and_cache_reader_documents_job" => RetrieveAndCacheReaderDocumentsJob, "travel_board_hearing_sync_job" => Hearings::TravelBoardHearingSyncJob, diff --git a/config/initializers/shoryuken.rb b/config/initializers/shoryuken.rb index 84400df8a42..bc2377beab6 100644 --- a/config/initializers/shoryuken.rb +++ b/config/initializers/shoryuken.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require "#{Rails.root}/app/jobs/middleware/job_monitoring_middleware" require "#{Rails.root}/app/jobs/middleware/job_request_store_middleware" require "#{Rails.root}/app/jobs/middleware/job_sentry_scope_middleware" @@ -9,16 +11,23 @@ .shoryuken_options(retry_intervals: [3.seconds, 30.seconds, 5.minutes, 30.minutes, 2.hours, 5.hours]) if Rails.application.config.sqs_endpoint - # override the sqs_endpoint Shoryuken::Client.sqs.config[:endpoint] = URI(Rails.application.config.sqs_endpoint) end if Rails.application.config.sqs_create_queues # create the development queues - Shoryuken::Client.sqs.create_queue({ queue_name: ActiveJob::Base.queue_name_prefix + '_low_priority' }) - Shoryuken::Client.sqs.create_queue({ queue_name: ActiveJob::Base.queue_name_prefix + '_high_priority' }) - Shoryuken::Client.sqs.create_queue({ queue_name: ActiveJob::Base.queue_name_prefix + '_send_notifications' }) - Shoryuken::Client.sqs.create_queue({ queue_name: ActiveJob::Base.queue_name_prefix + '_receive_notifications' }) + Shoryuken::Client.sqs.create_queue({ queue_name: ActiveJob::Base.queue_name_prefix + "_low_priority" }) + Shoryuken::Client.sqs.create_queue({ queue_name: ActiveJob::Base.queue_name_prefix + "_high_priority" }) + Shoryuken::Client.sqs.create_queue({ + queue_name: ( + ActiveJob::Base.queue_name_prefix + "_send_notifications.fifo" + ).to_sym, + attributes: { + "FifoQueue" => "true", + "FifoThroughputLimit" => "perQueue", + "ContentBasedDeduplication" => "false" + } + }) end Shoryuken.configure_server do |config| diff --git a/config/initializers/va_notify.rb b/config/initializers/va_notify.rb index de650eac2fc..80d05092136 100644 --- a/config/initializers/va_notify.rb +++ b/config/initializers/va_notify.rb @@ -1 +1,6 @@ -VANotifyService = (ApplicationController.dependencies_faked? ? Fakes::VANotifyService : ExternalApi::VANotifyService) \ No newline at end of file +case Rails.deploy_env +when :uat, :prod + VANotifyService = ExternalApi::VANotifyService +else + VANotifyService = Fakes::VANotifyService +end diff --git a/db/migrate/20240717145856_add_sm_sand_email_status_to_notifications.rb b/db/migrate/20240717145856_add_sm_sand_email_status_to_notifications.rb new file mode 100644 index 00000000000..ee83c718534 --- /dev/null +++ b/db/migrate/20240717145856_add_sm_sand_email_status_to_notifications.rb @@ -0,0 +1,6 @@ +class AddSmSandEmailStatusToNotifications < ActiveRecord::Migration[6.0] + def change + add_column :notifications, :sms_status_reason, :string, comment: "Context around why this VA Notify notification is in the sms status" + add_column :notifications, :email_status_reason, :string, comment: "Context around why this VA Notify notification is in the email status" + end +end diff --git a/db/schema.rb b/db/schema.rb index e66628cc6d2..f0e643b57d7 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1450,6 +1450,7 @@ t.string "email_notification_content", comment: "Full Email Text Content of Notification" t.string "email_notification_external_id", comment: "VA Notify Notification Id for the email notification send through their API " t.string "email_notification_status", comment: "Status of the Email Notification" + t.string "email_status_reason", comment: "Context around why this VA Notify notification is in the email status" t.date "event_date", null: false, comment: "Date of Event" t.string "event_type", null: false, comment: "Type of Event" t.bigint "notifiable_id" @@ -1465,6 +1466,7 @@ t.string "sms_notification_status", comment: "Status of SMS/Text Notification" t.string "sms_response_content", comment: "Message body of the sms notification response." t.datetime "sms_response_time", comment: "Date and Time of the sms notification response." + t.string "sms_status_reason", comment: "Context around why this VA Notify notification is in the sms status" t.datetime "updated_at", comment: "TImestamp of when Notification was Updated" t.index ["appeals_id", "appeals_type"], name: "index_appeals_notifications_on_appeals_id_and_appeals_type" t.index ["email_notification_external_id"], name: "index_notifications_on_email_notification_external_id" diff --git a/db/seeds/api_keys.rb b/db/seeds/api_keys.rb index 129c5fc8c86..0f27499b462 100644 --- a/db/seeds/api_keys.rb +++ b/db/seeds/api_keys.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -# create ApiKey seeds - module Seeds require "./app/models/api_key.rb" @@ -13,9 +11,10 @@ def seed! private def create_api_keys - ApiKey.create(consumer_name: "appeals_consumer", key_digest: "z1VxSVb2iae07+bYq8ZjQZs3ll4ZgSeVIUC9O5u+HfA=", - key_string: "5ecb5d7b440e429bb5fac331419c7e1a") + ApiKey.create!(consumer_name: "TestApiKey", key_string: "test") + ApiKey.create(consumer_name: "appeals_consumer", + key_digest: "z1VxSVb2iae07+bYq8ZjQZs3ll4ZgSeVIUC9O5u+HfA=", + key_string: "5ecb5d7b440e429bb5fac331419c7e1a") end end end - diff --git a/docker-compose-m1.yml b/docker-compose-m1.yml index 6d0f25a0f2b..7f8da3a3074 100644 --- a/docker-compose-m1.yml +++ b/docker-compose-m1.yml @@ -19,10 +19,9 @@ services: appeals-localstack-aws: platform: linux/amd64 container_name: localstack - image: localstack/localstack:0.11.4 + image: localstack/localstack:0.14.5 ports: - - "4567-4583:4567-4583" - - "8082:${PORT_WEB_UI-8080}" + - "4566:4566" environment: - SERVICES=sqs volumes: diff --git a/docker-compose.yml b/docker-compose.yml index 02ec69bc364..0ecd5d4fc06 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -24,10 +24,9 @@ services: appeals-localstack-aws: container_name: localstack - image: localstack/localstack:0.11.4 + image: localstack/localstack:0.14.5 ports: - - "4567-4583:4567-4583" - - "8082:${PORT_WEB_UI-8080}" + - "4566:4566" environment: - SERVICES=sqs volumes: diff --git a/lib/caseflow/error.rb b/lib/caseflow/error.rb index c60dc7bd5e9..69f46c0bdfb 100644 --- a/lib/caseflow/error.rb +++ b/lib/caseflow/error.rb @@ -510,4 +510,8 @@ def initialize(msg = "The batch size of jobs must not exceed 10") super(msg) end end + + class SqsUnexpectedQueueTypeError < StandardError; end + class SqsQueueNotFoundError < StandardError; end + class SqsQueueExhaustionError < StandardError; end end diff --git a/lib/fakes/va_notify_service.rb b/lib/fakes/va_notify_service.rb index 1dc5d00999a..af4dee700eb 100644 --- a/lib/fakes/va_notify_service.rb +++ b/lib/fakes/va_notify_service.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class Fakes::VANotifyService < ExternalApi::VANotifyService + VA_NOTIFY_ENDPOINT = "/api/v1/va_notify_update" + class << self # rubocop:disable Metrics/ParameterLists def send_email_notifications( @@ -11,7 +13,20 @@ def send_email_notifications( docket_number:, status: "" ) - fake_notification_response(email_template_id) + + external_id = SecureRandom.uuid + + unless Rails.deploy_env == :test + request = HTTPI::Request.new + request.url = "#{ENV['CASEFLOW_BASE_URL']}#{VA_NOTIFY_ENDPOINT}"\ + "?id=#{external_id}&status=delivered&to=test@example.com¬ification_type=email" + request.headers["Content-Type"] = "application/json" + request.headers["Authorization"] = "Bearer test" + + HTTPI.post(request) + end + + fake_notification_response(email_template_id, status, external_id) end def send_sms_notifications( @@ -22,11 +37,24 @@ def send_sms_notifications( docket_number:, status: "" ) + + external_id = SecureRandom.uuid + + unless Rails.deploy_env == :test + request = HTTPI::Request.new + request.url = "#{ENV['CASEFLOW_BASE_URL']}#{VA_NOTIFY_ENDPOINT}"\ + "?id=#{external_id}&status=delivered&to=+15555555555¬ification_type=sms" + request.headers["Content-Type"] = "application/json" + request.headers["Authorization"] = "Bearer test" + + HTTPI.post(request) + end + if participant_id.length.nil? return bad_participant_id_response end - fake_notification_response(sms_template_id) + fake_notification_response(sms_template_id, status, external_id) end # rubocop:enable Metrics/ParameterLists @@ -102,23 +130,23 @@ def bad_notification_response ) end - def fake_notification_response(email_template_id) + def fake_notification_response(template_id, status, external_id) HTTPI::Response.new( 200, {}, OpenStruct.new( - "id": SecureRandom.uuid, + "id": external_id, "reference": "string", "uri": "string", "template": { - "id" => email_template_id, + "id" => template_id, "version" => 0, "uri" => "string" }, "scheduled_for": "string", "content": { - "body" => "string", - "subject" => "string" + "body" => "Template: #{template_id} - Status: #{status}", + "subject" => "Test Subject" } ) ) diff --git a/lib/tasks/appeal_state_synchronizer.rake b/lib/tasks/appeal_state_synchronizer.rake index cd8e1f29149..ca1094a7c04 100644 --- a/lib/tasks/appeal_state_synchronizer.rake +++ b/lib/tasks/appeal_state_synchronizer.rake @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "#{Rails.root}/app/helpers/sync_decided_appeals_helper.rb" + namespace :appeal_state_synchronizer do desc "Used to synchronize appeal_states table using data from other sources." task sync_appeal_states: :environment do @@ -11,6 +13,12 @@ namespace :appeal_state_synchronizer do backfill_appeal_information end + task sync_legacy_appeal_decisions: :environment do + include SyncDecidedAppealsHelper + + sync_decided_appeals + end + def map_appeal_hearing_scheduled_state(appeal_state) if !appeal_state.appeal&.hearings&.empty? && appeal_state.appeal.hearings.max_by(&:scheduled_for).disposition.nil? return { hearing_scheduled: true } diff --git a/spec/controllers/api/v1/va_notify_controller_spec.rb b/spec/controllers/api/v1/va_notify_controller_spec.rb index 895b7bd04f9..a3d01684b6b 100644 --- a/spec/controllers/api/v1/va_notify_controller_spec.rb +++ b/spec/controllers/api/v1/va_notify_controller_spec.rb @@ -4,9 +4,13 @@ include ActiveJob::TestHelper before { Seeds::NotificationEvents.new.seed! } + before(:each) { wipe_queues } + after(:all) { wipe_queues } + let(:sqs_client) { SqsService.sqs_client } let(:api_key) { ApiKey.create!(consumer_name: "API Consumer").key_string } let!(:appeal) { create(:appeal) } + let!(:queue) { create_queue("receive_notifications", true) } let!(:notification_email) do create( :notification, @@ -27,7 +31,7 @@ appeals_type: "Appeal", event_date: "2023-02-27 13:11:51.91467", event_type: Constants.EVENT_TYPE_FILTERS.quarterly_notification, - notification_type: "Email", + notification_type: "Sms", notified_at: "2023-02-28 14:11:51.91467", sms_notification_external_id: "3fa85f64-5717-4562-b3fc-2c963f66afa6", sms_notification_status: "Preferences Declined" @@ -36,6 +40,66 @@ let(:default_payload) do { id: "3fa85f64-5717-4562-b3fc-2c963f66afa6", + to: "to", + status_reason: "status_reason", + body: "string", + completed_at: "2023-04-17T12:38:48.699Z", + created_at: "2023-04-17T12:38:48.699Z", + created_by_name: "string", + email_address: "user@example.com", + line_1: "string", + line_2: "string", + line_3: "string", + line_4: "string", + line_5: "string", + line_6: "string", + phone_number: "+16502532222", + postage: "string", + postcode: "string", + reference: "string", + scheduled_for: "2023-04-17T12:38:48.699Z", + sent_at: "2023-04-17T12:38:48.699Z", + sent_by: "string", + status: "created", + subject: "string", + notification_type: "Email" + } + end + + let(:error_payload1) do + { + id: "3fa85f64-5717-4562-b3fc-2c963f66afa6", + to: "to", + status_reason: nil, + body: "string", + completed_at: "2023-04-17T12:38:48.699Z", + created_at: "2023-04-17T12:38:48.699Z", + created_by_name: "string", + email_address: "user@example.com", + line_1: "string", + line_2: "string", + line_3: "string", + line_4: "string", + line_5: "string", + line_6: "string", + phone_number: "+16502532222", + postage: "string", + postcode: "string", + reference: "string", + scheduled_for: "2023-04-17T12:38:48.699Z", + sent_at: "2023-04-17T12:38:48.699Z", + sent_by: "string", + status: "created", + subject: "string", + notification_type: "Email" + } + end + + let(:error_payload2) do + { + id: nil, + to: "to", + status_reason: "status_reason", body: "string", completed_at: "2023-04-17T12:38:48.699Z", created_at: "2023-04-17T12:38:48.699Z", @@ -56,11 +120,13 @@ sent_by: "string", status: "created", subject: "string", - notification_type: "" + notification_type: "Emailx" } end context "email notification status is changed" do + before { Seeds::NotificationEvents.new.seed! } + let(:payload_email) do default_payload.deep_dup.tap do |payload| payload[:notification_type] = "email" @@ -72,7 +138,6 @@ post :notifications_update, params: payload_email perform_enqueued_jobs { ProcessNotificationStatusUpdatesJob.perform_later } - expect(notification_email.reload.email_notification_status).to eq("created") end end @@ -89,7 +154,6 @@ post :notifications_update, params: payload_sms perform_enqueued_jobs { ProcessNotificationStatusUpdatesJob.perform_later } - expect(notification_sms.reload.sms_notification_status).to eq("created") end end @@ -98,6 +162,8 @@ let(:payload_fake) do { id: "fake", + to: "to", + status_reason: "status_reason", body: "string", completed_at: "2023-04-17T12:38:48.699Z", created_at: "2023-04-17T12:38:48.699Z", @@ -133,16 +199,156 @@ } end - it "Update job raises error if UUID is passed in for a non-existant notification" do - expect_any_instance_of(ProcessNotificationStatusUpdatesJob).to receive(:log_error) do |_job, error| - expect(error.message).to eq("No notification matches UUID #{payload_fake.dig(:id)}") + it "Update job runs cleanly when UUID is missing" do + request.headers["Authorization"] = "Bearer #{api_key}" + post :notifications_update, params: payload_fake + expect(response.status).to eq(200) + + perform_enqueued_jobs { ProcessNotificationStatusUpdatesJob.perform_later } + end + end + + context "payload missing required params" do + before { Seeds::NotificationEvents.new.seed! } + + let(:payload_email) do + error_payload1.deep_dup.tap do |payload| + payload[:notification_type] = "email" end + end + it "is missing the id and properly errors out" do request.headers["Authorization"] = "Bearer #{api_key}" - post :notifications_update, params: payload_fake + post :notifications_update, params: payload_email + + expect(response.status).to eq(200) + + perform_enqueued_jobs { ProcessNotificationStatusUpdatesJob.perform_later } + end + end + + context "payload status is delivered and status_reason and to are null" do + before { Seeds::NotificationEvents.new.seed! } + let(:payload) do + error_payload1.deep_dup.tap do |payload| + payload[:status] = "delivered" + payload[:status_reason] = nil + payload[:to] = nil + end + end + + it "updates status of notification" do + request.headers["Authorization"] = "Bearer #{api_key}" + post :notifications_update, params: payload + + perform_enqueued_jobs { ProcessNotificationStatusUpdatesJob.perform_later } + expect(response.status).to eq(200) + end + end + + context "payload status is delivered and status_reason is null" do + before { Seeds::NotificationEvents.new.seed! } + let(:payload) do + error_payload1.deep_dup.tap do |payload| + payload[:status] = "delivered" + payload[:status_reason] = nil + end + end + + it "updates status of notification" do + request.headers["Authorization"] = "Bearer #{api_key}" + post :notifications_update, params: payload + + perform_enqueued_jobs { ProcessNotificationStatusUpdatesJob.perform_later } + expect(response.status).to eq(200) + end + end + + context "payload status is delivered and to is null" do + before { Seeds::NotificationEvents.new.seed! } + let(:payload) do + error_payload1.deep_dup.tap do |payload| + payload[:status] = "delivered" + payload[:to] = nil + end + end + + it "updates status of notification" do + request.headers["Authorization"] = "Bearer #{api_key}" + post :notifications_update, params: payload + + perform_enqueued_jobs { ProcessNotificationStatusUpdatesJob.perform_later } + expect(response.status).to eq(200) + end + end + + context "payload status is NOT delivered and status reason and to are null" do + before { Seeds::NotificationEvents.new.seed! } + let(:payload) do + error_payload1.deep_dup.tap do |payload| + payload[:status] = "Pending Delivery" + payload[:to] = nil + payload[:status_reason] = nil + end + end + + it "updates status of notification" do + request.headers["Authorization"] = "Bearer #{api_key}" + post :notifications_update, params: payload + + perform_enqueued_jobs { ProcessNotificationStatusUpdatesJob.perform_later } expect(response.status).to eq(200) + end + end + + context "payload status is NOT delivered and status reason is null" do + before { Seeds::NotificationEvents.new.seed! } + let(:payload) do + error_payload1.deep_dup.tap do |payload| + payload[:status] = "Pending Delivery" + payload[:status_reason] = nil + end + end + + it "updates status of notification" do + request.headers["Authorization"] = "Bearer #{api_key}" + post :notifications_update, params: payload perform_enqueued_jobs { ProcessNotificationStatusUpdatesJob.perform_later } + expect(response.status).to eq(200) end end + + context "payload status is NOT delivered and to is null" do + before { Seeds::NotificationEvents.new.seed! } + let(:payload) do + error_payload1.deep_dup.tap do |payload| + payload[:status] = "Pending Delivery" + payload[:to] = nil + end + end + + it "updates status of notification" do + request.headers["Authorization"] = "Bearer #{api_key}" + post :notifications_update, params: payload + + perform_enqueued_jobs { ProcessNotificationStatusUpdatesJob.perform_later } + expect(response.status).to eq(200) + end + end + + def create_queue(name, fifo = false) + sqs_client.create_queue({ + queue_name: "caseflow_test_#{name}#{fifo ? '.fifo' : ''}".to_sym, + attributes: fifo ? { "FifoQueue" => "true" } : {} + }) + end + + def wipe_queues + client = SqsService.sqs_client + + queues_to_delete = client.list_queues.queue_urls.filter { |url| url.include?("caseflow_test") } + + queues_to_delete.each { |queue_url| client.delete_queue(queue_url: queue_url) } + end end diff --git a/spec/helpers/sync_decided_appeals_helper_spec.rb b/spec/helpers/sync_decided_appeals_helper_spec.rb new file mode 100644 index 00000000000..b55dbe97ae7 --- /dev/null +++ b/spec/helpers/sync_decided_appeals_helper_spec.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +require_relative "../../app/helpers/sync_decided_appeals_helper" + +describe "SyncDecidedAppealsHelper" do + self.use_transactional_tests = false + + class Helper + include SyncDecidedAppealsHelper + end + + attr_reader :helper + + subject do + Helper.new + end + + context "#sync_decided_appeals" do + let(:decided_appeal_state) do + create_decided_appeal_state_with_case_record_and_hearing(true, true) + end + + let(:undecided_appeal_state) do + create_decided_appeal_state_with_case_record_and_hearing(false, true) + end + + let(:missing_vacols_case_appeal_state) do + create_decided_appeal_state_with_case_record_and_hearing(true, false) + end + + it "Job syncs decided appeals decision_mailed status", bypass_cleaner: true do + expect([decided_appeal_state, + undecided_appeal_state, + missing_vacols_case_appeal_state].all?(&:decision_mailed)).to eq false + + subject.sync_decided_appeals + + expect(decided_appeal_state.reload.decision_mailed).to eq true + expect(undecided_appeal_state.reload.decision_mailed).to eq false + expect(missing_vacols_case_appeal_state.reload.decision_mailed).to eq false + end + + it "catches standard errors", bypass_cleaner: true do + expect([decided_appeal_state, + undecided_appeal_state, + missing_vacols_case_appeal_state].all?(&:decision_mailed)).to eq false + + error_text = "Fatal error in sync_decided_appeals_helper" + allow(AppealState).to receive(:legacy).and_raise(StandardError.new(error_text)) + + expect(Rails.logger).to receive(:error) + + expect { subject.sync_decided_appeals }.to raise_error(StandardError) + end + + # Clean up parallel threads + after(:each) { clean_up_after_threads } + + # VACOLS record's decision date will be set to simulate a decided appeal + # decision_mailed will be set to false for the AppealState to verify the method + # functionality + def create_decided_appeal_state_with_case_record_and_hearing(decided_appeal, create_case) + case_hearing = create(:case_hearing) + decision_date = decided_appeal ? Time.current : nil + vacols_case = create_case ? create(:case, case_hearings: [case_hearing], bfddec: decision_date) : nil + appeal = create(:legacy_appeal, vacols_case: vacols_case) + + appeal.appeal_state.tap { _1.update!(decision_mailed: false) } + end + + def clean_up_after_threads + DatabaseCleaner.clean_with(:truncation, except: %w[vftypes issref notification_events]) + end + end +end diff --git a/spec/jobs/hearings/receive_notification_job_spec.rb b/spec/jobs/hearings/receive_notification_job_spec.rb deleted file mode 100644 index ab8a326a4ea..00000000000 --- a/spec/jobs/hearings/receive_notification_job_spec.rb +++ /dev/null @@ -1,182 +0,0 @@ -# frozen_string_literal: true - -# Testing plan: -# - 1. Create test records usiong factories and take note of notification ID and specific fields to compare -# - 2. Use custom message defined here to pass in perform method -# - 3. Test perform method by checking if field values in DB recored are equal to the field values in the message, -# - An update to record should only be called whenever there are differences between the message and the record in DB -# - 4. The updated record should be returned - -describe ReceiveNotificationJob, type: :job do - include ActiveJob::TestHelper - let(:current_user) { create(:user, roles: ["System Admin"]) } - # rubocop:disable Style/BlockDelimiters - let(:message) { - { - queue_url: "http://example_queue", - message_body: "Notification", - message_attributes: { - "id": { - data_type: "String", - string_value: "3fa85f64-5717-4562-b3fc-2c963f66afa6" - }, - "body": { - data_type: "String", - string_value: "AString" - }, - "created_at": { - data_type: "String", - string_value: "2022-09-02T20:40:11.184Z" - }, - "completed_at": { - data_type: "String", - string_value: "2022-09-02T20:40:11.184Z" - }, - "created_by_name": { - data_type: "String", - string_value: "John" - }, - "email_address": { - data_type: "String", - string_value: "user@example.com" - }, - "line_1": { - data_type: "String", - string_value: "address" - }, - "line_2": { - data_type: "String", - string_value: "address" - }, - "line_3": { - data_type: "String", - string_value: "address" - }, - "line_4": { - data_type: "String", - string_value: "address" - }, - "line_5": { - data_type: "String", - string_value: "address" - }, - "line_6": { - data_type: "String", - string_value: "address" - }, - "phone_number": { - data_type: "String", - string_value: nil - }, - "postage": { - data_type: "String", - string_value: "postage" - }, - "postcode": { - data_type: "String", - string_value: "postcode" - }, - "reference": { - data_type: "String", - string_value: "9" - }, - "scheduled_for": { - data_type: "String", - string_value: "2022-09-02T20:40:11.184Z" - }, - "sent_at": { - data_type: "String", - string_value: "2022-09-02T20:40:11.184Z" - }, - "sent_by": { - data_type: "String", - string_value: "sent-by" - }, - "status": { - data_type: "String", - string_value: "delivered" - }, - "subject": { - data_type: "String", - string_value: "subject" - }, - "type": { - string_value: "email", - data_type: "String" - } - - } - } - } - - # rubocop:enable Style/BlockDelimiters - let(:queue_name) { "caseflow_test_receive_notifications" } - - after do - clear_enqueued_jobs - clear_performed_jobs - end - - it "it is the correct queue" do - expect(ReceiveNotificationJob.new.queue_name).to eq(queue_name) - end - - context ".perform" do - # create notification event record - let(:hearing_scheduled_event) do - NotificationEvent.find_or_create_by(event_type: Constants.EVENT_TYPE_FILTERS.hearing_scheduled) do |event| - event.email_template_id = "27bf814b-f065-4fc8-89af-ae1292db894e" - event.sms_template_id = "c2798da3-4c7a-43ed-bc16-599329eaf7cc" - end - end - # create notification record - let(:notification) do - create(:notification, id: 9, appeals_id: 4, appeals_type: "Appeal", - event_type: Constants.EVENT_TYPE_FILTERS.hearing_scheduled, - participant_id: "123456789", notification_type: "Email", recipient_email: "", - event_date: Time.zone.now, email_notification_status: "Success") - end - - # add message to queue - subject(:job) { ReceiveNotificationJob.perform_later(message) } - - # make sure job count increases by 1 - describe "send message to queue" do - it "has one message in queue" do - expect { job }.to change(ActiveJob::Base.queue_adapter.enqueued_jobs, :size).by(1) - end - - # After receiving the notification (by notification id), check : if email is same, if number is still nil, - # if status changed form Success to delivered - it "updates notification" do - hearing_scheduled_event - notification - - # obtain record from compare_notification_audit_record function - record = ReceiveNotificationJob.perform_now(message) - - # run checks - expect(record.recipient_email).to eq(message[:message_attributes][:email_address][:string_value]) - expect(record.recipient_phone_number).to eq(nil) - expect(record.email_notification_status).to eq(message[:message_attributes][:status][:string_value].capitalize) - end - end - - describe "errors" do - it "logs error when message is nil" do - expect(Rails.logger).to receive(:error).with(/There was no message passed/) - perform_enqueued_jobs do - ReceiveNotificationJob.perform_later(nil) - end - end - - it "logs error when message_attributes is nil" do - message[:message_attributes] = nil - expect(Rails.logger).to receive(:error).with(/message_attributes was nil/) - perform_enqueued_jobs do - ReceiveNotificationJob.perform_later(message) - end - end - end - end -end diff --git a/spec/jobs/nightly_syncs_job_spec.rb b/spec/jobs/nightly_syncs_job_spec.rb index 700c5672e5b..2af498ffbfb 100644 --- a/spec/jobs/nightly_syncs_job_spec.rb +++ b/spec/jobs/nightly_syncs_job_spec.rb @@ -201,6 +201,23 @@ class FakeTask < Dispatch::Task expect(held_hearing_appeal_state.reload.hearing_scheduled).to eq false end + it "catches standard errors" do + expect([pending_hearing_appeal_state, + postponed_hearing_appeal_state, + withdrawn_hearing_appeal_state, + scheduled_in_error_hearing_appeal_state, + held_hearing_appeal_state].all?(&:hearing_scheduled)).to eq true + + allow(AppealState).to receive(:where).and_raise(StandardError) + slack_msg = "" + slack_msg_error_text = "Fatal error in sync_hearing_states" + allow_any_instance_of(SlackService).to receive(:send_notification) { |_, first_arg| slack_msg = first_arg } + + subject + + expect(slack_msg.include?(slack_msg_error_text)).to be true + end + # Hearing scheduled will be set to true to simulate Caseflow missing a # disposition update. def create_appeal_state_with_case_record_and_hearing(desired_disposition) @@ -213,6 +230,66 @@ def create_appeal_state_with_case_record_and_hearing(desired_disposition) end end + context "#sync_decided_appeals" do + let(:decided_appeal_state) do + create_decided_appeal_state_with_case_record_and_hearing(true, true) + end + + let(:undecided_appeal_state) do + create_decided_appeal_state_with_case_record_and_hearing(false, true) + end + + let(:missing_vacols_case_appeal_state) do + create_decided_appeal_state_with_case_record_and_hearing(true, false) + end + + it "Job syncs decided appeals decision_mailed status", bypass_cleaner: true do + expect([decided_appeal_state, + undecided_appeal_state, + missing_vacols_case_appeal_state].all?(&:decision_mailed)).to eq false + + subject + + expect(decided_appeal_state.reload.decision_mailed).to eq true + expect(undecided_appeal_state.reload.decision_mailed).to eq false + expect(missing_vacols_case_appeal_state.reload.decision_mailed).to eq false + end + + it "catches standard errors", bypass_cleaner: true do + expect([decided_appeal_state, + undecided_appeal_state, + missing_vacols_case_appeal_state].all?(&:decision_mailed)).to eq false + + allow(AppealState).to receive(:legacy).and_raise(StandardError) + slack_msg = "" + slack_msg_error_text = "Fatal error in sync_decided_appeals" + allow_any_instance_of(SlackService).to receive(:send_notification) { |_, first_arg| slack_msg = first_arg } + + subject + + expect(slack_msg.include?(slack_msg_error_text)).to be true + end + + # Clean up parallel threads + after(:each) { clean_up_after_threads } + + # VACOLS record's decision date will be set to simulate a decided appeal + # decision_mailed will be set to false for the AppealState to verify the method + # functionality + def create_decided_appeal_state_with_case_record_and_hearing(decided_appeal, create_case) + case_hearing = create(:case_hearing) + decision_date = decided_appeal ? Time.current : nil + vacols_case = create_case ? create(:case, case_hearings: [case_hearing], bfddec: decision_date) : nil + appeal = create(:legacy_appeal, vacols_case: vacols_case) + + appeal.appeal_state.tap { _1.update!(decision_mailed: false) } + end + + def clean_up_after_threads + DatabaseCleaner.clean_with(:truncation, except: %w[vftypes issref notification_events]) + end + end + context "when errors occur" do context "in the sync_vacols_cases step" do context "due to existing FK associations" do diff --git a/spec/jobs/notification_initialization_job_spec.rb b/spec/jobs/notification_initialization_job_spec.rb index 3b86fb7e74d..bf1e2d3fb15 100644 --- a/spec/jobs/notification_initialization_job_spec.rb +++ b/spec/jobs/notification_initialization_job_spec.rb @@ -49,6 +49,10 @@ ) end + before do + InitialTasksFactory.new(appeal_state.appeal).create_root_and_sub_tasks! + end + it "enqueues an SendNotificationJob" do expect { subject }.to have_enqueued_job(SendNotificationJob) end diff --git a/spec/jobs/process_decision_document_job_spec.rb b/spec/jobs/process_decision_document_job_spec.rb index f7295efe73a..ca8210c3ed5 100644 --- a/spec/jobs/process_decision_document_job_spec.rb +++ b/spec/jobs/process_decision_document_job_spec.rb @@ -1,8 +1,10 @@ # frozen_string_literal: true describe ProcessDecisionDocumentJob do + let(:contested) { true } + context ".perform" do - subject { ProcessDecisionDocumentJob.perform_now(decision_document.id) } + subject { ProcessDecisionDocumentJob.perform_now(decision_document.id, contested) } let(:decision_document) { build_stubbed(:decision_document) } diff --git a/spec/jobs/process_notification_status_updates_job_spec.rb b/spec/jobs/process_notification_status_updates_job_spec.rb index a5f94379961..e40699a0e6f 100644 --- a/spec/jobs/process_notification_status_updates_job_spec.rb +++ b/spec/jobs/process_notification_status_updates_job_spec.rb @@ -3,107 +3,163 @@ describe ProcessNotificationStatusUpdatesJob, type: :job do include ActiveJob::TestHelper - let(:redis) do - # Creates a fresh Redis connection before each test and deletes all keys in the store - Redis.new(url: Rails.application.secrets.redis_url_cache).tap(&:flushall) - end + before(:each) { wipe_queues } + after(:all) { wipe_queues } + + let(:sqs_client) { SqsService.sqs_client } context ".perform" do before { Seeds::NotificationEvents.new.seed! } subject(:job) { ProcessNotificationStatusUpdatesJob.perform_later } - let(:new_status) { "test_status" } let(:appeal) { create(:appeal, veteran_file_number: "500000102", receipt_date: 6.months.ago.to_date.mdY) } + + let(:email_external_id) { SecureRandom.uuid } let(:email_notification) do create(:notification, appeals_id: appeal.uuid, appeals_type: "Appeal", event_date: 6.days.ago, event_type: Constants.EVENT_TYPE_FILTERS.quarterly_notification, notification_type: "Email", - email_notification_external_id: SecureRandom.uuid) + email_notification_external_id: email_external_id) end + + let(:sms_external_id) { SecureRandom.uuid } let(:sms_notification) do create(:notification, appeals_id: appeal.uuid, appeals_type: "Appeal", event_date: 6.days.ago, event_type: Constants.EVENT_TYPE_FILTERS.hearing_scheduled, - sms_notification_external_id: SecureRandom.uuid, + sms_notification_external_id: sms_external_id, notification_type: "SMS") end - it "has one message in queue" do - expect { job }.to change(ActiveJob::Base.queue_adapter.enqueued_jobs, :size).by(1) - end - - it "processes email notifications from redis cache" do - expect(email_notification.email_notification_status).to_not eq(new_status) - - create_cache_entries(email_notification) - - expect(redis.keys.grep(/email_update:/).count).to eq(1) - - perform_enqueued_jobs { ProcessNotificationStatusUpdatesJob.perform_later } - - expect(redis.keys.grep(/email_update:/).count).to eq(0) - expect(email_notification.reload.email_notification_status).to eq(new_status) + let(:sms_notification_2) do + create(:notification, appeals_id: appeal.uuid, + appeals_type: "Appeal", + event_date: 6.days.ago, + event_type: Constants.EVENT_TYPE_FILTERS.postponement_of_hearing, + sms_notification_external_id: "1234", + notification_type: "SMS") end - it "processes sms notifications from redis cache" do - expect(sms_notification.sms_notification_status).to_not eq(new_status) - - create_cache_entries(sms_notification) - - expect(redis.keys.grep(/sms_update:/).count).to eq(1) - - perform_enqueued_jobs { ProcessNotificationStatusUpdatesJob.perform_later } - - expect(redis.keys.grep(/sms_update:/).count).to eq(0) - expect(sms_notification.reload.sms_notification_status).to eq(new_status) + it "has one message in queue" do + expect { job }.to change(ActiveJob::Base.queue_adapter.enqueued_jobs, :size).by(1) end - it "processes a mix of email and sms notifications from redis cache" do - create_cache_entries(sms_notification, email_notification) - - expect(redis.keys.grep(/(sms|email)_update:/).count).to eq(2) - - perform_enqueued_jobs { ProcessNotificationStatusUpdatesJob.perform_later } - - expect(redis.keys.grep(/(sms|email)_update:/).count).to eq(0) - expect(email_notification.reload.email_notification_status).to eq(new_status) - expect(sms_notification.reload.sms_notification_status).to eq(new_status) + context "Updates are pulled from the SQS queue and applied to the datebase" do + let(:recipient_email) { "test@test.com" } + let(:email_status) { "delivered" } + let(:email_status_reason) { "Email delivery was succesful" } + + let(:recipient_phone) { "123-456-7890" } + let(:sms_status) { "temporary-failure" } + let(:sms_status_reason) { "Provider is retrying." } + + let(:test_queue) do + sqs_client.create_queue({ + queue_name: "caseflow_test_receive_notifications.fifo".to_sym, + attributes: { + "FifoQueue" => "true" + } + }) + end + let(:queue_url) { test_queue.queue_url } + let!(:sms_sqs_message) do + sqs_client.send_message( + queue_url: queue_url, + message_body: { + notification_type: "sms", + external_id: sms_external_id, + status: sms_status, + status_reason: sms_status_reason, + recipient: recipient_phone + }.to_json, + message_deduplication_id: "1", + message_group_id: ProcessNotificationStatusUpdatesJob::MESSAGE_GROUP_ID + ) + end + + let!(:sms_sqs_message_wrong_group_id) do + sqs_client.send_message( + queue_url: queue_url, + message_body: { + notification_type: "sms", + external_id: "1234", + status: sms_status, + status_reason: sms_status_reason, + recipient: recipient_phone + }.to_json, + message_deduplication_id: "2", + message_group_id: "SomethingElse" + ) + end + + let!(:email_sqs_message) do + sqs_client.send_message( + queue_url: queue_url, + message_body: { + notification_type: "email", + external_id: email_external_id, + status: email_status, + status_reason: email_status_reason, + recipient: recipient_email + }.to_json, + message_deduplication_id: "3", + message_group_id: ProcessNotificationStatusUpdatesJob::MESSAGE_GROUP_ID + ) + end + + it "Status update info from messages with correct group ID is persisted correctly" do + expect(all_message_info_empty?).to eq true + + perform_enqueued_jobs { job } + + # Reload records + [email_notification, sms_notification, sms_notification_2].each(&:reload) + + expect(email_notification.email_notification_status).to eq email_status + expect(email_notification.email_status_reason).to eq email_status_reason + expect(email_notification.recipient_email).to eq recipient_email + + expect(sms_notification.sms_notification_status).to eq sms_status + expect(sms_notification.sms_status_reason).to eq sms_status_reason + expect(sms_notification.recipient_phone_number).to eq recipient_phone + + # Update with the wrong message_group_id should have been skipped. + expect([ + sms_notification_2.sms_notification_status, + sms_notification_2.sms_status_reason, + sms_notification_2.recipient_phone_number + ].all?(&:nil?)).to eq true + end end + end - it "an error is raised if a UUID doesn't match with a notification record, but the job isn't halted" do - expect_any_instance_of(ProcessNotificationStatusUpdatesJob).to receive(:log_error) do |_job, error| - expect(error.message).to eq("No notification matches UUID not-going-to-match") - end.exactly(:once) - - # This notification update will cause an error - redis.set("sms_update:not-going-to-match:#{new_status}", 0) - - # This notification update should be fine - create_cache_entries(email_notification) - - expect(redis.keys.grep(/(sms|email)_update:/).count).to eq(2) - - perform_enqueued_jobs { ProcessNotificationStatusUpdatesJob.perform_later } - - expect(sms_notification.reload.sms_notification_status).to be_nil - expect(email_notification.reload.email_notification_status).to eq(new_status) - - expect(redis.keys.grep(/(sms|email)_update:/).count).to eq(0) - end + def all_message_info_empty? + [ + email_notification.email_notification_status, + email_notification.email_status_reason, + email_notification.recipient_email + ].all?(&:nil?) && + [ + sms_notification.sms_notification_status, + sms_notification.sms_status_reason, + sms_notification.recipient_phone_number + ].all?(&:nil?) && + [ + sms_notification_2.sms_notification_status, + sms_notification_2.sms_status_reason, + sms_notification_2.recipient_phone_number + ].all?(&:nil?) end - private + def wipe_queues + client = SqsService.sqs_client - def create_cache_entries(*keys) - keys.each do |key| - notification_type = key.notification_type.downcase - external_id = key.send("#{notification_type}_notification_external_id".to_sym) + queues_to_delete = client.list_queues.queue_urls.filter { |url| url.include?("caseflow_test") } - redis.set("#{notification_type}_update:#{external_id}:#{new_status}", 0) - end + queues_to_delete.each { |queue_url| client.delete_queue(queue_url: queue_url) } end end diff --git a/spec/jobs/quarterly_notifications_job_spec.rb b/spec/jobs/quarterly_notifications_job_spec.rb index f7ba2c0a12b..55a368e53c7 100644 --- a/spec/jobs/quarterly_notifications_job_spec.rb +++ b/spec/jobs/quarterly_notifications_job_spec.rb @@ -2,6 +2,7 @@ describe QuarterlyNotificationsJob, type: :job do include ActiveJob::TestHelper + let(:appeal) { create(:appeal, :active) } let(:legacy_appeal) { create(:legacy_appeal, vacols_case: vacols_case) } let(:vacols_case) { create(:case) } @@ -48,6 +49,7 @@ it "pushes a new message" do expect_message_to_be_queued + expect_message_to_have_status("docketed") subject end @@ -68,6 +70,7 @@ it "pushes a new message" do expect_message_to_be_queued + expect_message_to_have_status("docketed") subject end @@ -89,6 +92,7 @@ it "pushes a new message" do expect_message_to_be_queued + expect_message_to_have_status("Privacy Act Pending") subject end @@ -109,6 +113,7 @@ it "pushes a new message" do expect_message_to_be_queued + expect_message_to_have_status("docketed") subject end @@ -123,13 +128,14 @@ created_by_id: user.id, updated_by_id: user.id, appeal_docketed: true, - hearing_withdrawn: true, - scheduled_in_error: true + scheduled_in_error: true, + privacy_act_pending: true ) end it "pushes a new message" do expect_message_to_be_queued + expect_message_to_have_status("Privacy Act Pending") subject end @@ -150,26 +156,7 @@ it "pushes a new message" do expect_message_to_be_queued - - subject - end - end - - context "Hearing Scheduled / Privacy Act Pending with ihp task" do - let(:hearing) { create(:hearing, :with_tasks) } - let!(:appeal_state) do - hearing.appeal.appeal_state.tap do - _1.update!( - appeal_docketed: true, - hearing_scheduled: true, - privacy_act_pending: true, - vso_ihp_pending: true - ) - end - end - - it "pushes a new message" do - expect_message_to_be_queued + expect_message_to_have_status("docketed") subject end @@ -191,25 +178,7 @@ it "pushes a new message" do expect_message_to_be_queued - - subject - end - end - - context "Hearing Scheduled with ihp task pending" do - let(:hearing) { create(:hearing, :with_tasks) } - let!(:appeal_state) do - hearing.appeal.appeal_state.tap do - _1.update!( - appeal_docketed: true, - hearing_scheduled: true, - vso_ihp_pending: true - ) - end - end - - it "pushes a new message" do - expect_message_to_be_queued + expect_message_to_have_status("VSO IHP Pending / Privacy Act Pending") subject end @@ -229,6 +198,7 @@ it "pushes a new message" do expect_message_to_be_queued + expect_message_to_have_status("Hearing Scheduled / Privacy Act Pending") subject end @@ -249,6 +219,7 @@ it "pushes a new message" do expect_message_to_be_queued + expect_message_to_have_status("Privacy Act Pending") subject end @@ -269,6 +240,7 @@ it "pushes a new message" do expect_message_to_be_queued + expect_message_to_have_status("VSO IHP Pending") subject end @@ -286,6 +258,7 @@ it "pushes a new message" do expect_message_to_be_queued + expect_message_to_have_status("Hearing Scheduled") subject end @@ -339,6 +312,17 @@ def expect_message_to_be_queued ) end + def expect_message_to_have_status(status) + expect_any_instance_of(NotificationInitializationJob) + .to receive(:initialize) + .with({ + appeal_id: appeal_state.appeal_id, + appeal_type: appeal_state.appeal_type, + template_name: Constants.EVENT_TYPE_FILTERS.quarterly_notification, + appeal_status: status + }) + end + def expect_message_to_not_be_enqueued expect_any_instance_of(QuarterlyNotificationsJob) .to_not receive(:enqueue_init_jobs) diff --git a/spec/jobs/send_notification_job_spec.rb b/spec/jobs/send_notification_job_spec.rb index 32ebb4b8296..cde49138139 100644 --- a/spec/jobs/send_notification_job_spec.rb +++ b/spec/jobs/send_notification_job_spec.rb @@ -178,21 +178,11 @@ context "#queue_name_suffix" do subject { described_class.queue_name_suffix } - it "returns non-FIFO name in development environment" do - is_expected.to eq :send_notifications - end - - it "returns FIFO name in non-development environment" do - allow(ApplicationController).to receive(:dependencies_faked?).and_return(false) - + it "returns FIFO name" do is_expected.to eq :"send_notifications.fifo" end end - it "it is the correct queue" do - expect(SendNotificationJob.new.queue_name).to eq(queue_name) - end - context ".perform" do subject(:job) { SendNotificationJob.perform_later(good_message.to_json) } diff --git a/spec/jobs/sync_reviews_job_spec.rb b/spec/jobs/sync_reviews_job_spec.rb index a285d531439..369c93b2e8d 100644 --- a/spec/jobs/sync_reviews_job_spec.rb +++ b/spec/jobs/sync_reviews_job_spec.rb @@ -143,7 +143,7 @@ SyncReviewsJob.perform_now end.to have_enqueued_job( ProcessDecisionDocumentJob - ).with(decision_document_needs_reprocessing.id).exactly(:once) + ).with(decision_document_needs_reprocessing.id, false).exactly(:once) end end end diff --git a/spec/jobs/va_notify_status_update_job_spec.rb b/spec/jobs/va_notify_status_update_job_spec.rb deleted file mode 100644 index 4c6f2c65842..00000000000 --- a/spec/jobs/va_notify_status_update_job_spec.rb +++ /dev/null @@ -1,271 +0,0 @@ -# frozen_string_literal: true - -describe VANotifyStatusUpdateJob, type: :job do - include ActiveJob::TestHelper - let(:current_user) { create(:user, roles: ["System Admin"]) } - let(:notifications_email_only) do - FactoryBot.create_list :notification_email_only, 10 - end - let(:notifications_sms_only) do - FactoryBot.create_list :notification_sms_only, 10 - end - let(:notifications_email_and_sms) do - FactoryBot.create_list :notification_email_and_sms, 10 - end - let(:email_only) do - create(:notification, - appeals_id: "5d70058f-8641-4155-bae8-5af4b61b1576", - appeals_type: "Appeal", - event_type: Constants.EVENT_TYPE_FILTERS.hearing_scheduled, - event_date: Time.zone.today, - notification_type: "Email", - email_notification_status: "Success") - end - let(:sms_only) do - create(:notification, - appeals_id: "5d70058f-8641-4155-bae8-5af4b61b1576", - appeals_type: "Appeal", - event_type: Constants.EVENT_TYPE_FILTERS.hearing_scheduled, - event_date: Time.zone.today, - notification_type: "SMS", - sms_notification_status: "Success") - end - let(:email_and_sms) do - create(:notification, - appeals_id: "5d70058f-8641-4155-bae8-5af4b61b1576", - appeals_type: "Appeal", - event_type: Constants.EVENT_TYPE_FILTERS.hearing_scheduled, - event_date: Time.zone.today, - notification_type: "Email and SMS", - email_notification_status: "Success", - sms_notification_status: "Success") - end - let(:notification_collection) do - create(:notification, - appeals_id: "5d70058f-8641-4155-bae8-5af4b61b1576", - appeals_type: "Appeal", - event_type: Constants.EVENT_TYPE_FILTERS.hearing_scheduled, - event_date: Time.zone.today, - notification_type: "Email", - email_notification_external_id: "0", - sms_notification_external_id: nil, - email_notification_status: "Success", - created_at: Time.zone.now) - create(:notification, - appeals_id: "5d70058f-8641-4155-bae8-5af4b61b1576", - appeals_type: "Appeal", - event_type: Constants.EVENT_TYPE_FILTERS.hearing_scheduled, - event_date: Time.zone.today, - notification_type: "SMS", - email_notification_external_id: nil, - sms_notification_external_id: "0", - sms_notification_status: "temporary-failure", - created_at: Time.zone.now) - create(:notification, - appeals_id: "5d70058f-8641-4155-bae8-5af4b61b1576", - appeals_type: "Appeal", - event_type: Constants.EVENT_TYPE_FILTERS.hearing_scheduled, - event_date: Time.zone.today, - notification_type: "SMS", - email_notification_external_id: nil, - sms_notification_external_id: "1", - sms_notification_status: "created", - created_at: Time.zone.now) - create(:notification, - appeals_id: "5d70058f-8641-4155-bae8-5af4b61b1576", - appeals_type: "Appeal", - event_type: Constants.EVENT_TYPE_FILTERS.hearing_scheduled, - event_date: Time.zone.today, - notification_type: "Email", - email_notification_external_id: "1", - sms_notification_external_id: nil, - email_notification_status: "technical-failure", - created_at: Time.zone.now) - create(:notification, - appeals_id: "5d70058f-8641-4155-bae8-5af4b61b1576", - appeals_type: "Appeal", - event_type: Constants.EVENT_TYPE_FILTERS.hearing_scheduled, - event_date: Time.zone.today, - notification_type: "Email and SMS", - email_notification_external_id: "2", - sms_notification_external_id: "2", - email_notification_status: "temporary-failure", - sms_notification_status: "temporary-failure", - created_at: Time.zone.now - 5.days) - create(:notification, - appeals_id: "5d70058f-8641-4155-bae8-5af4b61b1576", - appeals_type: "Appeal", - event_type: Constants.EVENT_TYPE_FILTERS.hearing_scheduled, - event_date: Time.zone.today, - notification_type: "Email and SMS", - email_notification_external_id: "3", - sms_notification_external_id: "3", - email_notification_status: "delivered", - sms_notification_status: "delivered", - created_at: Time.zone.now - 5.days) - create(:notification, - appeals_id: "5d70058f-8641-4155-bae8-5af4b61b1577", - appeals_type: "Appeal", - event_type: Constants.EVENT_TYPE_FILTERS.hearing_scheduled, - event_date: Time.zone.today, - notification_type: "Email and SMS", - email_notification_external_id: "4", - sms_notification_external_id: "4", - email_notification_status: "delivered", - sms_notification_status: "delivered", - created_at: Time.zone.now - 5.days) - end - - let(:collect) { Notification.where(id: [1, 2, 3, 4, 5]) } - - let(:queue_name) { "caseflow_test_low_priority" } - - before do - Seeds::NotificationEvents.new.seed! - end - - after(:each) do - clear_enqueued_jobs - clear_performed_jobs - end - - it "it is the correct queue" do - expect(VANotifyStatusUpdateJob.new.queue_name).to eq(queue_name) - end - - context ".perform" do - subject(:job) { VANotifyStatusUpdateJob.perform_later } - describe "send message to queue" do - it "has one message in queue" do - expect { job }.to change(ActiveJob::Base.queue_adapter.enqueued_jobs, :size).by(1) - end - - it "processes message" do - perform_enqueued_jobs do - result = VANotifyStatusUpdateJob.perform_later - expect(result.arguments[0]).to eq(nil) - end - end - - it "sends to VA Notify when no errors are present" do - expect(Rails.logger).not_to receive(:error) - expect { VANotifyStatusUpdateJob.perform_now.to receive(:send_to_va_notify) } - end - - it "defaults to 650 for the query limit if environment variable not found or invalid" do - stub_const("VANotifyStatusUpdateJob::QUERY_LIMIT", nil) - expect(Rails.logger).to receive(:info) - .with("VANotifyStatusJob can not read the VA_NOTIFY_STATUS_UPDATE_BATCH_LIMIT environment variable.\ - Defaulting to 650.") - VANotifyStatusUpdateJob.perform_now - end - - it "logs out an error to Raven when email type that is not Email or SMS is found" do - external_id = SecureRandom.uuid - email_only.update!(email_notification_external_id: external_id) - job_instance = VANotifyStatusUpdateJob.new - external_id = SecureRandom.uuid - result = job_instance.send(:get_current_status, external_id, "None") - expect(result).to eq(false) - end - end - - describe "feature flags" do - describe "Email" do - it "updates the Notification when successful" do - email_only.email_notification_external_id = SecureRandom.uuid - allow(job).to receive(:notifications_not_processed).and_return([email_only]) - job.perform_now - expect(email_only.email_notification_status).to eq("created") - end - it "logs when external id is not present" do - allow(job).to receive(:notifications_not_processed).and_return([email_only]) - job.perform_now - expect(email_only.email_notification_status).to eq("No External Id") - end - end - - describe "SMS" do - it "updates the Notification when successful" do - sms_only.sms_notification_external_id = SecureRandom.uuid - allow(job).to receive(:notifications_not_processed).and_return([sms_only]) - job.perform_now - expect(sms_only.sms_notification_status).to eq("created") - end - it "logs when external id is not present" do - allow(job).to receive(:notifications_not_processed).and_return([sms_only]) - job.perform_now - expect(sms_only.sms_notification_status).to eq("No External Id") - end - end - - describe "Email and SMS" do - it "updates the Notification when successful" do - email_and_sms.sms_notification_external_id = SecureRandom.uuid - email_and_sms.email_notification_external_id = SecureRandom.uuid - allow(job).to receive(:notifications_not_processed).and_return([email_and_sms]) - job.perform_now - expect(email_and_sms.sms_notification_status && email_and_sms.email_notification_status).to eq("created") - end - it "logs when external id is not present" do - allow(job).to receive(:notifications_not_processed).and_return([email_and_sms]) - job.perform_now - expect(email_and_sms.sms_notification_status && - email_and_sms.email_notification_status).to eq("No External Id") - end - - it "updates the email and sms notification status if an external id is found" do - email_and_sms.update!(sms_notification_external_id: SecureRandom.uuid, - email_notification_external_id: SecureRandom.uuid) - job.perform_now - notification = Notification.first - expect(notification.email_notification_status && notification.sms_notification_status).to eq("created") - end - end - end - end - - context "#get_current_status" do - subject(:job) { VANotifyStatusUpdateJob.perform_later } - it "handles VA Notify errors" do - email_and_sms.sms_notification_external_id = SecureRandom.uuid - email_and_sms.email_notification_external_id = SecureRandom.uuid - allow(job).to receive(:notifications_not_processed).and_return([email_and_sms]) - allow(VANotifyService).to receive(:get_status).and_raise(Caseflow::Error::VANotifyNotFoundError) - expect(job).to receive(:log_error).with(/VA Notify API returned error/).twice - job.perform_now - end - end - - context "#notifications_not_processed" do - subject(:job) { VANotifyStatusUpdateJob.perform_later } - it "queries the notification table using activerecord" do - allow(job).to receive(:find_notifications_not_processed).and_return([]) - expect(job.send(:find_notifications_not_processed)) - job.perform_now - end - end - - context "#find_notif_not_processed" do - subject(:job) { VANotifyStatusUpdateJob.perform_later } - it "returns a collection of notifications from the DB that hold the qualifying statuses" do - notification_collection - expect(job.send(:find_notifications_not_processed)).not_to include(Notification.where(id: [6, 7])) - end - end - - context "#default_to_650" do - before do - VANotifyStatusUpdateJob::QUERY_LIMIT = nil - end - - subject(:job) { VANotifyStatusUpdateJob.perform_later } - it "defaults to 650" do - expect(Rails.logger).to receive(:info).with( - "VANotifyStatusJob can not read the VA_NOTIFY_STATUS_UPDATE_BATCH_LIMIT environment variable.\ - Defaulting to 650." - ) - job.perform - end - end -end diff --git a/spec/models/appellant_notification_spec.rb b/spec/models/appellant_notification_spec.rb index 9b09924d134..b04d34c41b2 100644 --- a/spec/models/appellant_notification_spec.rb +++ b/spec/models/appellant_notification_spec.rb @@ -3,7 +3,7 @@ describe AppellantNotification do describe "class methods" do describe "self.handle_errors" do - let(:appeal) { create(:appeal, number_of_claimants: 1) } + let(:appeal) { create(:appeal, :active, number_of_claimants: 1) } let(:current_user) { User.system_user } context "if appeal is nil" do let(:empty_appeal) {} @@ -15,7 +15,7 @@ end context "with no claimant listed" do - let(:appeal) { create(:appeal, number_of_claimants: 0) } + let(:appeal) { create(:appeal, :active, number_of_claimants: 0) } it "returns error message" do expect(AppellantNotification.handle_errors(appeal)[:status]).to eq( AppellantNotification::NoClaimantError.new(appeal.id).status @@ -25,7 +25,7 @@ context "with no participant_id listed" do let(:claimant) { create(:claimant, participant_id: "") } - let(:appeal) { create(:appeal) } + let(:appeal) { create(:appeal, :active) } before do appeal.claimants = [claimant] end @@ -36,6 +36,16 @@ end end + context "with an inactive appeal" do + let(:appeal) { create(:appeal, :active, number_of_claimants: 1) } + it "returns error message" do + appeal.root_task.completed! + expect { AppellantNotification.handle_errors(appeal) }.to raise_error( + AppellantNotification::InactiveAppealError + ) + end + end + context "with no errors" do it "doesn't raise" do expect(AppellantNotification.handle_errors(appeal)[:status]).to eq "Success" @@ -44,7 +54,7 @@ end describe "veteran is deceased" do - let(:appeal) { create(:appeal, number_of_claimants: 1) } + let(:appeal) { create(:appeal, :active, number_of_claimants: 1) } let(:substitute_appellant) { create(:appellant_substitution) } it "with no substitute appellant" do @@ -62,8 +72,8 @@ end describe "self.create_payload" do - let(:good_appeal) { create(:appeal, number_of_claimants: 1) } - let(:bad_appeal) { create(:appeal) } + let(:good_appeal) { create(:appeal, :active, number_of_claimants: 1) } + let(:bad_appeal) { create(:appeal, :active) } let(:bad_claimant) { create(:claimant, participant_id: "") } let(:template_name) { "test" } @@ -148,14 +158,14 @@ it "Will notify appellant that the legacy appeal decision has been mailed (Non Contested)" do expect(AppellantNotification).to receive(:notify_appellant).with(legacy_appeal, non_contested) decision_document = dispatch.send dispatch_func, params - decision_document.process! + decision_document.process!(false) end it "Will notify appellant that the legacy appeal decision has been mailed (Contested)" do expect(AppellantNotification).to receive(:notify_appellant).with(legacy_appeal, contested) allow(legacy_appeal).to receive(:contested_claim).and_return(true) legacy_appeal.contested_claim decision_document = dispatch.send dispatch_func, params - decision_document.process! + decision_document.process!(true) end end @@ -205,7 +215,7 @@ it "Will notify appellant that the AMA appeal decision has been mailed (Non Contested)" do expect(AppellantNotification).to receive(:notify_appellant).with(appeal, non_contested) decision_document = dispatch.send dispatch_func, params - decision_document.process! + decision_document.process!(false) end it "Will notify appellant that the AMA appeal decision has been mailed (Contested)" do expect(AppellantNotification).to receive(:notify_appellant).with(contested_appeal, contested) @@ -213,7 +223,7 @@ contested_appeal.contested_claim? contested_decision_document = contested_dispatch .send dispatch_func, contested_params - contested_decision_document.process! + contested_decision_document.process!(true) end end end @@ -570,7 +580,7 @@ # Note: only privacyactrequestmailtask is tested because the process is the same as foiarequestmailtask describe "mail task" do - let(:appeal) { create(:appeal) } + let(:appeal) { create(:appeal, :active) } let(:appeal_state) { create(:appeal_state, appeal_id: appeal.id, appeal_type: appeal.class.to_s) } let(:current_user) { create(:user) } let(:priv_org) { PrivacyTeam.singleton } @@ -640,7 +650,7 @@ end context "Foia Colocated Tasks" do - let(:appeal) { create(:appeal) } + let(:appeal) { create(:appeal, :active) } let(:appeal_state) { create(:appeal_state, appeal_id: appeal.id, appeal_type: appeal.class.to_s) } let!(:attorney) { create(:user) } let!(:attorney_task) { create(:ama_attorney_task, appeal: appeal, assigned_to: attorney) } @@ -691,7 +701,7 @@ end context "Privacy Act Tasks" do - let(:appeal) { create(:appeal) } + let(:appeal) { create(:appeal, :active) } let(:appeal_state) { create(:appeal_state, appeal_id: appeal.id, appeal_type: appeal.class.to_s) } let(:attorney) { create(:user) } let(:current_user) { create(:user) } @@ -929,6 +939,12 @@ let(:task) { create(:informal_hearing_presentation_task, :in_progress, assigned_to: org) } let(:appeal_state) { create(:appeal_state, appeal_id: task.appeal.id, appeal_type: task.appeal.class.to_s) } let(:template_name) { Constants.EVENT_TYPE_FILTERS.vso_ihp_complete } + let(:appeal) { task.appeal } + + before do + InitialTasksFactory.new(appeal).create_root_and_sub_tasks! + end + it "will notify the appellant of the 'IhpTaskComplete' status" do allow(task).to receive(:verify_user_can_update!).with(user).and_return(true) expect(AppellantNotification).to receive(:notify_appellant).with(task.appeal, template_name) diff --git a/spec/models/decision_document_spec.rb b/spec/models/decision_document_spec.rb index a90aba992cb..9896879c68d 100644 --- a/spec/models/decision_document_spec.rb +++ b/spec/models/decision_document_spec.rb @@ -133,7 +133,7 @@ end context "#process!" do - subject { decision_document.process! } + subject { decision_document.process!(false) } before do allow(decision_document).to receive(:submitted_and_ready?).and_return(true) @@ -141,6 +141,7 @@ allow(VBMSService).to receive(:establish_claim!).and_call_original allow(VBMSService).to receive(:create_contentions!).and_call_original FeatureToggle.enable!(:send_email_for_dispatched_appeals) + InitialTasksFactory.new(decision_document.appeal).create_root_and_sub_tasks! end after { FeatureToggle.disable!(:send_email_for_dispatched_appeals) } diff --git a/spec/models/tasks/bva_dispatch_task_spec.rb b/spec/models/tasks/bva_dispatch_task_spec.rb index 9124d11eaf8..7e612ef0e51 100644 --- a/spec/models/tasks/bva_dispatch_task_spec.rb +++ b/spec/models/tasks/bva_dispatch_task_spec.rb @@ -113,7 +113,7 @@ decision_document = DecisionDocument.find_by(appeal_id: root_task.appeal.id) expect(ProcessDecisionDocumentJob).to have_received(:perform_later) - .with(decision_document.id, nil).exactly(:once) + .with(decision_document.id, false, nil).exactly(:once) expect(decision_document).to_not eq nil expect(decision_document.document_type).to eq "BVA Decision" expect(decision_document.source).to eq "BVA" @@ -144,7 +144,7 @@ decision_document = DecisionDocument.find_by(appeal_id: legacy_appeal.id) expect(ProcessDecisionDocumentJob).to have_received(:perform_later) - .with(decision_document.id, nil).exactly(:once) + .with(decision_document.id, false, nil).exactly(:once) expect(decision_document).to_not eq nil expect(decision_document.document_type).to eq "BVA Decision" expect(decision_document.source).to eq "BVA" @@ -248,7 +248,7 @@ decision_document = DecisionDocument.find_by(appeal_id: root_task.appeal.id) expect(ProcessDecisionDocumentJob).to have_received(:perform_later) - .with(decision_document.id, nil).exactly(:once) + .with(decision_document.id, false, nil).exactly(:once) expect(decision_document).to_not eq nil expect(decision_document.document_type).to eq "BVA Decision" expect(decision_document.source).to eq "BVA" diff --git a/spec/services/sqs_service_spec.rb b/spec/services/sqs_service_spec.rb new file mode 100644 index 00000000000..9322989ff54 --- /dev/null +++ b/spec/services/sqs_service_spec.rb @@ -0,0 +1,136 @@ +# frozen_string_literal: true + +describe SqsService do + let(:sqs_client) { SqsService.sqs_client } + + before(:each) { wipe_queues } + after(:all) { wipe_queues } + + context "#find_queue_url_by_name" do + let!(:queue) { create_queue(queue_name, fifo) } + + subject { SqsService.find_queue_url_by_name(name: queue_name, check_fifo: false) } + + context "FIFO" do + let(:fifo) { true } + let(:queue_name) { "my_fifo_queue" } + + it "the queue is found and is validated to be a FIFO queue" do + expect(subject { SqsService.find_queue_url_by_name(name: queue_name, check_fifo: true) }) + .to include("caseflow_test_my_fifo_queue.fifo") + end + + it "the queue is found while validation is opted out" do + is_expected.to include("caseflow_test_my_fifo_queue.fifo") + end + + it "a non-existent queue cannot be found" do + expect { SqsService.find_queue_url_by_name(name: "fake", check_fifo: false) }.to raise_error do |error| + expect(error).to be_a(Caseflow::Error::SqsQueueNotFoundError) + expect(error.to_s).to include("The fake SQS queue is missing in this environment.") + end + end + end + + context "non-FIFO" do + let(:fifo) { false } + let(:queue_name) { "my_normal_queue" } + + it "the queue is found" do + is_expected.to include("caseflow_test_my_normal_queue") + is_expected.to_not include(".fifo") + end + + it "the queue found fails the FIFO check" do + expect { SqsService.find_queue_url_by_name(name: queue_name, check_fifo: true) }.to raise_error do |error| + expect(error).to be_a(Caseflow::Error::SqsUnexpectedQueueTypeError) + expect(error.to_s).to include("No FIFO queue with name my_normal_queue could be located.") + end + end + end + end + + context "#batch_delete_messages" do + let!(:queue) { create_queue("batch_delete_test", false) } + let(:queue_url) { queue.queue_url } + + context "ten or fewer messages are deleted" do + let!(:initial_messages) { queue_messages(queue_url) } + let(:received_messages) do + SqsService.sqs_client.receive_message({ + queue_url: queue_url, + max_number_of_messages: 10 + }).messages + end + + it "the messages are deleted properly" do + expect(approximate_number_of_messages_in_queue(queue_url)).to eq 10 + + SqsService.batch_delete_messages(queue_url: queue_url, messages: received_messages) + + expect(approximate_number_of_messages_in_queue(queue_url)).to eq 0 + end + end + + context "more than ten messages are deleted" + let!(:initial_messages) { queue_messages(queue_url, 20) } + + let(:received_messages) do + Array.new(2).flat_map do + SqsService.sqs_client.receive_message( + { + queue_url: queue_url, + max_number_of_messages: 10 + } + ).messages + end + end + + it "the messages are deleted properly" do + expect(approximate_number_of_messages_in_queue(queue_url)).to eq 20 + + SqsService.batch_delete_messages(queue_url: queue.queue_url, messages: received_messages) + + expect(approximate_number_of_messages_in_queue(queue_url)).to eq 0 + end + end + + def create_queue(name, fifo = false) + sqs_client.create_queue({ + queue_name: "caseflow_test_#{name}#{fifo ? '.fifo' : ''}".to_sym, + attributes: fifo ? { "FifoQueue" => "true" } : {} + }) + end + + def queue_messages(queue_url, num_to_queue = 10) + bodies = Array.new(num_to_queue).map.with_index do |_val, idx| + { test: idx }.to_json + end + + bodies.each do |body| + sqs_client.send_message({ + queue_url: queue_url, + message_body: body + }) + end + end + + def approximate_number_of_messages_in_queue(queue_url) + resp = sqs_client.get_queue_attributes({ + queue_url: queue_url, + attribute_names: ["ApproximateNumberOfMessages"] + }) + + resp.attributes["ApproximateNumberOfMessages"].to_i + end + + def wipe_queues + client = SqsService.sqs_client + + queues_to_delete = client.list_queues.queue_urls.filter { _1.include?("caseflow_test") } + + queues_to_delete.each do |queue_url| + client.delete_queue(queue_url: queue_url) + end + end +end diff --git a/spec/workflows/ihp_tasks_factory_spec.rb b/spec/workflows/ihp_tasks_factory_spec.rb index 29f185ee4eb..7c80b37f763 100644 --- a/spec/workflows/ihp_tasks_factory_spec.rb +++ b/spec/workflows/ihp_tasks_factory_spec.rb @@ -3,7 +3,7 @@ require "rails_helper" describe IhpTasksFactory, :postgres do - let(:appeal) { create(:appeal) } + let(:appeal) { create(:appeal, :active) } let(:parent_task) { create(:task, appeal: appeal) } let(:ihp_tasks_factory) { IhpTasksFactory.new(parent_task) }