Merge pull request #1475 from department-of-veterans-affairs/Daniel/A…

…PPEAL-34625

hotfix/APPEALS-34625

app/jobs/v2/download_manifest_job.rb

@@ -2,7 +2,7 @@ class V2::DownloadManifestJob < ApplicationJob
   queue_as :high_priority
 
   def perform(manifest_source, user = nil)
-    return if manifest_source.current?
+    manifest_source.update(status: :pending)
 
     RequestStore.store[:current_user] = user if user
     Raven.extra_context(manifest_source: manifest_source.id)

app/models/manifest.rb

@@ -29,7 +29,7 @@ class Manifest < ApplicationRecord
 
   def start!
     # Reset stale manifests.
-    update!(fetched_files_status: :initialized) if stale?
+    update!(fetched_files_status: :initialized) if ready_for_refresh?
 
     vbms_source.start!
     vva_source.start! unless FeatureToggle.enabled?(:skip_vva)
@@ -54,9 +54,9 @@ def download_and_package_files!
     end
   end
 
-  # completed? because it can be recent pending with stale fetched_files_at
-  def stale?
-    completed? && fetched_files_at && fetched_files_at < UI_HOURS_UNTIL_EXPIRY.hours.ago
+  # ready_for_refresh? we want to refresh the manifest after 72 hours regardless of the state
+  def ready_for_refresh?
+    !initialized? && fetched_files_at && fetched_files_at < UI_HOURS_UNTIL_EXPIRY.hours.ago
   end
 
   def completed?

app/models/manifest_source.rb

@@ -17,24 +17,13 @@ class ManifestSource < ApplicationRecord
 
   delegate :file_number, to: :manifest
 
-  SECONDS_TO_AUTO_UNLOCK = 5
-
   def start!
-    s = Redis::Semaphore.new("download_manifest_source_#{id}".to_s,
-                             url: Rails.application.secrets.redis_url_cache,
-                             expiration: SECONDS_TO_AUTO_UNLOCK)
-    s.lock(SECONDS_TO_AUTO_UNLOCK) do
-      return if current? || pending?
-      update(status: :pending)
-    end
-
-    begin
-      V2::DownloadManifestJob.perform_later(self, RequestStore[:current_user])
-    rescue StandardError
-      update(status: :initialized)
+    return if current? || processing?
+    V2::DownloadManifestJob.perform_later(self, RequestStore[:current_user])
+  rescue StandardError
+    update(status: :initialized)
 
-      raise
-    end
+    raise
   end
 
   def service
@@ -53,4 +42,8 @@ def expiry_hours
   def current?
     success? && fetched_at && fetched_at > expiry_hours.hours.ago
   end
+
+  def processing?
+    pending? && fetched_at && fetched_at > 24.hours.ago
+  end
 end

app/services/manifest_fetcher.rb

@@ -6,15 +6,15 @@ class ManifestFetcher
   EXCEPTIONS = [VBMS::ClientError, VVA::ClientError].freeze
 
   def process
-    begin
-      DocumentCreator.new(manifest_source: manifest_source, external_documents: documents).create
-      manifest_source.update!(status: :success, fetched_at: Time.zone.now)
-      documents
-    rescue *EXCEPTIONS => e
-      manifest_source.update!(status: :failed)
-      ExceptionLogger.capture(e)
-      []
-    end
+    DocumentCreator.new(manifest_source: manifest_source, external_documents: documents).create
+    manifest_source.update!(status: :success, fetched_at: Time.zone.now)
+    documents
+  rescue *EXCEPTIONS => e
+    manifest_source.update!(status: :failed)
+    ExceptionLogger.capture(e)
+    raise e
+  ensure
+    return documents
   end
 
   def documents

spec/models/manifest_source_spec.rb

@@ -82,9 +82,9 @@
       end
     end
 
-    context "when manifest is pending" do
+    context "when manifest is pending less than 24 hours" do
       before do
-        source.update_attributes!(fetched_at: Time.zone.now - 7.hours, status: :pending)
+        source.update_attributes!(fetched_at: Time.zone.now - 3.hours, status: :pending)
       end
 
       it "does not start the manifest job" do
@@ -93,6 +93,17 @@
       end
     end
 
+    context "when manifest is pending more than 24 hours" do
+      before do
+        source.update_attributes!(fetched_at: Time.zone.now - 25.hours, status: :pending)
+      end
+
+      it "starts the manifest job" do
+        subject
+        expect(V2::DownloadManifestJob).to have_received(:perform_later)
+      end
+    end
+
     context "when fetched less than 3 hours ago" do
       before do
         source.update_attributes!(fetched_at: Time.zone.now - 2.hours, status: :success)

spec/services/manifest_fetcher_spec.rb

@@ -70,8 +70,8 @@
           allow(VBMSService).to receive(:v2_fetch_documents_for).and_raise(VBMS::ClientError)
         end
 
-        it "saves manifest status as failed" do
-          expect(subject).to eq []
+        it "saves manifest status as failed and propagates errors" do
+          expect{ subject }.to raise_error(VBMS::ClientError)
           expect(source.reload.status).to eq "failed"
           expect(source.reload.fetched_at).to be_nil
         end
@@ -99,7 +99,7 @@
         end
 
         it "saves manifest status as failed" do
-          expect(subject).to eq []
+          expect { subject }.to raise_error(VVA::ClientError)
           expect(source.reload.status).to eq "failed"
           expect(source.reload.fetched_at).to be_nil
         end