{"action":"created","comment":{"url":"https://api.github.com/repos/DoSomething/northstar/pulls/comments/171925229","pull_request_review_id":100868608,"id":171925229,"diff_hunk":"@@ -0,0 +1,190 @@\n+<?php\n+\n+namespace Northstar\\Http\\Controllers\\Two;\n+\n+use Illuminate\\Http\\Request;\n+use Northstar\\Auth\\Registrar;\n+use Northstar\\Auth\\Role;\n+use Northstar\\Exceptions\\NorthstarValidationException;\n+use Northstar\\Http\\Transformers\\Two\\UserTransformer;\n+use Northstar\\Models\\User;\n+use Symfony\\Component\\HttpKernel\\Exception\\NotFoundHttpException;\n+use Northstar\\Http\\Controllers\\Controller;\n+\n+class UserController extends Controller\n+{\n+    /**\n+     * The registrar handles creating, updating, and\n+     * validating user accounts.\n+     *\n+     * @var Registrar\n+     */\n+    protected $registrar;\n+\n+    /**\n+     * @var UserTransformer\n+     */\n+    protected $transformer;\n+\n+    /**\n+     * Make a new UserController, inject dependencies,\n+     * and set middleware for this controller's methods.\n+     *\n+     * @param Registrar $registrar\n+     * @param UserTransformer $transformer\n+     */\n+    public function __construct(Registrar $registrar, UserTransformer $transformer)\n+    {\n+        $this->registrar = $registrar;\n+        $this->transformer = $transformer;\n+\n+        $this->middleware('role:admin,staff', ['except' => ['show']]);\n+    }\n+\n+    /**\n+     * Display a listing of the resource.\n+     * GET /users\n+     *\n+     * @param Request $request\n+     * @return \\Illuminate\\Http\\Response\n+     */\n+    public function index(Request $request)\n+    {\n+        // Create an empty User query, which we can either filter (below)\n+        // or paginate to retrieve all user records.\n+        $query = $this->newQuery(User::class);\n+\n+        // Use `?filter[column]=value` for exact matches.\n+        $filters = normalize('credentials', $request->query('filter'));\n+        $query = $this->filter($query, $filters, User::$indexes);\n+\n+        // Use `?before[column]=time` to get records before given value.\n+        $befores = normalize('dates', $request->query('before'));\n+        $query = $this->filter($query, $befores, [User::CREATED_AT, User::UPDATED_AT], '<');\n+\n+        // Use `?after[column]=time` to get records after given value.\n+        $afters = normalize('dates', $request->query('after'));\n+        $query = $this->filter($query, $afters, [User::CREATED_AT, User::UPDATED_AT], '>');\n+\n+        // Use `?search[column]=value` or `?search=key` to find users matching one or more criteria.\n+        $searches = $request->query('search');\n+\n+        $query = $this->search($query, normalize('credentials', $searches), User::$uniqueIndexes);\n+\n+        return $this->paginatedCollection($query, $request);\n+    }\n+\n+    /**\n+     * Store a newly created resource in storage.\n+     * POST /users\n+     *\n+     * @param Request $request\n+     * @return \\Illuminate\\Http\\Response\n+     * @throws NorthstarValidationException\n+     */\n+    public function store(Request $request)\n+    {\n+        $existingUser = $this->registrar->resolve($request->only('id', 'email', 'mobile', 'drupal_id', 'facebook_id'));\n+\n+        // If `?upsert=true` and a record already exists, update a user with the $request fields.\n+        if ($request->query('upsert') && $existingUser) {","path":"app/Http/Controllers/Two/UserController.php","position":90,"original_position":90,"commit_id":"419a71bdb0d1210ed6e4dd3dd05090c73e917d9c","original_commit_id":"419a71bdb0d1210ed6e4dd3dd05090c73e917d9c","user":{"login":"DFurnes","id":583202,"avatar_url":"https://avatars1.githubusercontent.com/u/583202?v=4","gravatar_id":"","url":"https://api.github.com/users/DFurnes","html_url":"https://github.com/DFurnes","followers_url":"https://api.github.com/users/DFurnes/followers","following_url":"https://api.github.com/users/DFurnes/following{/other_user}","gists_url":"https://api.github.com/users/DFurnes/gists{/gist_id}","starred_url":"https://api.github.com/users/DFurnes/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/DFurnes/subscriptions","organizations_url":"https://api.github.com/users/DFurnes/orgs","repos_url":"https://api.github.com/users/DFurnes/repos","events_url":"https://api.github.com/users/DFurnes/events{/privacy}","received_events_url":"https://api.github.com/users/DFurnes/received_events","type":"User","site_admin":false},"body":"I think there are some unintended behavior changes here. For example, there's no `$this->registrar->validate` call in the second part of the conditional that runs when `?upsert=false`.\r\n\r\nWe can re-arrange and combine these two conditionals to be a bit less confusing by using a guard. Once we check for the case where `?upsert=true` and there's an existing user, we can continue on the same for both.\r\n\r\n```php\r\n$existingUser = ...;\r\n\r\nif ($existingUser && ! $request->query('upsert')) {\r\n    throw new NorthstarValidationException(['id' => ['A record matching one of the given indexes already exists.']], $existingUser);\r\n}\r\n\r\n// Normalize input and validate the request\r\n$request = normalize('credentials', $request);\r\n$this->registrar->validate($request, $existingUser);\r\n// and so on...\r\n```","created_at":"2018-03-02T18:28:33Z","updated_at":"2018-03-02T18:35:41Z","html_url":"https://github.com/DoSomething/northstar/pull/715#discussion_r171925229","pull_request_url":"https://api.github.com/repos/DoSomething/northstar/pulls/715","author_association":"OWNER","_links":{"self":{"href":"https://api.github.com/repos/DoSomething/northstar/pulls/comments/171925229"},"html":{"href":"https://github.com/DoSomething/northstar/pull/715#discussion_r171925229"},"pull_request":{"href":"https://api.github.com/repos/DoSomething/northstar/pulls/715"}}},"pull_request":{"url":"https://api.github.com/repos/DoSomething/northstar/pulls/715","id":172353130,"html_url":"https://github.com/DoSomething/northstar/pull/715","diff_url":"https://github.com/DoSomething/northstar/pull/715.diff","patch_url":"https://github.com/DoSomething/northstar/pull/715.patch","issue_url":"https://api.github.com/repos/DoSomething/northstar/issues/715","number":715,"state":"open","locked":false,"title":"adds v2/user endpoints","user":{"login":"chloealee","id":9019452,"avatar_url":"https://avatars2.githubusercontent.com/u/9019452?v=4","gravatar_id":"","url":"https://api.github.com/users/chloealee","html_url":"https://github.com/chloealee","followers_url":"https://api.github.com/users/chloealee/followers","following_url":"https://api.github.com/users/chloealee/following{/other_user}","gists_url":"https://api.github.com/users/chloealee/gists{/gist_id}","starred_url":"https://api.github.com/users/chloealee/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chloealee/subscriptions","organizations_url":"https://api.github.com/users/chloealee/orgs","repos_url":"https://api.github.com/users/chloealee/repos","events_url":"https://api.github.com/users/chloealee/events{/privacy}","received_events_url":"https://api.github.com/users/chloealee/received_events","type":"User","site_admin":false},"body":"#### What's this PR do?\r\n- Adds `v2/users` [resource controller & routes](https://laravel.com/docs/5.5/controllers#resource-controllers), using existing v1 controller and transformers as a template.\r\n- Removes deprecated fields from the newly created v2 `UserTransformer` (like `_id`, `mobilecommons_id`, `mobilecommons_status`, `parse_installation_ids`, `drupal_id`, and `addr_source`)\r\n- The `POST /v2/users` method should _not_ upsert by default (but allow `?upsert=true` to enable that behavior). (https://www.pivotaltracker.com/story/show/148973253)\r\n- Moves existing tests in [UserTest](https://github.com/DoSomething/northstar/blob/master/tests/Http/UserTest.php) into [LegacyUserTest](https://github.com/DoSomething/northstar/blob/master/tests/LegacyHttp/UserTest.php), and creates a baseline test suite for the v2 endpoints in `UserTest`.\r\n- Updates documentation.\r\n\r\n#### How should this be reviewed?\r\n- All endpoints should work correctly and all tests should pass! \r\n- I'd love an extra hard look at the `POST /v2/users` method to make sure I got the logic correct there to not upsert by default! \r\n\r\n#### Relevant Tickets\r\nFixes [this](https://www.pivotaltracker.com/n/projects/2019429/stories/155201607) Pivotal Card.\r\n\r\n#### Checklist\r\n- [x] Documentation added for changed endpoints.\r\n- [x] Tests added for new features/bug fixes.\r\n- [ ] Post a message in #api if this includes something that causes a rebuild!  \r\n\r\n---\r\nFor review: …\r\n","created_at":"2018-03-01T21:04:08Z","updated_at":"2018-03-02T18:35:41Z","closed_at":null,"merged_at":null,"merge_commit_sha":"2308aa59a636ea6c1b4118ddc963f2902e8c444c","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/DoSomething/northstar/pulls/715/commits","review_comments_url":"https://api.github.com/repos/DoSomething/northstar/pulls/715/comments","review_comment_url":"https://api.github.com/repos/DoSomething/northstar/pulls/comments{/number}","comments_url":"https://api.github.com/repos/DoSomething/northstar/issues/715/comments","statuses_url":"https://api.github.com/repos/DoSomething/northstar/statuses/419a71bdb0d1210ed6e4dd3dd05090c73e917d9c","head":{"label":"DoSomething:v2-endpoints","ref":"v2-endpoints","sha":"419a71bdb0d1210ed6e4dd3dd05090c73e917d9c","user":{"login":"DoSomething","id":731685,"avatar_url":"https://avatars2.githubusercontent.com/u/731685?v=4","gravatar_id":"","url":"https://api.github.com/users/DoSomething","html_url":"https://github.com/DoSomething","followers_url":"https://api.github.com/users/DoSomething/followers","following_url":"https://api.github.com/users/DoSomething/following{/other_user}","gists_url":"https://api.github.com/users/DoSomething/gists{/gist_id}","starred_url":"https://api.github.com/users/DoSomething/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/DoSomething/subscriptions","organizations_url":"https://api.github.com/users/DoSomething/orgs","repos_url":"https://api.github.com/users/DoSomething/repos","events_url":"https://api.github.com/users/DoSomething/events{/privacy}","received_events_url":"https://api.github.com/users/DoSomething/received_events","type":"Organization","site_admin":false},"repo":{"id":26884886,"name":"northstar","full_name":"DoSomething/northstar","owner":{"login":"DoSomething","id":731685,"avatar_url":"https://avatars2.githubusercontent.com/u/731685?v=4","gravatar_id":"","url":"https://api.github.com/users/DoSomething","html_url":"https://github.com/DoSomething","followers_url":"https://api.github.com/users/DoSomething/followers","following_url":"https://api.github.com/users/DoSomething/following{/other_user}","gists_url":"https://api.github.com/users/DoSomething/gists{/gist_id}","starred_url":"https://api.github.com/users/DoSomething/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/DoSomething/subscriptions","organizations_url":"https://api.github.com/users/DoSomething/orgs","repos_url":"https://api.github.com/users/DoSomething/repos","events_url":"https://api.github.com/users/DoSomething/events{/privacy}","received_events_url":"https://api.github.com/users/DoSomething/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/DoSomething/northstar","description":":sparkles: The DoSomething.org user & identity service.","fork":false,"url":"https://api.github.com/repos/DoSomething/northstar","forks_url":"https://api.github.com/repos/DoSomething/northstar/forks","keys_url":"https://api.github.com/repos/DoSomething/northstar/keys{/key_id}","collaborators_url":"https://api.github.com/repos/DoSomething/northstar/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/DoSomething/northstar/teams","hooks_url":"https://api.github.com/repos/DoSomething/northstar/hooks","issue_events_url":"https://api.github.com/repos/DoSomething/northstar/issues/events{/number}","events_url":"https://api.github.com/repos/DoSomething/northstar/events","assignees_url":"https://api.github.com/repos/DoSomething/northstar/assignees{/user}","branches_url":"https://api.github.com/repos/DoSomething/northstar/branches{/branch}","tags_url":"https://api.github.com/repos/DoSomething/northstar/tags","blobs_url":"https://api.github.com/repos/DoSomething/northstar/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/DoSomething/northstar/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/DoSomething/northstar/git/refs{/sha}","trees_url":"https://api.github.com/repos/DoSomething/northstar/git/trees{/sha}","statuses_url":"https://api.github.com/repos/DoSomething/northstar/statuses/{sha}","languages_url":"https://api.github.com/repos/DoSomething/northstar/languages","stargazers_url":"https://api.github.com/repos/DoSomething/northstar/stargazers","contributors_url":"https://api.github.com/repos/DoSomething/northstar/contributors","subscribers_url":"https://api.github.com/repos/DoSomething/northstar/subscribers","subscription_url":"https://api.github.com/repos/DoSomething/northstar/subscription","commits_url":"https://api.github.com/repos/DoSomething/northstar/commits{/sha}","git_commits_url":"https://api.github.com/repos/DoSomething/northstar/git/commits{/sha}","comments_url":"https://api.github.com/repos/DoSomething/northstar/comments{/number}","issue_comment_url":"https://api.github.com/repos/DoSomething/northstar/issues/comments{/number}","contents_url":"https://api.github.com/repos/DoSomething/northstar/contents/{+path}","compare_url":"https://api.github.com/repos/DoSomething/northstar/compare/{base}...{head}","merges_url":"https://api.github.com/repos/DoSomething/northstar/merges","archive_url":"https://api.github.com/repos/DoSomething/northstar/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/DoSomething/northstar/downloads","issues_url":"https://api.github.com/repos/DoSomething/northstar/issues{/number}","pulls_url":"https://api.github.com/repos/DoSomething/northstar/pulls{/number}","milestones_url":"https://api.github.com/repos/DoSomething/northstar/milestones{/number}","notifications_url":"https://api.github.com/repos/DoSomething/northstar/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/DoSomething/northstar/labels{/name}","releases_url":"https://api.github.com/repos/DoSomething/northstar/releases{/id}","deployments_url":"https://api.github.com/repos/DoSomething/northstar/deployments","created_at":"2014-11-19T22:33:42Z","updated_at":"2017-10-30T18:26:28Z","pushed_at":"2018-03-02T15:32:29Z","git_url":"git://github.com/DoSomething/northstar.git","ssh_url":"git@github.com:DoSomething/northstar.git","clone_url":"https://github.com/DoSomething/northstar.git","svn_url":"https://github.com/DoSomething/northstar","homepage":"https://profile.dosomething.org","size":2641,"stargazers_count":4,"watchers_count":4,"language":"PHP","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":11,"mirror_url":null,"archived":false,"open_issues_count":12,"license":{"key":"mit","name":"MIT License","spdx_id":"MIT","url":"https://api.github.com/licenses/mit"},"forks":11,"open_issues":12,"watchers":4,"default_branch":"master"}},"base":{"label":"DoSomething:master","ref":"master","sha":"2e3b2a0942162005eb06adf77d72c576d49c9fd7","user":{"login":"DoSomething","id":731685,"avatar_url":"https://avatars2.githubusercontent.com/u/731685?v=4","gravatar_id":"","url":"https://api.github.com/users/DoSomething","html_url":"https://github.com/DoSomething","followers_url":"https://api.github.com/users/DoSomething/followers","following_url":"https://api.github.com/users/DoSomething/following{/other_user}","gists_url":"https://api.github.com/users/DoSomething/gists{/gist_id}","starred_url":"https://api.github.com/users/DoSomething/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/DoSomething/subscriptions","organizations_url":"https://api.github.com/users/DoSomething/orgs","repos_url":"https://api.github.com/users/DoSomething/repos","events_url":"https://api.github.com/users/DoSomething/events{/privacy}","received_events_url":"https://api.github.com/users/DoSomething/received_events","type":"Organization","site_admin":false},"repo":{"id":26884886,"name":"northstar","full_name":"DoSomething/northstar","owner":{"login":"DoSomething","id":731685,"avatar_url":"https://avatars2.githubusercontent.com/u/731685?v=4","gravatar_id":"","url":"https://api.github.com/users/DoSomething","html_url":"https://github.com/DoSomething","followers_url":"https://api.github.com/users/DoSomething/followers","following_url":"https://api.github.com/users/DoSomething/following{/other_user}","gists_url":"https://api.github.com/users/DoSomething/gists{/gist_id}","starred_url":"https://api.github.com/users/DoSomething/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/DoSomething/subscriptions","organizations_url":"https://api.github.com/users/DoSomething/orgs","repos_url":"https://api.github.com/users/DoSomething/repos","events_url":"https://api.github.com/users/DoSomething/events{/privacy}","received_events_url":"https://api.github.com/users/DoSomething/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/DoSomething/northstar","description":":sparkles: The DoSomething.org user & identity service.","fork":false,"url":"https://api.github.com/repos/DoSomething/northstar","forks_url":"https://api.github.com/repos/DoSomething/northstar/forks","keys_url":"https://api.github.com/repos/DoSomething/northstar/keys{/key_id}","collaborators_url":"https://api.github.com/repos/DoSomething/northstar/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/DoSomething/northstar/teams","hooks_url":"https://api.github.com/repos/DoSomething/northstar/hooks","issue_events_url":"https://api.github.com/repos/DoSomething/northstar/issues/events{/number}","events_url":"https://api.github.com/repos/DoSomething/northstar/events","assignees_url":"https://api.github.com/repos/DoSomething/northstar/assignees{/user}","branches_url":"https://api.github.com/repos/DoSomething/northstar/branches{/branch}","tags_url":"https://api.github.com/repos/DoSomething/northstar/tags","blobs_url":"https://api.github.com/repos/DoSomething/northstar/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/DoSomething/northstar/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/DoSomething/northstar/git/refs{/sha}","trees_url":"https://api.github.com/repos/DoSomething/northstar/git/trees{/sha}","statuses_url":"https://api.github.com/repos/DoSomething/northstar/statuses/{sha}","languages_url":"https://api.github.com/repos/DoSomething/northstar/languages","stargazers_url":"https://api.github.com/repos/DoSomething/northstar/stargazers","contributors_url":"https://api.github.com/repos/DoSomething/northstar/contributors","subscribers_url":"https://api.github.com/repos/DoSomething/northstar/subscribers","subscription_url":"https://api.github.com/repos/DoSomething/northstar/subscription","commits_url":"https://api.github.com/repos/DoSomething/northstar/commits{/sha}","git_commits_url":"https://api.github.com/repos/DoSomething/northstar/git/commits{/sha}","comments_url":"https://api.github.com/repos/DoSomething/northstar/comments{/number}","issue_comment_url":"https://api.github.com/repos/DoSomething/northstar/issues/comments{/number}","contents_url":"https://api.github.com/repos/DoSomething/northstar/contents/{+path}","compare_url":"https://api.github.com/repos/DoSomething/northstar/compare/{base}...{head}","merges_url":"https://api.github.com/repos/DoSomething/northstar/merges","archive_url":"https://api.github.com/repos/DoSomething/northstar/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/DoSomething/northstar/downloads","issues_url":"https://api.github.com/repos/DoSomething/northstar/issues{/number}","pulls_url":"https://api.github.com/repos/DoSomething/northstar/pulls{/number}","milestones_url":"https://api.github.com/repos/DoSomething/northstar/milestones{/number}","notifications_url":"https://api.github.com/repos/DoSomething/northstar/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/DoSomething/northstar/labels{/name}","releases_url":"https://api.github.com/repos/DoSomething/northstar/releases{/id}","deployments_url":"https://api.github.com/repos/DoSomething/northstar/deployments","created_at":"2014-11-19T22:33:42Z","updated_at":"2017-10-30T18:26:28Z","pushed_at":"2018-03-02T15:32:29Z","git_url":"git://github.com/DoSomething/northstar.git","ssh_url":"git@github.com:DoSomething/northstar.git","clone_url":"https://github.com/DoSomething/northstar.git","svn_url":"https://github.com/DoSomething/northstar","homepage":"https://profile.dosomething.org","size":2641,"stargazers_count":4,"watchers_count":4,"language":"PHP","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":11,"mirror_url":null,"archived":false,"open_issues_count":12,"license":{"key":"mit","name":"MIT License","spdx_id":"MIT","url":"https://api.github.com/licenses/mit"},"forks":11,"open_issues":12,"watchers":4,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/DoSomething/northstar/pulls/715"},"html":{"href":"https://github.com/DoSomething/northstar/pull/715"},"issue":{"href":"https://api.github.com/repos/DoSomething/northstar/issues/715"},"comments":{"href":"https://api.github.com/repos/DoSomething/northstar/issues/715/comments"},"review_comments":{"href":"https://api.github.com/repos/DoSomething/northstar/pulls/715/comments"},"review_comment":{"href":"https://api.github.com/repos/DoSomething/northstar/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/DoSomething/northstar/pulls/715/commits"},"statuses":{"href":"https://api.github.com/repos/DoSomething/northstar/statuses/419a71bdb0d1210ed6e4dd3dd05090c73e917d9c"}},"author_association":"CONTRIBUTOR"}}
{"actor":{"display_login":"DFurnes"}}