From 6b92f100ccc00e0eb65c8f190c2386c03e6f6d98 Mon Sep 17 00:00:00 2001 From: Enrilo Ugalde Date: Thu, 24 Aug 2023 09:33:47 -0700 Subject: [PATCH 01/15] APPEALS-29120 - init commit, fixed linting and code climate issues rspec passes --- app/models/concerns/sync_lock.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/app/models/concerns/sync_lock.rb b/app/models/concerns/sync_lock.rb index ea1bbe02ae0..7127359927d 100644 --- a/app/models/concerns/sync_lock.rb +++ b/app/models/concerns/sync_lock.rb @@ -12,10 +12,12 @@ def hlr_sync_lock lock_key = "hlr_sync_lock:#{end_product_establishment.id}" begin - # create the sync lock with a key, value pair only IF it doesn't already exist and give it an expiration time upon creation - sync_lock_acquired = redis.set(lock_key, "lock is set", :nx => true, :ex => LOCK_TIMEOUT.to_i) + # create the sync lock with a key, value pair only IF it doesn't already exist + # and give it an expiration time upon creation + sync_lock_acquired = redis.set(lock_key, "lock is set", nx: true, ex: LOCK_TIMEOUT.to_i) + + fail Caseflow::Error::SyncLockFailed, message: Time.zone.now.to_s unless sync_lock_acquired - fail Caseflow::Error::SyncLockFailed, message: "#{Time.zone.now}" unless sync_lock_acquired # set expire as another failsafe redis.expire(lock_key, LOCK_TIMEOUT.to_i) yield From be9d3edb04cddeeca2ec75a24ae34cec8e3ab3cb Mon Sep 17 00:00:00 2001 From: Enrilo Ugalde Date: Thu, 24 Aug 2023 11:41:50 -0700 Subject: [PATCH 02/15] APPEALS-29120 - added Rails.logger.info for lock_key creation. confirmed in log works, and updated 1.9ruby hash syntax --- app/models/concerns/sync_lock.rb | 1 + spec/models/request_issue_spec.rb | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/concerns/sync_lock.rb b/app/models/concerns/sync_lock.rb index 7127359927d..b53f143b55b 100644 --- a/app/models/concerns/sync_lock.rb +++ b/app/models/concerns/sync_lock.rb @@ -10,6 +10,7 @@ def hlr_sync_lock if decision_review.is_a?(HigherLevelReview) && block_given? redis = Redis.new(url: Rails.application.secrets.redis_url_cache) lock_key = "hlr_sync_lock:#{end_product_establishment.id}" + Rails.logger.info(lock_key + " has been created") begin # create the sync lock with a key, value pair only IF it doesn't already exist diff --git a/spec/models/request_issue_spec.rb b/spec/models/request_issue_spec.rb index 464004cc5de..3688b41298e 100644 --- a/spec/models/request_issue_spec.rb +++ b/spec/models/request_issue_spec.rb @@ -2191,7 +2191,7 @@ it "prevents a request issue from acquiring the SyncLock when there is already a lock using the EPE's ID" do redis = Redis.new(url: Rails.application.secrets.redis_url_cache) lock_key = "hlr_sync_lock:#{epe.id}" - redis.set(lock_key, "lock is set", :nx => true, :ex => 5.seconds) + redis.set(lock_key, "lock is set", nx: true, ex: 5.seconds) expect { request_issue1.sync_decision_issues! }.to raise_error(sync_lock_err) redis.del(lock_key) end From ba392990a5075a699f73c583ac306f229ff50638 Mon Sep 17 00:00:00 2001 From: Enrilo Ugalde Date: Thu, 24 Aug 2023 11:47:14 -0700 Subject: [PATCH 03/15] APPEALS-29120 - added Rails.logger for lock_key being deleted --- app/models/concerns/sync_lock.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/concerns/sync_lock.rb b/app/models/concerns/sync_lock.rb index b53f143b55b..4647349439d 100644 --- a/app/models/concerns/sync_lock.rb +++ b/app/models/concerns/sync_lock.rb @@ -24,6 +24,7 @@ def hlr_sync_lock yield ensure redis.del(lock_key) + Rails.logger.info(lock_key + " has been deleted") end elsif block_given? yield From 119b1e4a207173d1cfe2cfa8c940bc2366105b9d Mon Sep 17 00:00:00 2001 From: Enrilo Ugalde Date: Thu, 24 Aug 2023 11:48:14 -0700 Subject: [PATCH 04/15] APPEALS-29120 - updated verbage from deleted to released --- app/models/concerns/sync_lock.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/concerns/sync_lock.rb b/app/models/concerns/sync_lock.rb index 4647349439d..891c4440eff 100644 --- a/app/models/concerns/sync_lock.rb +++ b/app/models/concerns/sync_lock.rb @@ -24,7 +24,7 @@ def hlr_sync_lock yield ensure redis.del(lock_key) - Rails.logger.info(lock_key + " has been deleted") + Rails.logger.info(lock_key + " has been released") end elsif block_given? yield From fce75361673d3ab98b4d0a4ad904b095e3855c2f Mon Sep 17 00:00:00 2001 From: Enrilo Ugalde Date: Thu, 24 Aug 2023 13:44:43 -0700 Subject: [PATCH 05/15] APPEALS-29120 - refactored Rails.logger for release, added Error logging,and added to rspec test --- app/jobs/decision_issue_sync_job.rb | 1 + app/models/concerns/sync_lock.rb | 9 +++++---- spec/jobs/decision_issue_sync_job_spec.rb | 4 +++- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/app/jobs/decision_issue_sync_job.rb b/app/jobs/decision_issue_sync_job.rb index 838ce178638..6501df05b89 100644 --- a/app/jobs/decision_issue_sync_job.rb +++ b/app/jobs/decision_issue_sync_job.rb @@ -15,6 +15,7 @@ def perform(request_issue_or_effectuation) request_issue_or_effectuation.update_error!(error.inspect) request_issue_or_effectuation.update!(decision_sync_attempted_at: Time.zone.now - 11.hours - 55.minutes) capture_exception(error: error) + Rails.logger.error error.inspect rescue Errno::ETIMEDOUT => error # no Raven report. We'll try again later. Rails.logger.error error diff --git a/app/models/concerns/sync_lock.rb b/app/models/concerns/sync_lock.rb index 891c4440eff..690c4d0c6b3 100644 --- a/app/models/concerns/sync_lock.rb +++ b/app/models/concerns/sync_lock.rb @@ -17,14 +17,15 @@ def hlr_sync_lock # and give it an expiration time upon creation sync_lock_acquired = redis.set(lock_key, "lock is set", nx: true, ex: LOCK_TIMEOUT.to_i) - fail Caseflow::Error::SyncLockFailed, message: Time.zone.now.to_s unless sync_lock_acquired + fail Caseflow::Error::SyncLockFailed, message: "#{Time.zone.now}" unless sync_lock_acquired - # set expire as another failsafe - redis.expire(lock_key, LOCK_TIMEOUT.to_i) yield ensure redis.del(lock_key) - Rails.logger.info(lock_key + " has been released") + # if lock_key is false then log has been released + unless redis.get(lock_key) + Rails.logger.info(lock_key + " has been released") + end end elsif block_given? yield diff --git a/spec/jobs/decision_issue_sync_job_spec.rb b/spec/jobs/decision_issue_sync_job_spec.rb index 8be8909dcbc..97cd85221d4 100644 --- a/spec/jobs/decision_issue_sync_job_spec.rb +++ b/spec/jobs/decision_issue_sync_job_spec.rb @@ -5,7 +5,7 @@ let(:request_issue) { create(:request_issue, end_product_establishment: epe) } let(:no_ratings_err) { Rating::NilRatingProfileListError.new("none!") } let(:bgs_transport_err) { BGS::ShareError.new("network!") } - let(:sync_lock_err) {Caseflow::Error::SyncLockFailed.new("#{Time.zone.now}")} + let(:sync_lock_err) { Caseflow::Error::SyncLockFailed.new("#{Time.zone.now}") } subject { described_class.perform_now(request_issue) } @@ -47,11 +47,13 @@ it "logs SyncLock errors" do capture_raven_log allow(request_issue).to receive(:sync_decision_issues!).and_raise(sync_lock_err) + allow(Rails.logger).to receive(:error) subject expect(request_issue.decision_sync_error).to eq("#") expect(request_issue.decision_sync_attempted_at).to be_within(5.minutes).of 12.hours.ago expect(@raven_called).to eq(false) + expect(Rails.logger).to have_received(:error).with(sync_lock_err) end it "ignores error on success" do From 926d7a28a649934fad6cd64cabea368fc5264983 Mon Sep 17 00:00:00 2001 From: Enrilo Ugalde Date: Thu, 24 Aug 2023 13:56:24 -0700 Subject: [PATCH 06/15] APPEALS-29120- fixed CodeClimate/Linting issues --- app/models/concerns/sync_lock.rb | 2 +- spec/jobs/decision_issue_sync_job_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/concerns/sync_lock.rb b/app/models/concerns/sync_lock.rb index 690c4d0c6b3..cff20ffe1a5 100644 --- a/app/models/concerns/sync_lock.rb +++ b/app/models/concerns/sync_lock.rb @@ -17,7 +17,7 @@ def hlr_sync_lock # and give it an expiration time upon creation sync_lock_acquired = redis.set(lock_key, "lock is set", nx: true, ex: LOCK_TIMEOUT.to_i) - fail Caseflow::Error::SyncLockFailed, message: "#{Time.zone.now}" unless sync_lock_acquired + fail Caseflow::Error::SyncLockFailed, message: Time.zone.now.to_s unless sync_lock_acquired yield ensure diff --git a/spec/jobs/decision_issue_sync_job_spec.rb b/spec/jobs/decision_issue_sync_job_spec.rb index 97cd85221d4..bcd74bf9794 100644 --- a/spec/jobs/decision_issue_sync_job_spec.rb +++ b/spec/jobs/decision_issue_sync_job_spec.rb @@ -5,7 +5,7 @@ let(:request_issue) { create(:request_issue, end_product_establishment: epe) } let(:no_ratings_err) { Rating::NilRatingProfileListError.new("none!") } let(:bgs_transport_err) { BGS::ShareError.new("network!") } - let(:sync_lock_err) { Caseflow::Error::SyncLockFailed.new("#{Time.zone.now}") } + let(:sync_lock_err) { Caseflow::Error::SyncLockFailed.new(Time.zone.now.to_s) } subject { described_class.perform_now(request_issue) } From b3984935b2ec40b61e2a068f7aadf1e30ea47018 Mon Sep 17 00:00:00 2001 From: Enrilo Ugalde Date: Thu, 24 Aug 2023 14:09:22 -0700 Subject: [PATCH 07/15] APPEALS-29120 - fixed more CodeClimate/Linting issues --- ...s_claims_decision_issues_and_request_decision_issues.rb | 5 +++-- spec/models/request_issue_spec.rb | 7 +++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/scripts/remove_rs_claims_decision_issues_and_request_decision_issues.rb b/scripts/remove_rs_claims_decision_issues_and_request_decision_issues.rb index 3d26487b2e6..0f8f11f66b8 100644 --- a/scripts/remove_rs_claims_decision_issues_and_request_decision_issues.rb +++ b/scripts/remove_rs_claims_decision_issues_and_request_decision_issues.rb @@ -1,5 +1,6 @@ - -end_product_establishment_ids = [4142,4143,4144,4145] +# frozen_string_literal: true +end_product_establishment_ids = [4142, 4143, 4144, 4145] + # :reek:FeatureEnvy def remove_rs_claims_decision_issues_and_request_decision_issues(end_product_establishment_ids) request_issues = RequestIssue.where(end_product_establishment_id: end_product_establishment_ids) request_issues.each do |request_issue| diff --git a/spec/models/request_issue_spec.rb b/spec/models/request_issue_spec.rb index 3688b41298e..0b7b70b968c 100644 --- a/spec/models/request_issue_spec.rb +++ b/spec/models/request_issue_spec.rb @@ -2213,11 +2213,11 @@ sc = SupplementalClaim.first expect(sc.request_issues.count).to eq(2) supplemental_claim_request_issue1 = sc.request_issues.first - supplemental_claim_request_issue2= sc.request_issues.last + supplemental_claim_request_issue2 = sc.request_issues.last # both request issues link to the same SupplementalClaim - expect(sc.id).to eq (request_issue1.end_product_establishment.source.remand_supplemental_claims.first.id) - expect(sc.id).to eq (request_issue2.end_product_establishment.source.remand_supplemental_claims.first.id) + expect(sc.id).to eq(request_issue1.end_product_establishment.source.remand_supplemental_claims.first.id) + expect(sc.id).to eq(request_issue2.end_product_establishment.source.remand_supplemental_claims.first.id) # DecisionIssue ID should match contested_decision_issue_id expect(DecisionIssue.count).to eq(2) @@ -2227,7 +2227,6 @@ expect(decision_issue1.id).to eq(supplemental_claim_request_issue1.contested_decision_issue_id) expect(decision_issue2.id).to eq(supplemental_claim_request_issue2.contested_decision_issue_id) end - end end end From 785d14c57916793bf5bad5dd3b15633ae64db9c3 Mon Sep 17 00:00:00 2001 From: Enrilo Ugalde Date: Thu, 24 Aug 2023 14:11:09 -0700 Subject: [PATCH 08/15] APPEALS-29120 - disabled FeatureEnvy smell --- ...ove_rs_claims_decision_issues_and_request_decision_issues.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/remove_rs_claims_decision_issues_and_request_decision_issues.rb b/scripts/remove_rs_claims_decision_issues_and_request_decision_issues.rb index 0f8f11f66b8..1b273ad8434 100644 --- a/scripts/remove_rs_claims_decision_issues_and_request_decision_issues.rb +++ b/scripts/remove_rs_claims_decision_issues_and_request_decision_issues.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true end_product_establishment_ids = [4142, 4143, 4144, 4145] - # :reek:FeatureEnvy +# :reek:FeatureEnvy def remove_rs_claims_decision_issues_and_request_decision_issues(end_product_establishment_ids) request_issues = RequestIssue.where(end_product_establishment_id: end_product_establishment_ids) request_issues.each do |request_issue| From a2468efbab57cb75242d3f1e4148de5b1b439b65 Mon Sep 17 00:00:00 2001 From: Enrilo Ugalde Date: Thu, 24 Aug 2023 14:23:03 -0700 Subject: [PATCH 09/15] APPEALS-29120 - fixed more linting issues --- ...ve_rs_claims_decision_issues_and_request_decision_issues.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/remove_rs_claims_decision_issues_and_request_decision_issues.rb b/scripts/remove_rs_claims_decision_issues_and_request_decision_issues.rb index 1b273ad8434..18241234503 100644 --- a/scripts/remove_rs_claims_decision_issues_and_request_decision_issues.rb +++ b/scripts/remove_rs_claims_decision_issues_and_request_decision_issues.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true end_product_establishment_ids = [4142, 4143, 4144, 4145] + # :reek:FeatureEnvy def remove_rs_claims_decision_issues_and_request_decision_issues(end_product_establishment_ids) request_issues = RequestIssue.where(end_product_establishment_id: end_product_establishment_ids) @@ -17,7 +18,7 @@ def remove_rs_claims_decision_issues_and_request_decision_issues(end_product_est decision_review_remanded: di.decision_review, benefit_type: di.benefit_type ) - supplemental_claim.destroy if supplemental_claim + supplemental_claim&.destroy if supplemental_claim di.destroy end request_issue.update(closed_at: nil, closed_status: nil) From 75140ebfb68a9bd729ccf7f0f4b5a26f7c620653 Mon Sep 17 00:00:00 2001 From: Enrilo Ugalde Date: Thu, 24 Aug 2023 14:39:01 -0700 Subject: [PATCH 10/15] added line break after frozen_string_literal --- ...move_rs_claims_decision_issues_and_request_decision_issues.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/remove_rs_claims_decision_issues_and_request_decision_issues.rb b/scripts/remove_rs_claims_decision_issues_and_request_decision_issues.rb index 18241234503..f33e25ed079 100644 --- a/scripts/remove_rs_claims_decision_issues_and_request_decision_issues.rb +++ b/scripts/remove_rs_claims_decision_issues_and_request_decision_issues.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + end_product_establishment_ids = [4142, 4143, 4144, 4145] # :reek:FeatureEnvy From 265645f57a1c9dd8eaf253582f3e1abded792c20 Mon Sep 17 00:00:00 2001 From: Enrilo Ugalde Date: Thu, 24 Aug 2023 15:30:35 -0700 Subject: [PATCH 11/15] APPEALS-29120 - add code coverage for new logger for hlr_sync_lock creation and release in request_issue_spec.rb --- spec/models/request_issue_spec.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/spec/models/request_issue_spec.rb b/spec/models/request_issue_spec.rb index 0b7b70b968c..e5c6edbd23a 100644 --- a/spec/models/request_issue_spec.rb +++ b/spec/models/request_issue_spec.rb @@ -2197,9 +2197,12 @@ end it "allows a request issue to sync if there is no existing lock using the EPE's ID" do + epe_id = request_issue2.end_product_establishment.id.to_s + allow(Rails.logger).to receive(:info) expect(request_issue2.sync_decision_issues!).to eq(true) + expect(Rails.logger).to have_received(:info).with("hlr_sync_lock:" + epe_id + " has been created") expect(request_issue2.processed?).to eq(true) - + expect(Rails.logger).to have_received(:info).with("hlr_sync_lock:" + epe_id + " has been released") expect(SupplementalClaim.count).to eq(1) end From 82216e53bd7f59d799a91e139bc26affb10803ab Mon Sep 17 00:00:00 2001 From: TuckerRose Date: Fri, 25 Aug 2023 15:42:22 -0400 Subject: [PATCH 12/15] removed end_product_establishment_ids variable since its now being used --- ...e_rs_claims_decision_issues_and_request_decision_issues.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/scripts/remove_rs_claims_decision_issues_and_request_decision_issues.rb b/scripts/remove_rs_claims_decision_issues_and_request_decision_issues.rb index f33e25ed079..92e7c4fced7 100644 --- a/scripts/remove_rs_claims_decision_issues_and_request_decision_issues.rb +++ b/scripts/remove_rs_claims_decision_issues_and_request_decision_issues.rb @@ -1,7 +1,3 @@ -# frozen_string_literal: true - -end_product_establishment_ids = [4142, 4143, 4144, 4145] - # :reek:FeatureEnvy def remove_rs_claims_decision_issues_and_request_decision_issues(end_product_establishment_ids) request_issues = RequestIssue.where(end_product_establishment_id: end_product_establishment_ids) From 6d49077ede204bc6b7d78b58a07a258d7451c484 Mon Sep 17 00:00:00 2001 From: TuckerRose Date: Fri, 25 Aug 2023 16:19:54 -0400 Subject: [PATCH 13/15] removed commas --- scripts/enable_features_dev.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/enable_features_dev.rb b/scripts/enable_features_dev.rb index f15d0ec5e18..ef1b30eddd4 100644 --- a/scripts/enable_features_dev.rb +++ b/scripts/enable_features_dev.rb @@ -59,9 +59,9 @@ def call cavc_dashboard_workflow poa_auto_refresh interface_version_2 - cc_vacatur_visibility, - acd_disable_legacy_distributions, - acd_disable_nonpriority_distributions, + cc_vacatur_visibility + acd_disable_legacy_distributions + acd_disable_nonpriority_distributions acd_disable_legacy_lock_ready_appeals ] From 305faeb4bea3f6c6b45902068c5cd4fb118bd28a Mon Sep 17 00:00:00 2001 From: Enrilo Ugalde Date: Fri, 25 Aug 2023 14:36:45 -0700 Subject: [PATCH 14/15] APPEALS-29120 - added magic comment --- ...ove_rs_claims_decision_issues_and_request_decision_issues.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/remove_rs_claims_decision_issues_and_request_decision_issues.rb b/scripts/remove_rs_claims_decision_issues_and_request_decision_issues.rb index 92e7c4fced7..763808af64c 100644 --- a/scripts/remove_rs_claims_decision_issues_and_request_decision_issues.rb +++ b/scripts/remove_rs_claims_decision_issues_and_request_decision_issues.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # :reek:FeatureEnvy def remove_rs_claims_decision_issues_and_request_decision_issues(end_product_establishment_ids) request_issues = RequestIssue.where(end_product_establishment_id: end_product_establishment_ids) From 3736ca68a71dcaffeb7e81d43bd43b84eca162aa Mon Sep 17 00:00:00 2001 From: Enrilo Ugalde Date: Fri, 25 Aug 2023 14:48:56 -0700 Subject: [PATCH 15/15] commit to init GHA --- app/models/concerns/sync_lock.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/concerns/sync_lock.rb b/app/models/concerns/sync_lock.rb index cff20ffe1a5..2440366a6ad 100644 --- a/app/models/concerns/sync_lock.rb +++ b/app/models/concerns/sync_lock.rb @@ -14,7 +14,7 @@ def hlr_sync_lock begin # create the sync lock with a key, value pair only IF it doesn't already exist - # and give it an expiration time upon creation + # and give it an expiration time upon creation. sync_lock_acquired = redis.set(lock_key, "lock is set", nx: true, ex: LOCK_TIMEOUT.to_i) fail Caseflow::Error::SyncLockFailed, message: Time.zone.now.to_s unless sync_lock_acquired