Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Find End Product without End Product Establishment for RAMP #10363

Merged
merged 7 commits into from
Apr 9, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions app/models/end_product_establishment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,12 @@ def rating?
end

# Find an end product that has the traits of the end product that should be created.
def preexisting_end_product
@preexisting_end_product ||= veteran.end_products.find { |ep| ep.active? && end_product_to_establish.matches?(ep) }
def active_preexisting_end_product
preexisting_end_products.find(&:active?)
end

def preexisting_end_products
@preexisting_end_products ||= veteran.end_products.select { |ep| end_product_to_establish.matches?(ep) }
end

def cancel_unused_end_product!
Expand All @@ -173,10 +177,10 @@ def cancel_unused_end_product!
end

def sync!
fail EstablishedEndProductNotFound, id unless result
leikkisa marked this conversation as resolved.
Show resolved Hide resolved
# There is no need to sync end_product_status if the status
# is already inactive since an EP can never leave that state
return true unless status_active?
fail EstablishedEndProductNotFound, id unless result

# load contentions now, in case "source" needs them.
# this VBMS call is slow and will cause the transaction below
Expand Down
2 changes: 1 addition & 1 deletion app/models/ramp_election.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def recreate_issues_from_contentions!(epe = nil)

# Load contentions outside of the Postgres transaction so we don't keep a connection
# open needlessly for the entirety of what could be a slow VBMS request.
contentions = epe ? epe.contentions : end_product_establishment.contentions
contentions = epe ? epe.contentions : end_product_establishment.contentions || matching_end_product&.contentions
transaction do
issues.destroy_all

Expand Down
2 changes: 1 addition & 1 deletion app/models/ramp_refiling_intake.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def validate_detail_on_start
@error_data = veteran_invalid_fields
elsif ramp_elections.empty?
self.error_code = :no_complete_ramp_election
elsif ramp_elections.all?(&:end_product_active?)
elsif ramp_elections.any?(&:end_product_active?)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this was a mistake in the original logic

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit 072065d suggests otherwise. It was any? before.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think this is a mistake.

Maybe what we need to check here is if there is at least one cleared RampElection. What we're seeing is that there can be active ones and canceled ones, and it seems to me that if there is an active RampElection (of which there can only be one at a time for each of the two modifiers) and a canceled RampElection, and no cleared RAMP elections, then it's not ready for a RampRefiling.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That said, this change isn't necessary for this PR, I just thought it was wrong logic, so we could also wait on this change for Shane.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it's not necessary for this PR, let's revert that line for now and open a ticket to remind ourselves to ask Shane.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New ticket: #10377

self.error_code = :ramp_election_is_active
elsif ramp_elections.all? { |election| election.issues.empty? }
self.error_code = :ramp_election_no_issues
Expand Down
16 changes: 15 additions & 1 deletion app/models/ramp_review.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def on_decision_issues_sync_processed
#
# Returns a symbol designating whether the end product was created or connected
def create_or_connect_end_product!
return connect_end_product! if end_product_establishment.preexisting_end_product
return connect_end_product! if end_product_establishment.active_preexisting_end_product

establish_end_product!(commit: true) && :created
end
Expand All @@ -83,6 +83,11 @@ def end_product_base_modifier

def end_product_active?
end_product_establishment.status_active?(sync: true)
rescue EndProductEstablishment::EstablishedEndProductNotFound
return true if end_product_establishment.active_preexisting_end_product
return false if end_product_establishment.preexisting_end_products

raise
end

def establish_end_product!(commit:)
Expand Down Expand Up @@ -127,6 +132,15 @@ def new_end_product_establishment
)
end

# Find the matching end product that was active in the right timeframe if an EndProductEstablishment is not saved
def matching_end_product
return if end_product_establishment.preexisting_end_products.empty?

end_product_establishment.preexisting_end_products.detect do |ep|
ep.last_action_date.nil? || ep.last_action_date > established_at
end
end

def veteran
@veteran ||= Veteran.find_or_create_by_file_number(veteran_file_number)
end
Expand Down
54 changes: 35 additions & 19 deletions spec/models/ramp_refiling_intake_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -187,11 +187,19 @@
Generators::EndProduct.build(
veteran_file_number: veteran_file_number,
bgs_attrs: {
status_type_code: end_product_status
claim_type_code: claim_type_code,
end_product_type_code: modifier,
claim_receive_date: claim_date,
status_type_code: end_product_status,
last_action_date: last_action_date
}
)
end

