State pension calculator amends #2192
Conversation
|
As far as I can see there is no explanation as to why this functionality has been removed in the commit notes, the PR description or the Pivotal Tracker story. |
erkde
force-pushed
the
state_pension_calculator_amends
branch
9 times, most recently
from
f889546 to
b81a2ab
Compare
|
Have just rebased this with master |
erkde
force-pushed
the
state_pension_calculator_amends
branch
from
b81a2ab to
349273b
Compare
|
Have just rebased this with master |
|
Changed this to don't merge until we have a known release date |
|
Minor: There are quite a few commits with exactly the same first line which makes it tricky to work out which one is which. I'd be inclined to include some kind of differentiation in these first lines (e.g. method name) to make things clearer. |
|
I think it would be useful to include a "before" and "after" visualisation of the flow in the PR description. |
erkde
force-pushed
the
state_pension_calculator_amends
branch
2 times, most recently
from
dfb4d72 to
dcd690f
Compare
|
In this line, the lac of capitalisation of "pension credit" seems to be inconsistent with other copy. However, it looks as if this was the case before this PR anyway. |
|
The code in |
|
I've now finished reviewing this PR. The majority of my comments were about better ways to structure the commits . If time is pressing, I don't think there is anything preventing this PR from being merged. However, if there is time, it would be much better to address as many of the comments as possible before merging. |
erkde
force-pushed
the
state_pension_calculator_amends
branch
from
dcd690f to
71e9d1a
Compare
|
With the caveats mentioned in my most recent comment, I'm going to remove the "Don't merge" label and add the "LGTM" label. |
erkde
force-pushed
the
state_pension_calculator_amends
branch
from
71e9d1a to
779d7eb
Compare
As both the bus pass and state pension age calculations need the user’s date of birth, this change moves the dob_age node to Q2 and shares the response for both calculations. The duplicate bus_pass_age question is no longer needed, and the test data updated with the new ordering with dob_age now in position Q2 for both calculations.
Rename bus_pass_age_result outcome to bus_pass_result to be inline with logic doc. Inline the partial content, and remove the link to calculate state pension age.
The state_pension_age_calculator can be used for calculating the stage_pension_age, and the bus pass age. As the bus pass age defaults to :female for gender, this gender param to the new method can be optional. With gender an optional param, changing gender from a read to a read/write accessor allows its’ value to be set in subsequent questions.
The gender is now settable via an attr_accessor so no longer needs to be passed as an argument to the pension_credit_date method.
StatePensionDateQuery was used both within the calculator and the smart answer DSL, creating a dependency in two places. As the calculator already has an existing call into StatePensionDateQuery, this change allows smart answer to do away with it’s dependency and simply call the methods on the calculator.
Rather than hardcode a period of 30 days, or 4 months, to determine how close the user is to the state_pension_date, add an optional argument that defaults to 0 days, so that the smart answer or outcome template can query how close a user is to the state pension date.
And add test coverage.
Updates the query method from ‘find’ to ‘state_pension_date’ to better describe the type of result expected back. Adds tests around the complete set of stage pension age, pension credit age data, and bus pass qualification age data at unit level.
Removes unused methods in the state pension age calculator which were a carry over from calculating the state pension amount, and introduces new methods for showing the state pension age and bus pass age outcomes. With the StatePensionDateQuery class now having test coverage for the pension age data, it removes the tests duplicating testing this data and adds tests around the functionality of the calculator, including edge conditions that flip boolean results of a method call.
Rename outcomes to be inline with updated logic doc, and replace three state pension age outcomes with two, one for before reaching state pension and another for when you reached. Tidy up integration tests to cover all paths through the smart answer flow, and remove those tests that test data - such as when you reach a state pension, which are now covered at unit level
Add capitalisation and the word qualifying to ‘pension credit age’, inline with other occurrences in the smart answer
With uncertainty around the date of the new DWP state pension calculator going live, change this URL to point to the existing state pension age and handle with a redirect once launched
Replace comma with ‘and’ inline with other occurrences in the smart answer
erkde
force-pushed
the
state_pension_calculator_amends
branch
from
779d7eb to
d40a4e4
Compare
|
Have just rebased with master |
|
The 5 new commits look good to me. |
PT story: https://www.pivotaltracker.com/story/show/108748030
DWP are launching their own State Pension calculator.
This PR removes the 'estimate State Pension amount' option and associated logic. The start page is amended to reflect the reduced functionality of the updated flow and many of the questions and outcomes are removed.
Fact check
https://pt-108748030.herokuapp.com/state-pension-age
Expected changes
Before
After
Before
After