{"action":"created","issue":{"url":"https://api.github.com/repos/code-corps/code-corps-api/issues/448","repository_url":"https://api.github.com/repos/code-corps/code-corps-api","labels_url":"https://api.github.com/repos/code-corps/code-corps-api/issues/448/labels{/name}","comments_url":"https://api.github.com/repos/code-corps/code-corps-api/issues/448/comments","events_url":"https://api.github.com/repos/code-corps/code-corps-api/issues/448/events","html_url":"https://github.com/code-corps/code-corps-api/pull/448","id":187873745,"number":448,"title":"Refactor Task Controller Tests with helpers","user":{"login":"amyschools","id":19355893,"avatar_url":"https://avatars.githubusercontent.com/u/19355893?v=3","gravatar_id":"","url":"https://api.github.com/users/amyschools","html_url":"https://github.com/amyschools","followers_url":"https://api.github.com/users/amyschools/followers","following_url":"https://api.github.com/users/amyschools/following{/other_user}","gists_url":"https://api.github.com/users/amyschools/gists{/gist_id}","starred_url":"https://api.github.com/users/amyschools/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/amyschools/subscriptions","organizations_url":"https://api.github.com/users/amyschools/orgs","repos_url":"https://api.github.com/users/amyschools/repos","events_url":"https://api.github.com/users/amyschools/events{/privacy}","received_events_url":"https://api.github.com/users/amyschools/received_events","type":"User","site_admin":false},"labels":[{"id":409301452,"url":"https://api.github.com/repos/code-corps/code-corps-api/labels/help%20wanted","name":"help wanted","color":"128A0C","default":true},{"id":409365921,"url":"https://api.github.com/repos/code-corps/code-corps-api/labels/in%20progress","name":"in progress","color":"c2e0c6","default":false},{"id":409366045,"url":"https://api.github.com/repos/code-corps/code-corps-api/labels/needs%20submitter%20response","name":"needs submitter response","color":"fbca04","default":false}],"state":"open","locked":false,"assignee":{"login":"amyschools","id":19355893,"avatar_url":"https://avatars.githubusercontent.com/u/19355893?v=3","gravatar_id":"","url":"https://api.github.com/users/amyschools","html_url":"https://github.com/amyschools","followers_url":"https://api.github.com/users/amyschools/followers","following_url":"https://api.github.com/users/amyschools/following{/other_user}","gists_url":"https://api.github.com/users/amyschools/gists{/gist_id}","starred_url":"https://api.github.com/users/amyschools/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/amyschools/subscriptions","organizations_url":"https://api.github.com/users/amyschools/orgs","repos_url":"https://api.github.com/users/amyschools/repos","events_url":"https://api.github.com/users/amyschools/events{/privacy}","received_events_url":"https://api.github.com/users/amyschools/received_events","type":"User","site_admin":false},"assignees":[{"login":"amyschools","id":19355893,"avatar_url":"https://avatars.githubusercontent.com/u/19355893?v=3","gravatar_id":"","url":"https://api.github.com/users/amyschools","html_url":"https://github.com/amyschools","followers_url":"https://api.github.com/users/amyschools/followers","following_url":"https://api.github.com/users/amyschools/following{/other_user}","gists_url":"https://api.github.com/users/amyschools/gists{/gist_id}","starred_url":"https://api.github.com/users/amyschools/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/amyschools/subscriptions","organizations_url":"https://api.github.com/users/amyschools/orgs","repos_url":"https://api.github.com/users/amyschools/repos","events_url":"https://api.github.com/users/amyschools/events{/privacy}","received_events_url":"https://api.github.com/users/amyschools/received_events","type":"User","site_admin":false}],"milestone":null,"comments":3,"created_at":"2016-11-08T01:00:24Z","updated_at":"2016-11-09T11:16:36Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/code-corps/code-corps-api/pulls/448","html_url":"https://github.com/code-corps/code-corps-api/pull/448","diff_url":"https://github.com/code-corps/code-corps-api/pull/448.diff","patch_url":"https://github.com/code-corps/code-corps-api/pull/448.patch"},"body":"# What's in this PR?\r\n\r\nAdd api_case.ex helpers to Task controller tests.\r\n\r\nStuck on a fix to make two tests pass: update: create and update: renders 422 error. \r\n\r\n## References\r\nCloses #442 \r\n\r\nProgress on: #413 \r\n"},"comment":{"url":"https://api.github.com/repos/code-corps/code-corps-api/issues/comments/259390934","html_url":"https://github.com/code-corps/code-corps-api/pull/448#issuecomment-259390934","issue_url":"https://api.github.com/repos/code-corps/code-corps-api/issues/448","id":259390934,"user":{"login":"begedin","id":1925540,"avatar_url":"https://avatars.githubusercontent.com/u/1925540?v=3","gravatar_id":"","url":"https://api.github.com/users/begedin","html_url":"https://github.com/begedin","followers_url":"https://api.github.com/users/begedin/followers","following_url":"https://api.github.com/users/begedin/following{/other_user}","gists_url":"https://api.github.com/users/begedin/gists{/gist_id}","starred_url":"https://api.github.com/users/begedin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/begedin/subscriptions","organizations_url":"https://api.github.com/users/begedin/orgs","repos_url":"https://api.github.com/users/begedin/repos","events_url":"https://api.github.com/users/begedin/events{/privacy}","received_events_url":"https://api.github.com/users/begedin/received_events","type":"User","site_admin":false},"created_at":"2016-11-09T11:16:35Z","updated_at":"2016-11-09T11:16:35Z","body":"@amyschools Looks like you didn't push up, or at least, I'm not seeing the changes. I'll try to reply best I can.\r\n\r\nFor the update test, we have the following: \r\n\r\n```Elixir\r\ndescribe \"update\" do\r\n  @tag authenticated: :admin\r\n  test \"updates and renders chosen resource when data is valid\", %{conn: conn, current_user: current_user} do\r\n    task = insert(:task, user: current_user)\r\n\r\n    assert conn |> request_update(@valid_attrs) |> json_response(200)\r\n  end\r\n\r\n  @tag :authenticated\r\n  test \"renders 422 when data is invalid\", %{conn: conn, current_user: current_user} do\r\n      assert conn |> request_update(@invalid_attrs) |> json_response(422)\r\n   end\r\n\r\n  # bits removed\r\nend\r\n```\r\n\r\nFor the first test, it's tagged to authenticate as admin.  \r\n\r\nYou create a task, but then create the version of the `request_update` function that doesn't accept a task and instead creates its own resource. The tests pass, since you're authenticated as site admin, but the correct approach would be either to \r\n\r\n### Option A\r\n\r\nRemove the `task = insert(:task, user: current_user)`line (and also remove the current user from the setup hash in the test signature), \r\n\r\n### Option B\r\n\r\nKeep the task line, tag as `:authenticated`(so not `authenticated: :admin`) and then call\r\n\r\n```Elixir\r\nassert conn |> request_update(@valid_attrs) |> json_response(200)\r\n```\r\n\r\nMy vote would be option B, since as a side-effect, it's testing a more complex case of authorization.\r\n\r\nFor the second test, as you can see, you're not authenticating as admin, which is why it's failing. If you go with option B, then the solution here would be to basically copy everything from the first test and just replace `@valid_attrs` with `@invalid_attrs`. That should do the trick, so you would end up with:\r\n\r\n```Elixir\r\n@tag :authenticated\r\ntest \"renders 422 when data is invalid\", %{conn: conn, current_user: current_user} do\r\n  task = insert(:task, user: current_user)\r\n  assert conn |> request_update(task, @invalid_attrs) |> json_response(422)\r\nend\r\n```\r\n\r\nFor create, the problem is similar, but not quite the same\r\n\r\n```\r\nassert conn |> request_create(@invalid_attrs) |> json_response(422)\r\n```\r\n\r\n`@invalid_attrs` doesn't have a user_id or a project id assigned. The authorization runs, tries to see if the user is the author of the task and figures out it not (philosophical, really 😄 ) and concludes that you are not authorized.\r\n\r\nIn order to get it working, you mimic the valid request approach:\r\n\r\n```Elixir\r\nproject = insert(:project)\r\nattrs = @valid_attrs |> Map.merge(%{project: project, user: current_user})\r\nassert conn |> request_create(attrs) |> json_response(201)]\r\n```\r\n\r\nThe difference being, you replace `@valid_attrs` with `@invalid_attrs`.\r\n\r\n```Elixir\r\nproject = insert(:project)\r\nattrs = @invalid_attrs |> Map.merge(%{project: project, user: current_user})\r\nassert conn |> request_create(attrs) |> json_response(422)\r\n```\r\n\r\nIt's a bit complicated, but basically, in all our cases, we run authorization before we validate the record. That means that, to test validation error responses, you want to set up your request so authorization passes, but validation fails after. On some controllers, that isn't even possible, but in this one, it is, albeit it gets a bit complicated.\r\n\r\n@JoshSmith Writing all this, maybe we should at one point look into our process being something like:\r\n\r\n```\r\nget_record_if_applicable\r\ninitialize_changeset_for_record_and_attributes\r\nrespond_with_422_if_changeset_invalid\r\nauthorize_if_changeset_valid\r\nrespond_with_403_if_unauthorized\r\nrespond_with_ok_if_authorized\r\n```\r\n\r\nIt might make many things simpler, but I'm not sure.\r\n\r\nFor your create and update valid request tests, it seems to me like they should work, from the changes here. However, it may be you altered something, so I'll take another look when you push your changes up."}}
{"actor":{"display_login":"begedin"}}