-
Notifications
You must be signed in to change notification settings - Fork 120
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
National Minimum Wage 1 April 2016 #2423
Conversation
\* This figure has been rounded to the nearest penny. | ||
|
||
^Your actual pay should increase to at least £7.20 per hour from 1 April 2016.^ | ||
\* This figure has been rounded to the nearest penny. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very minor: The indentation of this line looks wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated. Thanks
In general this looks like a big improvement on #2419 - I found it much easier to follow in terms of the structure of the commits. As I mention in a couple of the (large) commits which change the regression test artefacts, I can't be completely confident that all the changes are intentional. However, as long as the Content Team and/or the relevant Department are happy that the functionality is as required, I think this should be OK to merge. |
I expect you're already aware, but GitHub is indicating that there are conflicts between this branch and master which will need resolving before it can be merged. |
Other than my comments above, I'm happy for this to be merged. |
8ff6e09
to
c8fb843
Compare
Thanks for your review @floehopper, and for your comments on #2419. It might be interesting to write in our docs the approach that smart-answers is following when facing commits granularity (small vs squashed commits) cc/@chrisroos? It might help other devs to follow the same approach. I have tackled all your comments, so I have rebased with master and force pushed in preparation for merging. |
I had already added this Trello card and I've just added this other one to write-up some documentation about the structure of commits. |
ca5f790
to
95c75f6
Compare
After 1 April 2016 the copy need to be updated. It also updates `national living wage` into `National Living Wage` as this is the agreed convention across the calculator.
After 1 April 2016 there is no need to ask if you get the national living wage from 1 April.
Question is not accesible from the flow because the option `current_payment_aril_2016` has been removed.
The checking option is not accesible from the flow because the option `current_payment_aril_2016` has been removed.
The validation will never fail because `current_payment_aril_2016` has been removed. Hence it will always be true and can be removed.
The option `what_to_check` will never be `current_payment_april_2016` so the check can be removed.
After 1 April 2016 this branch would be outdated.
The parameter does not apply for `am-i-getting-minimum-wage` flow. It is not removed from the calculator yet because it is being used in `minimum-wage-calculator-employers`
The outcome is never reached after 1 April.
Copy should be updated to support the National Living Wage from April 2016
Notes about the changes. It has been validated with content team that: 1. Apprentice over 19 second year should get £7.20 2. Not an apprentice over 25 has been updated to £7.20 because the regression test has run on 2016. In 2014 it was £6.50
Need to be regenerated because: - Option `current_payment_april_2016` for Q1 has been removed - Question `will_you_be_a_first_year_apprentice?` has been removed
This convention has been agreed by the Digital Content team.
After 1 April 2016, if you are over 25 you will get 7.20 as the Minimum Living Wage, so the calculator should be updated accordingly. Test cases should be updated because the test data is heavily based on 25 years old to exercise the National Minimum Wages rates. Hence this commit updates them to 24 years old, and create a new test case for 25.
New logic flow is: - If over 16 but under 24 and not an apprentice, use the national minimum wage calculations - If over 25 use the national living wage rates
Add 24 years old as an answer to exercise all branches for the new logic introduced on the 1st April, as if you are over 25 you will be getting the National Living Wage.
After 1 April 2016 the copy need to be updated.
New logic flow is: - If over 16 but under 24 and not an apprentice, use the national minimum wage calculations - If over 25 use the national living wage rates
95c75f6
to
56e9c53
Compare
PR #2423 included an issue when calculating the minimum wage for past years if the user is over 25. The returned value is £7.20, which is the minimum living wage, and it should be calculated according to the the yaml file. This commit revert the artefacts as a firs step to fix this issue. This way we can start fresh, so any change on the artefacts can be checked against a working version.
PR #2423 included an issue when calculating the minimum wage for past years if the user is over 25. The returned value is £7.20, which is the minimum living wage, and it should be calculated according to the the yaml file. This commit revert the artefacts as a firs step to fix this issue. This way we can start fresh, so any change on the artefacts can be checked against a working version.
Trello card: https://trello.com/c/kZ0I4a4G/58-national-minimum-wage-april-2016-update
Covers the updates required after 1 April 2016:
Factchecks
Expected Changes
A detailed detailed description of the expected changes has been described in this document by the Content Digital Team: https://docs.google.com/document/d/1HYr7oPtx1zyGo1uiq4-dlqljsfVzR7YKRcc0zhvRksw/edit
Employee
Before
After
Employers
Before
After