{"push_id":1064716617,"size":33,"distinct_size":3,"ref":"refs/heads/marriage-abroad-content-changes-sweden","head":"f9f83ad00c9c6c77b4a50e0ad991080a4f5e7982","before":"0fe44b44601c7a39f78feacdbfd35b33d386eac3","commits":[{"sha":"aca1b46bf791e7bfac5afd8f5e5ca06f73832e48","author":{"email":"95ff60c7d7e33c5bbd817e4643a0ea35251cc1c8@digital.cabinet-office.gov.uk","name":"Leena Gupte"},"message":"Add note to docs to commit changes before updating checksum","distinct":false,"url":"https://api.github.com/repos/alphagov/smart-answers/commits/aca1b46bf791e7bfac5afd8f5e5ca06f73832e48"},{"sha":"a806305a5e96751e9965c7e0595c3763c721fbad","author":{"email":"5b8bf9c3deb8f82525b9e62900575458457cb410@gmail.com","name":"Pablo Manrubia"},"message":"Merge pull request #2449 from alphagov/fix-typo-in-state-pension-checker\n\nFix typo in state pension checker","distinct":false,"url":"https://api.github.com/repos/alphagov/smart-answers/commits/a806305a5e96751e9965c7e0595c3763c721fbad"},{"sha":"06bc3ebd00a250c2cb8535b1423ea7d0ea71d24a","author":{"email":"474ba67bdb289c6263b36dfd8a7bed6c85b04943@floehopper.org","name":"James Mead"},"message":"Restore indexable_content behaviour for outcome nodes\n\nTrello card: https://trello.com/c/nZJqFYLE\n\nDuring the transition from YAML to ERB templates this behaviour got broken and\nthen removed in order to avoid errors.\n\nNow that we have `MethodMissingHelper` we can avoid errors when outcome\ntemplates include ERB interpolation.\n\nI've made a small change to the `OutcomePresenter` constructor to make it\npossible to specify extra helper modules c.f. `QuestionPresenter`.\n\nI've inlined `NODE_PRESENTER_METHODS`, becuase it was only being used in one\nplace.\n\nI've added a new outcome template fixture in order to test ERB interpolation.","distinct":false,"url":"https://api.github.com/repos/alphagov/smart-answers/commits/06bc3ebd00a250c2cb8535b1423ea7d0ea71d24a"},{"sha":"50ab9b8d4963530eae7142f230d07ae75d04fb93","author":{"email":"474ba67bdb289c6263b36dfd8a7bed6c85b04943@floehopper.org","name":"James Mead"},"message":"Handle template exceptions when generating indexable_content\n\nI came across this problem when trying to run the `panopticon:register` rake\ntask locally as suggested by @chrisroos.\n\nUnfortunately the `MethodMissingHelper` & `MethodMissingObject` approach, which\nwe use to render node templates in the absence of any state variables, isn't\nperfect.\n\nIn particular in the `outcome_os_other_countries` outcome in `marriage-abroad`\nthere is a `case` statement which switches on `calculator.ceremony_country`.\nWhen rendering this outcome for `indexable_content` purposes, this will be an\ninstance of `MethodMissingObject` and will not match *any* of the `when`\nclauses. Thus execution will fall through to the `else` statement which raises\nan exception.\n\nI couldn't see a good way to avoid this happening and so I decided to rescue\nexceptions raised when rendering a template. This means that we won't get any\nindexable content for the affected node(s). However, I don't think this is too\nbad since only one node is currently affected.\n\nNote that we have to render the whole template to extract any one piece of\ncontent (e.g. title, body, etc), so if there an exception is raised in any one\npart of the template, then we won't be able to extract any of the pieces of\ncontent.\n\nI've also added an integration test which builds a `FlowRegistrationPresenter`\nfor each flow and calls the `indexable_content` method on each of them. This\nreplicates the essentials of the `panopticon:register` rake task which is\ncalled on deployment, although it does actually not send the indexable content\nto Panopticon.\n\nAlthough this test takes 15 seconds to run on my local machine I think it's\nworth having to avoid possible errors on deployment.\n\nNote also that the rescuing of `ActionView::Template::Error` only applies in\nthe context of the generation of indexable content at deploy time i.e. such\nexceptions will not be rescued during user requests in production.","distinct":false,"url":"https://api.github.com/repos/alphagov/smart-answers/commits/50ab9b8d4963530eae7142f230d07ae75d04fb93"},{"sha":"965592db1e6ce4aa0cdcf64250acbc26c86e9a5f","author":{"email":"474ba67bdb289c6263b36dfd8a7bed6c85b04943@floehopper.org","name":"James Mead"},"message":"Fix Lint/UselessAssignment rubocop violation","distinct":false,"url":"https://api.github.com/repos/alphagov/smart-answers/commits/965592db1e6ce4aa0cdcf64250acbc26c86e9a5f"},{"sha":"013f864165bb1c9903308e9314441963f3c845bb","author":{"email":"474ba67bdb289c6263b36dfd8a7bed6c85b04943@floehopper.org","name":"James Mead"},"message":"Merge pull request #2454 from alphagov/restore-indexable-content-behaviour-for-outcome-nodes-rebased\n\nRestore indexable_content behaviour for outcome nodes (rebased)","distinct":false,"url":"https://api.github.com/repos/alphagov/smart-answers/commits/013f864165bb1c9903308e9314441963f3c845bb"},{"sha":"a2b35380abdaac3ca5984bf8fa2ce6c6f220ad60","author":{"email":"95ff60c7d7e33c5bbd817e4643a0ea35251cc1c8@gmail.com","name":"Leena Gupte"},"message":"Merge pull request #2435 from alphagov/update-checksum-doc\n\nAdd note to docs to commit changes before updating checksum","distinct":false,"url":"https://api.github.com/repos/alphagov/smart-answers/commits/a2b35380abdaac3ca5984bf8fa2ce6c6f220ad60"},{"sha":"c628791bb714d83cc5f2d56f2f92e56b9cf676be","author":{"email":"474ba67bdb289c6263b36dfd8a7bed6c85b04943@floehopper.org","name":"James Mead"},"message":"Avoid double render error if exception raised after render\n\nThe `if performed?` conditional in `ApplicationController#error` was introduced\nin this commit [1] in order to avoid a `DoubleRenderError` if the relevant\nexception was raised after rendering had completed. However, the change was\nmade along with a test that didn't test what it was meant to test - I think the\ntest is triggering an exception *in the middle of* rendering and not *after*\nrendering.\n\nTo remedy this, I replaced the originally added controller test and added a new\nRails integration test, `ErrorHandlingTest`. This gives coverage of both\nbranches of the `if/else` statement and a failure of one of these tests\nsurfaced the fact that the line, `response_body = error_message` is not doing\nanything useful - it's just assigning the value in `error_message` to a local\nvariable, `response_body` which is not then used for anything.\n\nIn fact I first came across this because this method was causing a\n`Lint/UselessAssignment` Rubocop violation where a `response_body` local\nvariable was being assigned, but not used. I _think_ the intention was probably\nto use the `ActionController::Metal#response_body` writer method.\n\nI fixed the failing test by changing `response_body = error_message` to\n`self.response_body = error_message`.\n\nI also decided to change `response.status = status_code` so that it too uses a\nwriter method on `ActionController::Metal` rather than accessing the\n`response`. The latter is only made available by\n`ActionController::RackDelegation` i.e. it may not be available in all\nrequests. Doing this felt more consistent and more robust.\n\nI had to use `set_slimmer_headers(skip: true)` in the test controller action,\nbecause Slimmer ignores the body content of an error page and so we'd never see\nwhat it's being set to. The body content is ignored because\n`Slimmer::Skin#error` does not include `Slimmer::Processors::BodyInserter` in\nthe list of processors for the request.\n\nThus with Slimmer enabled the relevant ERB error template (e.g. 404.html.erb\n[2] and its related partial [3]) from the static app is the main source of the\nerror page, albeit with a few things changed e.g. the page title.\n\n[1]: 168b69edf25c2a816be2b947c93c4d6630b3f6a6\n[2]: https://github.com/alphagov/static/blob/master/app/views/root/404.html.erb\n[3]: https://github.com/alphagov/static/blob/master/app/views/root/_error_page.html.erb","distinct":false,"url":"https://api.github.com/repos/alphagov/smart-answers/commits/c628791bb714d83cc5f2d56f2f92e56b9cf676be"},{"sha":"831f5b5e8f40e8be25a44dc25cb78e08dad06147","author":{"email":"474ba67bdb289c6263b36dfd8a7bed6c85b04943@floehopper.org","name":"James Mead"},"message":"Merge pull request #2400 from alphagov/replace-response-body-on-error-if-render-already-performed\n\nAvoid double render error if exception raised after render","distinct":false,"url":"https://api.github.com/repos/alphagov/smart-answers/commits/831f5b5e8f40e8be25a44dc25cb78e08dad06147"},{"sha":"3bc732c8c8811e4e8ae1c26e0c8e310e4e3ee9e6","author":{"email":"474ba67bdb289c6263b36dfd8a7bed6c85b04943@floehopper.org","name":"James Mead"},"message":"Add unit test for SmartAnswer::Calculation\n\nNote that this is just documenting the existing behaviour - it's not intended\nas an endorsement of the existing behaviour.","distinct":false,"url":"https://api.github.com/repos/alphagov/smart-answers/commits/3bc732c8c8811e4e8ae1c26e0c8e310e4e3ee9e6"},{"sha":"46fcf2f99b4ae1e4bdf56fa64eee388a8ab5a29a","author":{"email":"474ba67bdb289c6263b36dfd8a7bed6c85b04943@floehopper.org","name":"James Mead"},"message":"Introduce SmartAnswer::Block\n\nThis does pretty much everything that `SmartAnswer::Calculation` does, except\nthat it doesn't assign the result of the block to a specified state variable.\nHopefully you can see this by comparing the unit tests for the two classes.\n\nI'm planning to use this to introduce a new type of block to the Flow/Question\nDSL which will be called immediately after receiving the response from the\nuser. However, I've named the new class very generically, because there's no\nreason it couldn't be used at other points in the DSL.\n\nI did contemplate other slightly more specific names e.g.\n`NonAssigningCalculation` and `BlockEvaluatableInContextOfState`.\n\nFor the former, I thought that the `Calculation` suffix was a bit confusing,\nbecause I think it conveys the idea that it has a result.\n\nI thought the latter was unwieldy and came to the conclusion that really this\nis just a block - it's just enhanced with a method that allows it to be called\nin the context of a `SmartAnswer::State`. And the method signature of the\n`#evaluate` method is enough to explain the latter.\n\nThus I think naming it `SmartAnswer::Block` is the best option.","distinct":false,"url":"https://api.github.com/repos/alphagov/smart-answers/commits/46fcf2f99b4ae1e4bdf56fa64eee388a8ab5a29a"},{"sha":"63795f28201bb80c1d475a22e919ea7264969b95","author":{"email":"474ba67bdb289c6263b36dfd8a7bed6c85b04943@floehopper.org","name":"James Mead"},"message":"Introduce on_response blocks to flow/question DSL\n\nThe idea is that we'll use these blocks to set attributes on the\ncalculator/policy object instead of doing that in `next_node` blocks.\n\nOne advantage of doing this is to separate out the attribute-setting code which\nshould make it easier to do automatically in future.\n\nAnother advantage is that we can avoid having to pass the response into any\nvalidation-related methods on the calculator which should make them more\nActiveModel-like.\n\nThe new `on_response` blocks are run *before* both `next_node_calculation` and\n`validate` blocks.\n\nThe existing unit tests around this area of the codebase leave a bit to be\ndesired, but I've added new tests as best I can roughly following existing\npatterns. I did also contemplate adding an `EngineIntegrationTest` with a\nnew fixture flow, but I decided that it would be overkill and added the test to\n`FlowTest` instead. I feel as if this gives sufficient test coverage for the\nnew functionality.","distinct":false,"url":"https://api.github.com/repos/alphagov/smart-answers/commits/63795f28201bb80c1d475a22e919ea7264969b95"},{"sha":"75611b30ae326f56f13bce2d678b7416135784ec","author":{"email":"474ba67bdb289c6263b36dfd8a7bed6c85b04943@floehopper.org","name":"James Mead"},"message":"Make CalculateStatutorySickPayFlowTest more realistic\n\nUse a real instance of the calculator instead of a stub:\n\n* Using a real object rather than a stub means that we'll see an error if we\n  stub a non-existent method.\n* We can re-use the calculator instance created in the top-level setup.\n* Stubbing simple accessor methods seems like overkill.","distinct":false,"url":"https://api.github.com/repos/alphagov/smart-answers/commits/75611b30ae326f56f13bce2d678b7416135784ec"},{"sha":"4b65fc7cc9fff829dfe1980a175439ea30f5a895","author":{"email":"474ba67bdb289c6263b36dfd8a7bed6c85b04943@floehopper.org","name":"James Mead"},"message":"DRY up CalculateStatutorySickPayFlowTest","distinct":false,"url":"https://api.github.com/repos/alphagov/smart-answers/commits/4b65fc7cc9fff829dfe1980a175439ea30f5a895"},{"sha":"36e25ae0c8d5ae09cbe4c983401d8db07425a83f","author":{"email":"474ba67bdb289c6263b36dfd8a7bed6c85b04943@floehopper.org","name":"James Mead"},"message":"Remove unnecessary setup from CalculateStatutorySickPayFlowTest\n\nI think these are remnants from when we weren't stubbing the validation\nmethods on the calculator.","distinct":false,"url":"https://api.github.com/repos/alphagov/smart-answers/commits/36e25ae0c8d5ae09cbe4c983401d8db07425a83f"},{"sha":"5147d6d9b2c3c5deb65bba7bccc9dbf55cb72ca7","author":{"email":"474ba67bdb289c6263b36dfd8a7bed6c85b04943@floehopper.org","name":"James Mead"},"message":"Use on_response blocks in calculate-statutory-sick-pay\n\nIt seems better to separate the setting of attributes on the calculator from\nthe routing logic in `next_node` blocks. The recently introduced `on_response`\nblock allows us to do this. It also means the response will be available on the\ncalculator as an attribute when validation methods are called which means we\nwon't need to pass the response into these methods, thus simplifying the code.\n\nNote that I've had to set the `calculator` state variable in the first\n`on_response` block, because these blocks are executed before\n`next_node_calculation` blocks.","distinct":false,"url":"https://api.github.com/repos/alphagov/smart-answers/commits/5147d6d9b2c3c5deb65bba7bccc9dbf55cb72ca7"},{"sha":"b2fe9c04efdb4efa37f2d0589ef1e536314e0537","author":{"email":"474ba67bdb289c6263b36dfd8a7bed6c85b04943@floehopper.org","name":"James Mead"},"message":"Simplify validation methods in calculate-statutory-sick-pay\n\nNow that the responses are available as attributes we don't need to pass the\nresponse into the validation methods. This makes the validations closer to the\nActiveModel style.\n\nI've had to use a slight bodge in `#valid_last_payday_before_offset?`. I've\nadded 1 day to the RHS to counteract the fact that the validation was previously\ndone on the raw response rather than `relevant_period_from` which is the raw\nresponse plus 1 day. I'm planning to improve this later as part of this Trello\ncard [1].\n\n[1]: https://trello.com/c/uu0HeZee","distinct":false,"url":"https://api.github.com/repos/alphagov/smart-answers/commits/b2fe9c04efdb4efa37f2d0589ef1e536314e0537"},{"sha":"6ab84975cb0555b3d9bbcd814777124258f974bb","author":{"email":"474ba67bdb289c6263b36dfd8a7bed6c85b04943@floehopper.org","name":"James Mead"},"message":"Fix Lint/UnusedMethodArgument Rubocop violation","distinct":false,"url":"https://api.github.com/repos/alphagov/smart-answers/commits/6ab84975cb0555b3d9bbcd814777124258f974bb"},{"sha":"8b2e5c2482ffbbed7f54f9b3393e9e33961a0822","author":{"email":"474ba67bdb289c6263b36dfd8a7bed6c85b04943@floehopper.org","name":"James Mead"},"message":"Fix Style/ClosingParenthesisIndentation Rubocop violations","distinct":false,"url":"https://api.github.com/repos/alphagov/smart-answers/commits/8b2e5c2482ffbbed7f54f9b3393e9e33961a0822"},{"sha":"8ed258e35165b7580809d550b9b13a5f7150378d","author":{"email":"474ba67bdb289c6263b36dfd8a7bed6c85b04943@floehopper.org","name":"James Mead"},"message":"Update regression test checksums for calculate-statutory-sick-pay\n\nThe regression tests all pass.","distinct":false,"url":"https://api.github.com/repos/alphagov/smart-answers/commits/8ed258e35165b7580809d550b9b13a5f7150378d"}]}