Find End Product without End Product Establishment for RAMP #10363
Conversation
ghost
assigned 
|
Code Climate has analyzed commit e10fb23 and detected 0 issues on this pull request. View more on Code Climate. |
app/models/ramp_refiling_intake.rb
Outdated
| @@ -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?) | |||
There was a problem hiding this comment.
I believe this was a mistake in the original logic
There was a problem hiding this comment.
This commit 072065d suggests otherwise. It was any? before.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
The code looks fine.
The approach makes me a little nervous though, because we are changing some business rules to try and match just-in-time within the existing models, rather than applying a one-time backfill via an external rake task or script.
What would you think about refactoring the matching logic to not be in the models themselves but as a ramp:backfill_epe rake task that we could run in off hours to create the missing EPEs? We could do a few at a time to verify it works.
app/models/ramp_refiling_intake.rb
Outdated
| @@ -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?) | |||
There was a problem hiding this comment.
This commit 072065d suggests otherwise. It was any? before.
|
I would be a big fan of doing a backfill for the unconnected RampElections, that would be less hacky. Shane is nervous about backfilling data at all so this is more of a bandaid. However, I do feel that the explanation here: #9946 |
leikkisa
added
the
Ready-to-Merge
connects #9964
If the EndProductEstablishment for a RAMP intake isn't saved, find the matching end product when needed