{"action":"created","issue":{"url":"https://api.github.com/repos/HTBox/allReady/issues/607","repository_url":"https://api.github.com/repos/HTBox/allReady","labels_url":"https://api.github.com/repos/HTBox/allReady/issues/607/labels{/name}","comments_url":"https://api.github.com/repos/HTBox/allReady/issues/607/comments","events_url":"https://api.github.com/repos/HTBox/allReady/issues/607/events","html_url":"https://github.com/HTBox/allReady/pull/607","id":141327979,"number":607,"title":"TaskApiController: adding unit tests and refactoring to mediator","user":{"login":"mgmccarthy","id":2428993,"avatar_url":"https://avatars.githubusercontent.com/u/2428993?v=3","gravatar_id":"","url":"https://api.github.com/users/mgmccarthy","html_url":"https://github.com/mgmccarthy","followers_url":"https://api.github.com/users/mgmccarthy/followers","following_url":"https://api.github.com/users/mgmccarthy/following{/other_user}","gists_url":"https://api.github.com/users/mgmccarthy/gists{/gist_id}","starred_url":"https://api.github.com/users/mgmccarthy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mgmccarthy/subscriptions","organizations_url":"https://api.github.com/users/mgmccarthy/orgs","repos_url":"https://api.github.com/users/mgmccarthy/repos","events_url":"https://api.github.com/users/mgmccarthy/events{/privacy}","received_events_url":"https://api.github.com/users/mgmccarthy/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"MisterJames","id":1197383,"avatar_url":"https://avatars.githubusercontent.com/u/1197383?v=3","gravatar_id":"","url":"https://api.github.com/users/MisterJames","html_url":"https://github.com/MisterJames","followers_url":"https://api.github.com/users/MisterJames/followers","following_url":"https://api.github.com/users/MisterJames/following{/other_user}","gists_url":"https://api.github.com/users/MisterJames/gists{/gist_id}","starred_url":"https://api.github.com/users/MisterJames/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/MisterJames/subscriptions","organizations_url":"https://api.github.com/users/MisterJames/orgs","repos_url":"https://api.github.com/users/MisterJames/repos","events_url":"https://api.github.com/users/MisterJames/events{/privacy}","received_events_url":"https://api.github.com/users/MisterJames/received_events","type":"User","site_admin":false},"milestone":null,"comments":11,"created_at":"2016-03-16T16:32:21Z","updated_at":"2016-04-12T13:42:07Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/HTBox/allReady/pulls/607","html_url":"https://github.com/HTBox/allReady/pull/607","diff_url":"https://github.com/HTBox/allReady/pull/607.diff","patch_url":"https://github.com/HTBox/allReady/pull/607.patch"},"body":"Fixes #604\r\n\r\nA couple notes:\r\n\r\nFirst, I added an abstraction called `IDetermineIfATaskIsEditable` that is injected into TaskApiController via constructor injection. The implementation is in the `DetermineIfATaskIsEditable` class. The goal here was to encapsulate, the code that previously was in TaskApiController's `private bool HasTaskEditPermissions(AllReadyTask task)` method.\r\n\r\nBreaking this out into a separate class allowed me to unit test whether a user has permissions to edit the task or if the AllReadyTask is in an editable state. \r\n\r\nThat freed the Post, Put and Delete methods from having to worry about the setup and outcome of this check, as well as I only had to write the unit test for \"is editable\" once instead of every time I had to write a unit test against any of these three methods in the controller.\r\n\r\nSecond, as much as I wanted to, I could not remove the injection of IAllReadyDataAccess from TaskApiController's constructor, even moving 100% to mediator.\r\n\r\nThe reason is this call in the fist line of the Post method:\r\n`task.ToModel(_allReadyDataAccess);`\r\n\r\n'ToModel' does a BUNCH of work turning the TaskViewModel into an AllReadyTask, including making database queires via IAllReadyDataAccess. There are also a larege number of other things going on in here, so much so, that I would say someone needs to take a look at this code, and either start to decompose it, or change some of these rules to live somehwere else. I don't know enough about the app to make those calls.\r\n\r\nTwo good steps would be:\r\n- removing the injected IAllReadyDataAccess\r\n- writing unit tests around this view model class\r\n\r\nthat could start the ball rolling."},"comment":{"url":"https://api.github.com/repos/HTBox/allReady/issues/comments/208911043","html_url":"https://github.com/HTBox/allReady/pull/607#issuecomment-208911043","issue_url":"https://api.github.com/repos/HTBox/allReady/issues/607","id":208911043,"user":{"login":"mgmccarthy","id":2428993,"avatar_url":"https://avatars.githubusercontent.com/u/2428993?v=3","gravatar_id":"","url":"https://api.github.com/users/mgmccarthy","html_url":"https://github.com/mgmccarthy","followers_url":"https://api.github.com/users/mgmccarthy/followers","following_url":"https://api.github.com/users/mgmccarthy/following{/other_user}","gists_url":"https://api.github.com/users/mgmccarthy/gists{/gist_id}","starred_url":"https://api.github.com/users/mgmccarthy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mgmccarthy/subscriptions","organizations_url":"https://api.github.com/users/mgmccarthy/orgs","repos_url":"https://api.github.com/users/mgmccarthy/repos","events_url":"https://api.github.com/users/mgmccarthy/events{/privacy}","received_events_url":"https://api.github.com/users/mgmccarthy/received_events","type":"User","site_admin":false},"created_at":"2016-04-12T13:42:07Z","updated_at":"2016-04-12T13:42:07Z","body":"ping @MisterJames. any movement on this PR?"}}