From 0b8e00f489ef0f5657a4a5f99f4800a2e3b1358a Mon Sep 17 00:00:00 2001
From: Mike Rotondo <mrotondo@codeforamerica.org>
Date: Wed, 7 Aug 2024 20:20:25 -0400
Subject: [PATCH] fix xml return sample service to handle invalid keys
 gracefully (#4694)

---
 .../state_file/xml_return_sample_service.rb    | 16 ++++++++--------
 .../xml_return_sample_service_spec.rb          | 18 ++++++++++++++----
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/app/services/state_file/xml_return_sample_service.rb b/app/services/state_file/xml_return_sample_service.rb
index ced3bbb897..388f3a7e6f 100644
--- a/app/services/state_file/xml_return_sample_service.rb
+++ b/app/services/state_file/xml_return_sample_service.rb
@@ -1,8 +1,8 @@
 module StateFile
   class XmlReturnSampleService
     def initialize
-      @_samples = {}
-      @_submission_id_lookup = {
+      @samples = {}
+      @submission_id_lookup = {
         'ny_rudy_v2' => '1016422024027ate001k',
         'ny_javier' => '1016422024018atw000x',
         'ny_matthew_v2' => '1016422024026atw001u',
@@ -18,7 +18,7 @@ def initialize
 
     def samples
       load_samples
-      @_samples
+      @samples
     end
 
     def self.key(us_state, sample_name)
@@ -30,7 +30,7 @@ def self.label(sample_name)
     end
 
     def lookup_submission_id(key)
-      @_submission_id_lookup[key] || '12345202201011234570'
+      @submission_id_lookup[key] || '12345202201011234570'
     end
 
     def include?(key)
@@ -51,13 +51,13 @@ def old_sample
     TAX_YEAR = Rails.configuration.statefile_current_tax_year.to_s.freeze
 
     def load_samples
-      return if @_samples.present?
+      return if @samples.present?
 
       StateFile::StateInformationService.active_state_codes.each do |us_state|
-        @_samples[us_state] = []
+        @samples[us_state] = []
         xml_path_glob = File.join(BASE_PATH, TAX_YEAR, us_state, '*.xml')
         Dir.glob(xml_path_glob).each do |xml_path|
-          @_samples[us_state].push(File.basename(xml_path, ".xml"))
+          @samples[us_state].push(File.basename(xml_path, ".xml"))
         end
       end
     end
@@ -67,7 +67,7 @@ def path(key)
       return @old_sample if key == "abcdefg"
 
       us_state, sample_name = key.split("_", 2)
-      if @_samples[us_state].include? sample_name
+      if @samples.include?(us_state) && @samples[us_state].include?(sample_name)
         File.join(BASE_PATH, TAX_YEAR, us_state, "#{sample_name}.xml")
       end
     end
diff --git a/spec/services/state_file/xml_return_sample_service_spec.rb b/spec/services/state_file/xml_return_sample_service_spec.rb
index c4a37cc795..38e3c199c3 100644
--- a/spec/services/state_file/xml_return_sample_service_spec.rb
+++ b/spec/services/state_file/xml_return_sample_service_spec.rb
@@ -6,7 +6,8 @@
   let(:state_code) { 'az' }
   let(:sample_name) { 'superman_v2' }
   let(:key) { 'az_superman_v2' }
-  let(:bad_key) { 'az_superman_does_not_exist' }
+  let(:missing_key) { 'az_superman_does_not_exist' }
+  let(:invalid_key) { 'asdf' }
   let(:label) { 'Az superman v2' }
   let(:unique_file_contents) { 'KENT' }
   let(:old_sample_unique_file_contents) { 'TESTERSON' }
@@ -31,7 +32,7 @@
     end
 
     it 'returns the default submission id if one is not found' do
-      expect(xml_return_sample_service.lookup_submission_id(bad_key)).to eq default_submission_id
+      expect(xml_return_sample_service.lookup_submission_id(missing_key)).to eq default_submission_id
     end
   end
 
@@ -41,8 +42,13 @@
     end
 
     it 'returns false if the sample does not exist' do
-      expect(xml_return_sample_service.include?(bad_key)).to be_falsey
+      expect(xml_return_sample_service.include?(missing_key)).to be_falsey
     end
+
+    it 'returns false if the key is invalid' do
+      expect(xml_return_sample_service.include?(invalid_key)).to be_falsey
+    end
+
   end
 
   describe '#read' do
@@ -51,7 +57,11 @@
     end
 
     it 'returns nil if the sample does not exist' do
-      expect(xml_return_sample_service.read(bad_key)).to be_nil
+      expect(xml_return_sample_service.read(missing_key)).to be_nil
+    end
+
+    it 'returns nil if the key is invalid' do
+      expect(xml_return_sample_service.read(invalid_key)).to be_nil
     end
   end