{"action":"created","issue":{"url":"https://api.github.com/repos/alphagov/smart-answers/issues/1827","labels_url":"https://api.github.com/repos/alphagov/smart-answers/issues/1827/labels{/name}","comments_url":"https://api.github.com/repos/alphagov/smart-answers/issues/1827/comments","events_url":"https://api.github.com/repos/alphagov/smart-answers/issues/1827/events","html_url":"https://github.com/alphagov/smart-answers/pull/1827","id":96036395,"number":1827,"title":"Always expose cost_change_4_weeks to views","user":{"login":"tadast","id":218239,"avatar_url":"https://avatars.githubusercontent.com/u/218239?v=3","gravatar_id":"","url":"https://api.github.com/users/tadast","html_url":"https://github.com/tadast","followers_url":"https://api.github.com/users/tadast/followers","following_url":"https://api.github.com/users/tadast/following{/other_user}","gists_url":"https://api.github.com/users/tadast/gists{/gist_id}","starred_url":"https://api.github.com/users/tadast/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tadast/subscriptions","organizations_url":"https://api.github.com/users/tadast/orgs","repos_url":"https://api.github.com/users/tadast/repos","events_url":"https://api.github.com/users/tadast/events{/privacy}","received_events_url":"https://api.github.com/users/tadast/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"milestone":null,"comments":4,"created_at":"2015-07-20T10:37:06Z","updated_at":"2015-07-21T09:50:26Z","closed_at":"2015-07-20T13:30:28Z","pull_request":{"url":"https://api.github.com/repos/alphagov/smart-answers/pulls/1827","html_url":"https://github.com/alphagov/smart-answers/pull/1827","diff_url":"https://github.com/alphagov/smart-answers/pull/1827.diff","patch_url":"https://github.com/alphagov/smart-answers/pull/1827.patch"},"body":"This is addressing an error from errbit.\r\nSample affected path: /childcare-costs-for-tax-credits/y/yes/yes/monthly_diff_amount/2400.0/127.0\r\n\r\ncost_change_4_weeks was only set if it's true, otherwise erb views could not\r\nsee it as a defined variable. There was also no input set for regression tests\r\nthat covers a case where `ten_or_more` is true, therefore this code branch\r\nin erb was never exected.\r\n\r\nThe issue is fixed by explicitly exposing cost_change_4_weeks for the outcome\r\nthat may need it and setting it to false unless it's already set.\r\n\r\nAdd a regression test input to reproduce the issue and some extra assertions\r\nto highlight the importance of cost_change_4_weeks.\r\n\r\nThis is very similar to https://github.com/alphagov/smart-answers/pull/1826"},"comment":{"url":"https://api.github.com/repos/alphagov/smart-answers/issues/comments/123244654","html_url":"https://github.com/alphagov/smart-answers/pull/1827#issuecomment-123244654","issue_url":"https://api.github.com/repos/alphagov/smart-answers/issues/1827","id":123244654,"user":{"login":"chrisroos","id":6556,"avatar_url":"https://avatars.githubusercontent.com/u/6556?v=3","gravatar_id":"","url":"https://api.github.com/users/chrisroos","html_url":"https://github.com/chrisroos","followers_url":"https://api.github.com/users/chrisroos/followers","following_url":"https://api.github.com/users/chrisroos/following{/other_user}","gists_url":"https://api.github.com/users/chrisroos/gists{/gist_id}","starred_url":"https://api.github.com/users/chrisroos/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chrisroos/subscriptions","organizations_url":"https://api.github.com/users/chrisroos/orgs","repos_url":"https://api.github.com/users/chrisroos/repos","events_url":"https://api.github.com/users/chrisroos/events{/privacy}","received_events_url":"https://api.github.com/users/chrisroos/received_events","type":"User","site_admin":false},"created_at":"2015-07-21T09:50:26Z","updated_at":"2015-07-21T09:50:26Z","body":"> 1) To avoid confusion. In current master you can access any 'variable' on a State because it inherits from OpenStruct. However when we pass values to the view we only consider variables that are explicitly set. This makes it a bit confusing.\r\n\r\nI agree that this is confusing. The reason I asked about the `|| false` part of the `precalculate` block is that we've added these sort of `precalculate` blocks elsewhere but in every other case we're simply returning the same variable (i.e. we're not conditionally setting it to something else).\r\n\r\n> 2) Reliable testing. Explicitly setting the variable to false allows for a more reliable testing, see here. If I were to assert for nil the tests would pass on master, which is not what I want.\r\n\r\nThat makes sense. I'd like to think we'll get to a point where testing the state variables directly isn't necessary but we're not there yet so this seems like a good approach for now."}}