Merge branch 'master' into sally/13483-async-withdrawal-bug

.reek.yml

@@ -22,6 +22,7 @@ detectors:
       - HearingRepository#slot_new_hearing
       - LegacyDocket#distribute_appeals
       - Seeds::Tasks#create_legacy_case_with_open_schedule_hearing_task
+      - SlackService#pick_color
       - Task#actions_allowable?
       - TaskSorter#sort_requires_case_norm?
       - VBMSCaseflowLogger#log
@@ -165,6 +166,7 @@ detectors:
     exclude:
       - AddressMapper#get_address_from_veteran_record
       - ClaimReviewAsyncStatsReporter#seconds_to_hms
+      - ETLClasses#syncer_klasses
       - ETL::Builder#last_built
       - ETL::HearingSyncer#filter?
       - ETL::LegacyHearingSyncer#filter?

Gemfile

@@ -8,7 +8,7 @@ gem "aasm", "4.11.0"
 gem "activerecord-import"
 gem "acts_as_tree"
 # BGS
-gem "bgs", git: "https://github.com/department-of-veterans-affairs/ruby-bgs.git", ref: "40561cdf7fc0b939d811f4bc489c598f42f5d51c"
+gem "bgs", git: "https://github.com/department-of-veterans-affairs/ruby-bgs.git", ref: "b6d05017344a861b4b858c037f8b277e25ea84f2"
 # Bootsnap speeds up app boot (and started to be a default gem in 5.2).
 gem "bootsnap", require: false
 gem "business_time", "~> 0.9.3"
@@ -45,9 +45,7 @@ gem "pg", platforms: :ruby
 # Puma was chosen because it handles load of 40+ concurrent users better than Unicorn and Passenger
 # Discussion: https://github.com/18F/college-choice/issues/597#issuecomment-139034834
 gem "puma", "~> 3.12.4"
-# rack versions before 2.0.6 are affected by CVE-2018-16470 and CVE-2018-16471.
-# Explicitly define rack version here to avoid that.
-gem "rack", "~> 2.0.6"
+gem "rack", "~> 2.2.0"
 gem "rails", "5.2.4.2"
 # Used to colorize output for rake tasks
 gem "rainbow"

Gemfile.lock

@@ -47,12 +47,12 @@ GIT
 
 GIT
   remote: https://github.com/department-of-veterans-affairs/ruby-bgs.git
-  revision: 40561cdf7fc0b939d811f4bc489c598f42f5d51c
-  ref: 40561cdf7fc0b939d811f4bc489c598f42f5d51c
+  revision: b6d05017344a861b4b858c037f8b277e25ea84f2
+  ref: b6d05017344a861b4b858c037f8b277e25ea84f2
   specs:
     bgs (0.2)
       httpclient
-      nokogiri (~> 1.10.4)
+      nokogiri (~> 1.10.8)
       savon (~> 2.12)
 
 GIT
@@ -382,7 +382,7 @@ GEM
       pry (~> 0.10)
     public_suffix (3.1.1)
     puma (3.12.4)
-    rack (2.0.9)
+    rack (2.2.2)
     rack-contrib (2.1.0)
       rack (~> 2.0)
     rack-test (1.1.0)
@@ -650,7 +650,7 @@ DEPENDENCIES
   pry
   pry-byebug
   puma (~> 3.12.4)
-  rack (~> 2.0.6)
+  rack (~> 2.2.0)
   rails (= 5.2.4.2)
   rails-erd
   rainbow

app/controllers/api/docs/v3/decision_reviews.yaml

@@ -1170,6 +1170,7 @@ components:
       type: string
       enum:
         - compensation
+        - compensation_all
         - pension
         - fiduciary
         - insurance
@@ -1199,14 +1200,21 @@ components:
     NonratingIssueCategory:
       type: string
       enum:
