CAPT 1995/remove ecp #3415
Open
CAPT 1995/remove ecp #3415
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Part of refactoring to remove ECP. These concerns are only used in these classes, inlining them to make tracking the difference between ECP and LUP easier.
This was the only method EligibilityCheckable was being pulled in for. While we're duplicating the definition of a trainee teacher, inlining the method here will make splitting up the EligibilityCheckable policy easier.
FINAL_LUP_POLICY_YEAR is a dupe of Lup::POLICY_END_YEAR. We should have updated Lup::POLICY_END_YEAR to 2025 as part of capt-1832
This method is only used in the specs. Inline the method to where it's used and remove the method and it's tests. One less thing to keep track off!
Splits this method by policy so it's easier to break out LUP into it's own journey.
There was a lot of duplicated business rules in this method. We were explicity checking for `nqt_in_academic_year_after_itt`, and if it was absent we were falling back to only the lup subjects. The ECP policy is ineligible for non `nqt_in_academic_year_after_itt` claims so it will be filtered out when we check for `potentially_still_eligible_policies`. We returned an empty array if the `current_academic_year` was outside the lup policy dates, however the LUP#subject_symbols method already returns an empty array of available subjects for years outside the policy. There's some weirdness in the `AdditionalPaymentsForTeaching.selectable_subject_symbols` method. If we're dealing with a trainee teacher, we return a hard coded list of subjects, however trainee teachers won't be eligible as the `EligibilityCheckable#common_ineligible_attributes?` checks if the claimant is a trainee teacher, and is thus ineligible. It seems weird to return a list of subjects if the claimant is ineligible. This was the existing behaviour so I've kept it in place but this likely needs revisiting.
As part of the refactor to remove ECP we want to move the policy specific functionality into the respective polices. This commit moves the `selectable_itt_years_for_claim_year` out of the lib class and into each policy, with the journey delegating this method to any potentially_still_eligible policies.
Move fixed_lup_subject_symbols of off JourneySubjectEligibilityChecker and onto LevellingUpPremiumPayments policy as this method is policy specific.
rjlynch
force-pushed
the
CAPT-1995/remove-ecp
branch
from
be09470
to
e24a24a
Compare
This class is no longer needed
rjlynch
force-pushed
the
CAPT-1995/remove-ecp
branch
from
e24a24a
to
429884b
Compare
Just making the code a little easier to follow
This branch can't get hit as this method is only called when the claimant is a trainee teacher, ie `nqt_in_academic_year_after_itt` is `false`.
rjlynch
force-pushed
the
CAPT-1995/remove-ecp
branch
from
f38db53
to
96ae744
Compare
ECP `specific_ineligible_attributes?` already checks if `trainee_teacher?` is set. For LUP trainee teachers can still be eligible later, so we want to make sure we don't return ineligible in that case. This previously hasn't surfaced as an issue due to how the code was called (a short circut specific to trainee teachers in the page sequence) but we want to move some of the code around which exposes this bug.
Removes the ineligiblity branches from the special case trainee teacher journey. We want all eligibility checking to happen in one place (the policy eligibility checker). This commit updates the mini trainee journey to not check eligibility. This also surfaced a bug in the policy eligiblity checker, a trainee teacher with an ineligible itt subject is not yet ineligible, they may still be eligible later, we don't know until they've entered their degree subject (at least according to the logic in `spec/features/trainee_teacher_subjourney_for_lup_schools_spec.rb:59`)
rjlynch
force-pushed
the
CAPT-1995/remove-ecp
branch
2 times, most recently
from
2277de3
to
ca6b041
Compare
Previously the page sequence contained specific logic for handling an additional payments sub journey (traniee teacher), and rather than modifying the slug sequence just overwrote what is returned. Additionally there was a condition in the slug sequence that caused the whole thing to be swapped out if we were dealing with a trainee teacher. This commit removes the additional payments specific handling from the page sequence, leaving it journey agnostic, and updates the additional payments slug sequence to move the trainee teacher sub journey branching to the top level.
rjlynch
force-pushed
the
CAPT-1995/remove-ecp
branch
from
ca6b041
to
47236ce
Compare
LUP will extend past ECP, so we can't use the combined end date when deciding what subjects to show for nqts who haven't entered an itt, otherwise we render an empty list on the itt subject screen.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR introduces some refactors to separate the LUP and ECP logic as part of the move to splitting LUP into it's own journey and deprecating ECP.
This will be a lot easier to review looking at the individual commits!
At a high level this PR
lib/journey_subject_eligibility_checker.rbWe'll introduce the LUP only journey in a separate PR.