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.

Sign up for GitHub

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

Jump to bottom

Allow RatesQuery date to be set using environment variable #2412

Merged
merged 20 commits into from
Mar 31, 2016
Merged

Allow RatesQuery date to be set using environment variable #2412

merged 20 commits into from
Mar 31, 2016

Conversation

chrisroos
Copy link
Contributor

This will allow us to deploy rates changes to Heroku and ask someone from the content team/department to factcheck the changes. This was previously hard because the RatesQuery defaulted to today's date.

This was suggested by @floehopper in a comment on the Trello card about updating the rates for register-a-birth.

I tested that this was working as expected by setting the RATES_QUERY_DATE environment variable and viewing an outcome for the register-a-birth Smart Answer.

Before

$ rails s
$ open http://localhost:3000/register-a-birth/y/afghanistan/mother/yes/same_country#pay

pr-2412-before

Observe that it's displaying the latest rates available in births_and_deaths_document_return_fees.yml.

After

$ RATES_QUERY_DATE=2015-07-30 rails s
$ open http://localhost:3000/register-a-birth/y/afghanistan/mother/yes/same_country#pay

pr-2412-after

Observe that it's displaying the earlier rates available in births_and_deaths_document_return_fees.yml.

@chrisroos chrisroos mentioned this pull request Mar 30, 2016
Closed
@ikennaokpala ikennaokpala self-assigned this Mar 30, 2016
@ikennaokpala
Copy link
Contributor

@chrisroos great piece of work.. should be good to go for #2407 et al.

I tested this locally with:

RATES_QUERY_DATE=2015-07-30 GOVUK_APP_DOMAIN=integration.publishing.service.gov.uk PLEK_SERVICE_CONTENTAPI_URI=https://www.gov.uk/api PLEK_SERVICE_STATIC_URI=https://assets-origin.integration.publishing.service.gov.uk  bundle exec rails s -p 4000

I suppose one would have to update this (RATES_QUERY_DATE) manually on heroku for factcheck..

@chrisroos
Copy link
Contributor Author

Thanks, @ikennaokpala!

I suppose one would have to update this (RATES_QUERY_DATE) manually on heroku for factcheck..

That's correct. You've reminded me that I should update the docs to explain how to do this. I'll do that as part of this branch and then get this merged.

chrisroos added 20 commits April 1, 2016 12:01
@chrisroos
Avoid unnecessary namespacing
c8c631b 
The test is already in the `SmartAnswer::Calculators` namespace so we don't
need to prefix `RatesQuery`.
@chrisroos
Add load_path param to RatesQuery#initialize
6db50c4 
To avoid having to stub a private method in our tests.
@chrisroos
Add RatesQuery.from_file
04c373a 
I want RatesQuery#initialize to take an array of rates so that it's easier to
test without having to amend external YAML files of data.
@chrisroos
Update RatesQuery#initialize
c7601da 
So that it accepts an array of rates hashes, instead of a filename. This should
make it easier to test some changes I want to make to this class.
@chrisroos
Use assertion to make test more explicit
69aa7c6 
I _think_ this makes it clearer that we want #rates to return different values
when called with different arguments.
@chrisroos
Use setup block to DRY up test
796e24f
@chrisroos
Rename relevant_date parameter
2fc0bd9 
I think `date` is clear enough as a parameter name.
@chrisroos
Add test for default date value
05c3714 
I want a test around the existing behaviour before changing the implementation.

Note the use of `Date.today - 1.day` and `Date.today + 1.day` to obtain
yesterday's and today's dates. I initially used `Date.yesterday` and
`Date.tomorrow` but in some cases found that `Date.tomorrow` returned
the same date as `Date.today` (see below). It's possile that this is
related to timezones but I haven't spent any time investigating.

    $ rails c
    >> Time.now
    => 2016-04-01 12:18:53 +1300
    >> Date.yesterday
    => Wed, 30 Mar 2016
    >> Date.today
    => Fri, 01 Apr 2016
    >> Date.tomorrow
    => Fri, 01 Apr 2016
@chrisroos
Avoid setting default date in method signature
c309bcd 
In preparation for adding support for setting the date using an environment
variable.
@chrisroos
Add support for setting date using environment variable
b7b3925 
This should make it possible to deploy rate changes to Heroku and have them
factchecked by people in the content team/department.
@chrisroos
Update checksum data for calculate-employee-redundancy-pay
89ce3fe 
The regression tests are passing so I'm happy to update the checksum
data.

Updated using:

    $ rails r script/generate-checksums-for-smart-answer.rb \
    calculate-employee-redundancy-pay
@chrisroos
Update checksum data for calculate-married-couples-allowance
f6f6eb1 
The regression tests are passing so I'm happy to update the checksum
data.

Updated using:

    $ rails r script/generate-checksums-for-smart-answer.rb \
    calculate-married-couples-allowance
@chrisroos
Update checksum data for calculate-statutory-sick-pay
cbbfe3e 
The regression tests are passing so I'm happy to update the checksum
data.

Updated using:

    $ rails r script/generate-checksums-for-smart-answer.rb \
    calculate-statutory-sick-pay
@chrisroos
Update checksum data for calculate-your-redundancy-pay
022bc9e 
The regression tests are passing so I'm happy to update the checksum
data.

Updated using:

    $ rails r script/generate-checksums-for-smart-answer.rb \
    calculate-your-redundancy-pay
@chrisroos
Update checksum data for marriage-abroad
f3ec969 
The regression tests are passing so I'm happy to update the checksum
data.

Updated using:

    $ rails r script/generate-checksums-for-smart-answer.rb \
    marriage-abroad
@chrisroos
Update checksum data for maternity-paternity-calculator
be41fe4 
The regression tests are passing so I'm happy to update the checksum
data.

Updated using:

    $ rails r script/generate-checksums-for-smart-answer.rb \
    maternity-paternity-calculator
@chrisroos
Update checksum data for register-a-birth
7138531 
The regression tests are passing so I'm happy to update the checksum
data.

Updated using:

    $ rails r script/generate-checksums-for-smart-answer.rb \
    register-a-birth
@chrisroos
Update checksum data for register-a-death
fa59fe2 
The regression tests are passing so I'm happy to update the checksum
data.

Updated using:

    $ rails r script/generate-checksums-for-smart-answer.rb \
    register-a-death
@chrisroos
Update checksum data for state-pension-through-partner
3edd968 
The regression tests are passing so I'm happy to update the checksum
data.

Updated using:

    $ rails r script/generate-checksums-for-smart-answer.rb \
    state-pension-through-partner
@chrisroos
Update factcheck.md
0bc9e19 
Add instructions for setting `RATES_QUERY_DATE` on Heroku.
@chrisroos chrisroos force-pushed the allow-rates-query-date-to-be-set-using-environment-variable branch from bfb0bd6 to 0bc9e19 Compare March 31, 2016 23:47
@chrisroos
Copy link
Contributor Author

I've updated the factcheck docs in 0bc9e19.

I've rebased this on master and force pushed (there were no commit-comments) in preparation for merging.

@chrisroos chrisroos merged commit a18f98b into master Mar 31, 2016
@chrisroos chrisroos deleted the allow-rates-query-date-to-be-set-using-environment-variable branch March 31, 2016 23:51
@ikennaokpala ikennaokpala mentioned this pull request Jan 8, 2017
Merged
This was referenced Jan 8, 2017
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@ikennaokpala ikennaokpala

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@chrisroos @ikennaokpala