-        # for benefit type:  compensation
+        # for benefit type:  compensation_all
         - Unknown issue category
         - Apportionment
         - Incarceration Adjustments
         - Audit Error Worksheet (DFAS)
         - Active Duty Adjustments
         - Drill Pay Adjustments
         - Character of discharge determinations
+        - Contested Claims (other than apportionment)
+        - Constested Claims - Apportionment
+        - Constested Claims - Attorney fees
+        - Constested Claims - Dependents Indemnity Compensation (DIC)
+        - Constested Claims - Insurance
+        - Constested Claims - Loan Guaranty (LGY)
+        - Constested Claims - Veteran death
         - Income/net worth (pension)
         - Dependent child - Adopted
         - Dependent child - Stepchild
@@ -1218,6 +1226,24 @@ components:
         - Contested Claims (other than apportionment)
         - Lack of Qualifying Service
         - Other non-rated
+        # for benefit type:  compensation
+        - Unknown issue category
+        - Apportionment
+        - Incarceration Adjustments
+        - Audit Error Worksheet (DFAS)
+        - Active Duty Adjustments
+        - Drill Pay Adjustments
+        - Character of discharge determinations
+        - Income/net worth (pension)
+        - Dependent child - Adopted
+        - Dependent child - Stepchild
+        - Dependent child - Biological
+        - Dependency Spouse - Common law marriage
+        - Dependency Spouse - Inference of marriage
+        - Dependency Spouse - Deemed valid marriage
+        - Military Retired Pay
+        - Lack of Qualifying Service
+        - Other non-rated
         # for benefit type:  pension
         - Eligibility | Wartime service
         - Eligibility | Veteran Status

app/controllers/certification_cancellations_controller.rb

@@ -16,6 +16,10 @@ def create
 
   private
 
+  def set_application
+    RequestStore.store[:application] = :certification
+  end
+
   def certification_cancellation_params
     params.require(:certification_cancellation).permit(:certification_id, :cancellation_reason, :other_reason, :email)
   end

app/controllers/concerns/collect_data_dog_metrics.rb

@@ -13,11 +13,12 @@ def collect_data_dog_metrics
   end
 
   def collect_postgres_metrics
-    conns = ActiveRecord::Base.connection_pool.connections
-
-    active = conns.count { |c| c.in_use? && c.owner.alive? }
-    dead = conns.count { |c| c.in_use? && !c.owner.alive? }
-    idle = conns.count { |c| !c.in_use? }
+    # safe-navigating c.owner&.alive? even though c.in_use? is aliased to
+    # c.owner, because we were occasionally seeing the following error:
+    # NoMethodError: undefined method 'alive?' for nil:NilClass
+    active = ActiveRecord::Base.connection_pool.connections.count { |c| c.in_use? && c.owner&.alive? }
+    dead = ActiveRecord::Base.connection_pool.connections.count { |c| c.in_use? && !c.owner&.alive? }
+    idle = ActiveRecord::Base.connection_pool.connections.count { |c| !c.in_use? }
 
     emit_datadog_point("postgres", "active", active)
     emit_datadog_point("postgres", "dead", dead)

app/controllers/intakes_controller.rb

@@ -92,7 +92,8 @@ def index_props
         editContentionText: FeatureToggle.enabled?(:edit_contention_text, user: current_user),
         unidentifiedIssueDecisionDate: FeatureToggle.enabled?(:unidentified_issue_decision_date, user: current_user),
         covidTimelinessExemption: FeatureToggle.enabled?(:covid_timeliness_exemption, user: current_user),
-        verifyUnidentifiedIssue: FeatureToggle.enabled?(:verify_unidentified_issue, user: current_user)
+        verifyUnidentifiedIssue: FeatureToggle.enabled?(:verify_unidentified_issue, user: current_user),
+        attorneyFees: FeatureToggle.enabled?(:attorney_fees, user: current_user)
       }
     }
   rescue StandardError => error