let(:claim_date) { nil }
let(:modifier) { nil }
let(:claim_type_code) { nil }
let(:last_action_date) { nil }
let(:end_product_status) { "CLR" }

context "there is not a completed ramp election for veteran" do
Expand All @@ -212,7 +220,8 @@
create(:ramp_election,
veteran_file_number: veteran_file_number,
notice_date: 3.days.ago,
established_at: Time.zone.now)
established_at: Time.zone.now,
option_selected: "higher_level_review")
end

context "the EP associated with original RampElection is still pending" do
Expand All @@ -230,6 +239,19 @@
expect(subject).to eq(false)
expect(intake.error_code).to eq("ramp_election_is_active")
end

context "there is no End Product Establishment, but there is an active matching End Product" do
let!(:end_product_establishment) { nil }
let(:claim_date) { ramp_election.receipt_date.mdY }
let(:modifier) { "682" }
let(:claim_type_code) { "682HLRRRAMP" }
let(:end_product_status) { "PEND" }

it "adds ramp_election_is_active and returns false" do
expect(subject).to eq(false)
expect(intake.error_code).to eq("ramp_election_is_active")
end
end
end

context "the EP associated with original RampElection is closed" do
Expand Down Expand Up @@ -265,27 +287,21 @@
)
end

let!(:pending_ramp_election) do
create(:ramp_election,
veteran_file_number: veteran_file_number,
notice_date: 4.days.ago,
established_at: Time.zone.now)
end

let!(:pending_end_product_establishment) do
create(
:end_product_establishment,
veteran_file_number: veteran_file_number,
source: pending_ramp_election,
established_at: Time.zone.now,
synced_status: "PEND"
)
end

before { ramp_election.recreate_issues_from_contentions! }

it { is_expected.to eq(true) }

context "there is no End Product Establishment, and the matching end products are closed" do
let!(:end_product_establishment) { nil }
let(:claim_date) { ramp_election.receipt_date.mdY }
let(:modifier) { "682" }
let(:claim_type_code) { "682HLRRRAMP" }
let(:end_product_status) { "CLR" }
let(:last_action_date) { (ramp_election.established_at + 1.day).to_date.mdY }

it { is_expected.to eq(true) }
end

context "a saved RampRefiling already exists for the veteran" do
let!(:preexisting_ramp_refiling) { RampRefiling.create!(veteran_file_number: veteran_file_number) }

Expand Down
68 changes: 68 additions & 0 deletions spec/models/ramp_review_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
# frozen_string_literal: true

describe RampReview do
let(:ramp_election) { create(:ramp_election, option_selected: "higher_level_review", established_at: Time.zone.now) }
let(:claim_date) { ramp_election.receipt_date.to_date.mdY }
let(:reference_id) { nil }
let(:end_product_status) { nil }
let(:last_action_date) { nil }

let(:modifier) { "682" }
let(:claim_type_code) { "682HLRRRAMP" }

let!(:end_product) do
Generators::EndProduct.build(
veteran_file_number: ramp_election.veteran_file_number,
bgs_attrs: {
claim_type_code: claim_type_code,
end_product_type_code: modifier,
claim_receive_date: claim_date,
status_type_code: end_product_status,
last_action_date: last_action_date
}
)
end

context "#end_product_active?" do
subject { ramp_election.end_product_active? }

context "when the end_product_establishment can sync" do
let(:reference_id) { end_product.claim_id }
let!(:end_product_establishment) do
create(
:end_product_establishment,
source: ramp_election,
reference_id: reference_id,
veteran_file_number: ramp_election.veteran_file_number,
synced_status: end_product_status
)
end

context "when the end product is inactive" do
let(:end_product_status) { "CLR" }

it { is_expected.to eq(false) }
end

context "when the end product is active" do
let(:end_product_status) { "PEND" }

it { is_expected.to eq(true) }
end
end

context "when there is no EPE and all preexisting EPs are inactive" do
context "all preexisting end products are inactive" do
let(:end_product_status) { "CLR" }

it { is_expected.to eq(false) }
end

context "one preexisting end product is active" do
let(:end_product_status) { "PEND" }

it { is_expected.to eq(true) }
end
end
end
end