AppealsApi: Convert updater jobs to flipper (#11680)

* AppealsApi: Add & use Flipper flags for status update jobs

* AppealsApi: Remove unused Settings

* Remove old throttling code

config/features.yml

@@ -225,6 +225,18 @@ features:
     actor_type: user
     description: Enables the supplemental claim PII expunge job
     enable_in_development: true
+  decision_review_hlr_status_updater_enabled:
+      actor_type: user
+      description: Enables the Higher Level Review status update batch job
+      enable_in_development: true
+  decision_review_nod_status_updater_enabled:
+    actor_type: user
+    description: Enables the Notice of Disagreement status update batch job
+    enable_in_development: true
+  decision_review_sc_status_updater_enabled:
+    actor_type: user
+    description: Enables the Supplemental Claim status update batch job
+    enable_in_development: true
   decision_review_icn_updater_enabled:
     actor_type: user
     description: Enables the ICN lookup job

config/settings.yml

@@ -281,40 +281,7 @@ modules_appeals_api:
       # List of flags to indicate parts of documentation that are still WIP and not ready for production.
       # See the rswag:appeals_api:dev rake task, spec_helper, and DocHelpers#wip_doc_enabled? for usage info to opt-in.
       # NOTE: Do not remove this key even if it's empty.
-  higher_level_review_updater_enabled: true
   legacy_appeals_enabled: true
-  supplemental_claims_enabled: true
-  notice_of_disagreement_updater_enabled: true
-  supplemental_claim_updater_enabled: true
-  job_schedules:
-    appeals_api_daily_report:
-      class: AppealsApi::DecisionReviewReportDaily
-      description: "Daily report of appeals submissions"
-      cron: "0 23 * * MON-FRI America/New_York"
-    appeals_api_weekly_report:
-      class: AppealsApi::DecisionReviewReportWeekly
-      description: "Weekly report of appeals submissions"
-      cron: "0 23 * * SUN America/New_York"
-    appeals_api_daily_error_report:
-      class: AppealsApi::DailyErrorReport
-      description: "Daily report of appeals errors"
-      cron: "0 23 * * MON-FRI America/New_York"
-    appeals_api_hlr_cleanup_pii:
-      class: AppealsApi::HigherLevelReviewCleanUpWeekOldPii
-      description: "Remove PII of HigherLevelReviews"
-      every: ['24h', first_in: '45m']
-    appeals_api_hlr_status_batch:
-      class: AppealsApi::HigherLevelReviewUploadStatusBatch
-      description: "Update HigherLevelReview statuses with their Central Mail status"
-      every: "60m"
-    appeals_api_nod_cleanup_pii:
-      class: AppealsApi::NoticeOfDisagreementCleanUpWeekOldPii
-      description: "Remove PII of NoticeOfDisagreements"
-      every: ['24h', first_in: '45m']
-    appeals_api_nod_status_batch:
-      class: AppealsApi::NoticeOfDisagreementUploadStatusBatch
-      description: "Update NoticeOfDisagreement statuses with their Central Mail status"
-      every: "60m"
   reports:
     daily_decision_review:
       enabled: false

config/settings/test.yml

@@ -88,7 +88,6 @@ modules_appeals_api:
   documentation:
     path_enabled_flag: true
   legacy_appeals_enabled: true
-  supplemental_claims_enabled: true
 
 bgs:
   mock_response_location: /cache/bgs

modules/appeals_api/app/workers/appeals_api/higher_level_review_upload_status_batch.rb

@@ -9,11 +9,6 @@ class HigherLevelReviewUploadStatusBatch
     # No need to retry since the schedule will run this every hour
     sidekiq_options retry: false, unique_for: 30.minutes
 
-    # Throttling works by grabbing a majority of the oldest records, but also some of the newest.
-    # This way more recent records won't appear to "stall out" while we play catch-up.
-    THROTTLED_OLDEST_LIMIT = 40
-    THROTTLED_NEWEST_LIMIT = 10
-
     BATCH_SIZE = 100
 
     def perform
@@ -29,34 +24,11 @@ def perform
     private
 
     def higher_level_review_ids
-      relation = HigherLevelReview.v2.incomplete_statuses
-
-      @higher_level_review_ids ||= if Flipper.enabled?(:decision_review_hlr_status_update_throttling)
-                                     throttled_ids relation
-                                   else
-                                     unthrottled_ids relation
-                                   end
-    end
-
-    def unthrottled_ids(relation)
-      relation.order(created_at: :asc).pluck(:id)
-    end
-
-    def throttled_ids(relation)
-      ids = relation.order(created_at: :asc).limit(THROTTLED_OLDEST_LIMIT).pluck(:id)
-      if ids.size < THROTTLED_OLDEST_LIMIT
-        Rails.logger.warn('AppealsApi::HigherLevelReviewUploadStatusBatch::ThrottleWarning',
-                          'throttle_limit' => THROTTLED_OLDEST_LIMIT,
-                          'actual_count' => ids.size)
-        return ids
-      end
-
-      ids += relation.order(created_at: :desc).limit(THROTTLED_NEWEST_LIMIT).pluck(:id)
-      ids.uniq
+      @higher_level_review_ids ||= HigherLevelReview.v2.incomplete_statuses.order(created_at: :asc).pluck(:id)
     end
 
     def enabled?
-      Settings.modules_appeals_api.higher_level_review_updater_enabled
+      Flipper.enabled? :decision_review_hlr_status_updater_enabled
     end
   end
 end

modules/appeals_api/app/workers/appeals_api/notice_of_disagreement_upload_status_batch.rb

@@ -28,7 +28,7 @@ def notice_of_disagreement_ids
     end
 
     def enabled?
-      Settings.modules_appeals_api.notice_of_disagreement_updater_enabled
+      Flipper.enabled? :decision_review_nod_status_updater_enabled
     end
   end
 end

modules/appeals_api/app/workers/appeals_api/supplemental_claim_upload_status_batch.rb

@@ -28,7 +28,7 @@ def supplemental_claim_ids
     end
 
     def enabled?
-      Settings.modules_appeals_api.supplemental_claim_updater_enabled
+      Flipper.enabled? :decision_review_sc_status_updater_enabled
     end
   end
 end

modules/appeals_api/config/routes.rb

@@ -57,19 +57,15 @@
 
       get 'legacy_appeals', to: 'legacy_appeals#index' if Settings.modules_appeals_api.legacy_appeals_enabled
 
-      if Settings.modules_appeals_api.supplemental_claims_enabled
-        resources :supplemental_claims, only: %i[index create show] do
-          collection do
-            get 'schema'
-            post 'validate'
-          end
+      resources :supplemental_claims, only: %i[index create show] do
+        collection do
+          get 'schema'
+          post 'validate'
         end
       end
 
-      if Settings.modules_appeals_api.supplemental_claims_enabled
-        namespace :supplemental_claims do
-          resources :evidence_submissions, only: %i[create show]
-        end
+      namespace :supplemental_claims do
+        resources :evidence_submissions, only: %i[create show]
       end
     end
   end

modules/appeals_api/spec/workers/higher_level_review_upload_status_batch_spec.rb

@@ -23,12 +23,12 @@
     end
 
     context 'when status updater is enabled' do
+      before { Flipper.enable :decision_review_hlr_status_updater_enabled }
+
       it 'updates all the statuses' do
-        with_settings(Settings.modules_appeals_api, higher_level_review_updater_enabled: true) do
-          Sidekiq::Testing.inline! { AppealsApi::HigherLevelReviewUploadStatusBatch.new.perform }
-          upload.reload
-          expect(upload.status).to eq('processing')
-        end
+        Sidekiq::Testing.inline! { AppealsApi::HigherLevelReviewUploadStatusBatch.new.perform }
+        upload.reload
+        expect(upload.status).to eq('processing')
       end
 
       context 'with HLRv1 records' do
@@ -47,71 +47,19 @@
         let(:upload) { create(:higher_level_review_v2, status: 'pending', created_at: '2021-02-03') }
 
         it 'updates their status' do
-          with_settings(Settings.modules_appeals_api, higher_level_review_updater_enabled: true) do
-            Sidekiq::Testing.inline! { AppealsApi::HigherLevelReviewUploadStatusBatch.new.perform }
-            upload.reload
-            expect(upload.status).to eq('processing')
-          end
-        end
-      end
-
-      context 'throttling flag' do
-        let!(:uploads) { create_list :higher_level_review_v2, 12, status: :submitted }
-        let(:cmp_status) { 'Complete' }
-
-        before do
-          stub_const("#{described_class}::THROTTLED_OLDEST_LIMIT", 5)
-          stub_const("#{described_class}::THROTTLED_NEWEST_LIMIT", 5)
-        end
-
-        context 'is enabled' do
-          before { Flipper.enable :decision_review_hlr_status_update_throttling }
-
-          it 'only updates a limited number of records each run' do
-            Sidekiq::Testing.inline! { AppealsApi::HigherLevelReviewUploadStatusBatch.new.perform }
-            tally = AppealsApi::HigherLevelReview.all.pluck(:status).tally
-            expect(tally['complete']).to eq 10
-            expect(tally['submitted']).to eq 2
-
-            # Run again and ensure we pick up the remaining that were skipped by throttling
-            Sidekiq::Testing.inline! { AppealsApi::HigherLevelReviewUploadStatusBatch.new.perform }
-            tally = AppealsApi::HigherLevelReview.all.pluck(:status).tally
-            expect(tally['complete']).to eq 12
-          end
-
-          it 'logs a warning when throttling has caught up to current records' do
-            allow(Rails.logger).to receive(:warn)
-            Sidekiq::Testing.inline! { AppealsApi::HigherLevelReviewUploadStatusBatch.new.perform }
-            expect(Rails.logger).not_to have_received(:warn)
-
-            Sidekiq::Testing.inline! { AppealsApi::HigherLevelReviewUploadStatusBatch.new.perform }
-            expect(Rails.logger).to have_received(:warn).with(
-              'AppealsApi::HigherLevelReviewUploadStatusBatch::ThrottleWarning',
-              'throttle_limit' => 5,
-              'actual_count' => 2
-            )
-          end
-        end
-
-        context 'is disabled' do
-          before { Flipper.disable :decision_review_hlr_status_update_throttling }
-
-          it 'updates all records each run' do
-            Sidekiq::Testing.inline! { AppealsApi::HigherLevelReviewUploadStatusBatch.new.perform }
-            tally = AppealsApi::HigherLevelReview.all.pluck(:status).tally
-            expect(tally['complete']).to eq 12
-          end
+          Sidekiq::Testing.inline! { AppealsApi::HigherLevelReviewUploadStatusBatch.new.perform }
+          upload.reload
+          expect(upload.status).to eq('processing')
         end
       end
     end
 
     context 'when status updater is disabled' do
       it 'does not update statuses' do
-        with_settings(Settings.modules_appeals_api, higher_level_review_updater_enabled: false) do
-          Sidekiq::Testing.inline! { AppealsApi::HigherLevelReviewUploadStatusBatch.new.perform }
-          upload.reload
-          expect(upload.status).to eq('submitted')
-        end
+        Flipper.disable :decision_review_hlr_status_updater_enabled
+        Sidekiq::Testing.inline! { AppealsApi::HigherLevelReviewUploadStatusBatch.new.perform }
+        upload.reload
+        expect(upload.status).to eq('submitted')
       end
     end
   end

modules/appeals_api/spec/workers/notice_of_disagreement_upload_status_batch_spec.rb

@@ -24,12 +24,12 @@
     end
 
     context 'when status updater is enabled' do
+      before { Flipper.enable :decision_review_nod_status_updater_enabled }
+
       it 'updates all the statuses' do
-        with_settings(Settings.modules_appeals_api, notice_of_disagreement_updater_enabled: true) do
-          Sidekiq::Testing.inline! { AppealsApi::NoticeOfDisagreementUploadStatusBatch.new.perform }
-          upload.reload
-          expect(upload.status).to eq('processing')
-        end
+        Sidekiq::Testing.inline! { AppealsApi::NoticeOfDisagreementUploadStatusBatch.new.perform }
+        upload.reload
+        expect(upload.status).to eq('processing')
       end
 
       # There was a time where "success" was the final status, then that changed to "complete", so make sure
@@ -39,21 +39,19 @@
         let(:cmp_status) { 'VBMS Complete' }
 
         it 'updates beyond success status and into complete' do
-          with_settings(Settings.modules_appeals_api, notice_of_disagreement_updater_enabled: true) do
-            Sidekiq::Testing.inline! { AppealsApi::NoticeOfDisagreementUploadStatusBatch.new.perform }
-            upload.reload
-            expect(upload.status).to eq('complete')
-          end
+          Sidekiq::Testing.inline! { AppealsApi::NoticeOfDisagreementUploadStatusBatch.new.perform }
+          upload.reload
+          expect(upload.status).to eq('complete')
         end
       end
     end
 
     context 'when status updater is disabled' do
       it 'does not update statuses' do
-        with_settings(Settings.modules_appeals_api, notice_of_disagreement_updater_enabled: false) do
-          Sidekiq::Testing.inline! { AppealsApi::NoticeOfDisagreementUploadStatusBatch.new.perform }
-          expect(upload.status).to eq('submitted')
-        end
+        Flipper.disable :decision_review_nod_status_updater_enabled
+        Sidekiq::Testing.inline! { AppealsApi::NoticeOfDisagreementUploadStatusBatch.new.perform }
+        upload.reload
+        expect(upload.status).to eq('submitted')
       end
     end
   end

modules/appeals_api/spec/workers/supplemental_claim_upload_status_batch_spec.rb

@@ -23,21 +23,21 @@
     end
 
     context 'when status updater is enabled' do
+      before { Flipper.enable :decision_review_sc_status_updater_enabled }
+
       it 'updates all the statuses' do
-        with_settings(Settings.modules_appeals_api, supplemental_claim_updater_enabled: true) do
-          Sidekiq::Testing.inline! { AppealsApi::SupplementalClaimUploadStatusBatch.new.perform }
-          upload.reload
-          expect(upload.status).to eq('processing')
-        end
+        Sidekiq::Testing.inline! { AppealsApi::SupplementalClaimUploadStatusBatch.new.perform }
+        upload.reload
+        expect(upload.status).to eq('processing')
       end
     end
 
     context 'when status updater is disabled' do
       it 'does not update statuses' do
-        with_settings(Settings.modules_appeals_api, supplemental_claim_updater_enabled: false) do
-          Sidekiq::Testing.inline! { AppealsApi::SupplementalClaimUploadStatusBatch.new.perform }
-          expect(upload.status).to eq('submitted')
-        end
+        Flipper.disable :decision_review_sc_status_updater_enabled
+        Sidekiq::Testing.inline! { AppealsApi::SupplementalClaimUploadStatusBatch.new.perform }
+        upload.reload
+        expect(upload.status).to eq('submitted')
       end
     end
   end