app/controllers/judge_assign_tasks_controller.rb

@@ -11,7 +11,7 @@ def create
   private
 
   def tasks
-    @tasks ||= ActiveRecord::Base.multi_transaction do
+    @tasks ||= ActiveRecord::Base.transaction do
       create_params.map do |create_param|
         judge_assign_task = JudgeAssignTask.find(create_param[:parent_id])
         AttorneyTaskCreator.new(judge_assign_task, create_param).call

app/controllers/tasks_controller.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+# Controller that handles requests about tasks.
+# Often used by Caseflow Queue.
 class TasksController < ApplicationController
   include Errors
 
@@ -97,6 +99,14 @@ def update
     tasks_to_return = (QueueForRole.new(user_role).create(user: current_user).tasks + tasks).uniq
 
     render json: { tasks: json_tasks(tasks_to_return) }
+  rescue AssignHearingDispositionTask::HearingAssociationMissing => error
+    Raven.capture_exception(error)
+    render json: {
+      "errors": [
+        "title": "Missing Associated Hearing",
+        "detail": error
+      ]
+    }, status: :bad_request
   end
 
   def for_appeal
@@ -147,13 +157,12 @@ def queue_config
   end
 
   def verify_view_access
-    return true unless FeatureToggle.enabled?(:scm_view_judge_assign_queue)
-
     return true if user == current_user || Judge.new(current_user).attorneys.include?(user)
 
-    if !SpecialCaseMovementTeam.singleton.user_has_access?(current_user)
-      fail Caseflow::Error::ActionForbiddenError, message: "Only accessible by members of the Case Movement Team."
-    end
+    return true if FeatureToggle.enabled?(:scm_view_judge_assign_queue) &&
+                   current_user.member_of_organization?(SpecialCaseMovementTeam.singleton)
+
+    fail Caseflow::Error::ActionForbiddenError, message: "Only accessible by members of the Case Movement Team."
   end
 
   def verify_task_access

app/controllers/work_modes_controller.rb

@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+# Controller to allow work modes (e.g., overtime) to be modified.
+# Only allows judges who have an assigned task on this appeal to toggle overtime.
+
+class WorkModesController < ApplicationController
+  before_action :validate_modification_access_to_overtime
+
+  def create
+    appeal.overtime = overtime_param
+    render json: { work_mode: appeal.work_mode }
+  rescue Caseflow::Error::WorkModeCouldNotUpdateError
+    render json: { params: params, work_mode: appeal.work_mode }, status: :internal_server_error
+  end
+
+  private
+
+  def overtime_param
+    ActiveRecord::Type::Boolean.new.deserialize(params.require(:overtime))
+  end
+
+  def appeal
+    @appeal ||= Appeal.find_appeal_by_id_or_find_or_create_legacy_appeal_by_vacols_id(params[:appeal_id])
+  end
+
+  def validate_modification_access_to_overtime
+    fail(Caseflow::Error::ActionForbiddenError) unless FeatureToggle.enabled?(:overtime_revamp)
+
+    unless current_user.judge? && current_user.appeal_has_task_assigned_to_user?(appeal)
+      msg = "Only judges assigned to this appeal can toggle overtime status"
+      fail(Caseflow::Error::ActionForbiddenError, message: msg)
+    end
+    true
+  end
+end

app/jobs/application_job.rb

@@ -21,7 +21,7 @@ def application_attr(app_name)
     attr_reader :app_name
   end
 
-  rescue_from VBMS::ClientError, BGS::ShareError do |error|
+  rescue_from Caseflow::Error::TransientError, VBMS::ClientError, BGS::ShareError do |error|
     capture_exception(error: error)
   end
 

app/jobs/data_integrity_checks_job.rb

@@ -13,6 +13,7 @@ class DataIntegrityChecksJob < CaseflowJob
     PendingIncompleteAndUncancelledTaskTimersChecker
     ReviewsWithDuplicateEpErrorChecker
     StuckAppealsChecker
+    StuckVirtualHearingsChecker
     UntrackedLegacyAppealsChecker
   ].freeze
 
@@ -39,7 +40,13 @@ def perform
 
   private
 
+  def report_msg(msg)
+    return "[WARN] #{msg}" unless msg.match?(/\[(INFO|WARN|ERROR)\]/)
+
+    msg
+  end
+
   def send_to_slack(checker)
-    slack_service.send_notification(checker.report, checker.class.name, checker.slack_channel)
+    slack_service.send_notification(report_msg(checker.report), checker.class.name, checker.slack_channel)
   end
 end

app/jobs/etl_builder_job.rb

@@ -7,23 +7,36 @@ class ETLBuilderJob < CaseflowJob
   application_attr :etl
 
   SLACK_CHANNEL = "#appeals-delta"
-  DATADOG_NAME = "etl_builder_job"
 
   def perform
     RequestStore.store[:current_user] = User.system_user
 
+    sweep_etl
     build_etl
+    datadog_report_runtime(metric_group_name: "etl_builder_job")
   end
 
   private
 
+  def sweep_etl
+    start = Time.zone.now
+    swept = ETL::Sweeper.new.call
+    datadog_report_time_segment(segment: "etl_sweeper", start_time: start)
+
+    return unless swept > 0
+
+    msg = "[INFO] ETL swept up #{swept} deleted records -- see logs for details"
+    slack_service.send_notification(msg, "ETL::Sweeper", SLACK_CHANNEL)
+  end
+
   def build_etl
+    start = Time.zone.now
     etl_build = ETL::Builder.new.incremental
+    datadog_report_time_segment(segment: "etl_builder", start_time: start)
 
-    datadog_report_runtime(metric_group_name: DATADOG_NAME)
-
-    msg = "ETL failed to sync any records"
+    return unless etl_build.built == 0
 
-    slack_service.send_notification(msg, self.class.to_s, SLACK_CHANNEL) if etl_build.built == 0
+    msg = "[WARN] ETL failed to sync any records"
+    slack_service.send_notification(msg, self.class.to_s, SLACK_CHANNEL)
   end
 end

app/jobs/stats_collector_job.rb

@@ -96,7 +96,7 @@ def log_success
     msg = "#{self.class.name} completed after running for #{duration}."
     Rails.logger.info(msg)
 
-    slack_service.send_notification(msg)
+    slack_service.send_notification("[INFO] #{msg}") # may not need this
   end
 
   def log_error(collector_name, err)
@@ -107,6 +107,6 @@ def log_error(collector_name, err)
 
     Raven.capture_exception(err, [:stats_collector_name] => collector_name)
 
-    slack_service.send_notification(msg)
+    slack_service.send_notification("[ERROR] #{msg}")
   end
 end

app/jobs/update_appellant_representation_job.rb

@@ -101,7 +101,7 @@ def log_error(start_time, err)
 
     Raven.capture_exception(err)
 
-    slack_service.send_notification(msg)
+    slack_service.send_notification("[ERROR] #{msg}")
 
     datadog_report_runtime(metric_group_name: METRIC_GROUP_NAME)
   end

app/jobs/update_cached_appeals_attributes_job.rb

@@ -202,7 +202,7 @@ def increment_appeal_count(count, appeal_type)
   def log_error(start_time, err)
     duration = time_ago_in_words(start_time)
     msg = "UpdateCachedAppealsAttributesJob failed after running for #{duration}. Fatal error: #{err.message}"
-    slack_msg = "UpdateCachedAppealsAttributesJob failed after running for #{duration}. See Sentry for error"
+    slack_msg = "[ERROR] UpdateCachedAppealsAttributesJob failed after running for #{duration}. See Sentry for error"
 
     Rails.logger.info(msg)
     Rails.logger.info(err.backtrace.join("\n"))