{"action":"created","comment":{"url":"https://api.github.com/repos/hypothesis/h/pulls/comments/171283169","pull_request_review_id":100108257,"id":171283169,"diff_hunk":"@@ -187,9 +184,16 @@ def update_annotation(session, id_, data):\n     # annotation object.\n     document = data.pop('document', None)\n \n-    annotation = session.query(models.Annotation).get(id_)\n+    annotation = request.db.query(models.Annotation).get(id_)\n     annotation.updated = updated\n \n+    group = group_service.find(annotation.groupid)\n+    if group is None:\n+        raise schemas.ValidationError('group: ' +\n+                                      _('Invalid group specified for annotation'))\n+    if request.feature('filter_groups_by_scope') and data.get('target_uri', None):","path":"h/storage.py","position":58,"original_position":58,"commit_id":"94338b34cd953d2b26c6c94dc654dba469008734","original_commit_id":"94338b34cd953d2b26c6c94dc654dba469008734","user":{"login":"lyzadanger","id":439947,"avatar_url":"https://avatars2.githubusercontent.com/u/439947?v=4","gravatar_id":"","url":"https://api.github.com/users/lyzadanger","html_url":"https://github.com/lyzadanger","followers_url":"https://api.github.com/users/lyzadanger/followers","following_url":"https://api.github.com/users/lyzadanger/following{/other_user}","gists_url":"https://api.github.com/users/lyzadanger/gists{/gist_id}","starred_url":"https://api.github.com/users/lyzadanger/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lyzadanger/subscriptions","organizations_url":"https://api.github.com/users/lyzadanger/orgs","repos_url":"https://api.github.com/users/lyzadanger/repos","events_url":"https://api.github.com/users/lyzadanger/events{/privacy}","received_events_url":"https://api.github.com/users/lyzadanger/received_events","type":"User","site_admin":false},"body":"@robertknight \r\n\r\n> I think the main time this would be an issue is when adding or removing scopes from the group, which is a thing we have not currently implemented.\r\n\r\nIs this worth tracking as a separate issue (data integrity for annotations as group scopes are added, changed or deleted)?","created_at":"2018-02-28T15:32:41Z","updated_at":"2018-02-28T15:32:41Z","html_url":"https://github.com/hypothesis/h/pull/4835#discussion_r171283169","pull_request_url":"https://api.github.com/repos/hypothesis/h/pulls/4835","author_association":"CONTRIBUTOR","_links":{"self":{"href":"https://api.github.com/repos/hypothesis/h/pulls/comments/171283169"},"html":{"href":"https://github.com/hypothesis/h/pull/4835#discussion_r171283169"},"pull_request":{"href":"https://api.github.com/repos/hypothesis/h/pulls/4835"}},"in_reply_to_id":171044284},"pull_request":{"url":"https://api.github.com/repos/hypothesis/h/pulls/4835","id":171764767,"html_url":"https://github.com/hypothesis/h/pull/4835","diff_url":"https://github.com/hypothesis/h/pull/4835.diff","patch_url":"https://github.com/hypothesis/h/pull/4835.patch","issue_url":"https://api.github.com/repos/hypothesis/h/issues/4835","number":4835,"state":"open","locked":false,"title":"Enforce group scope on `PATCH /api/annotations`","user":{"login":"lyzadanger","id":439947,"avatar_url":"https://avatars2.githubusercontent.com/u/439947?v=4","gravatar_id":"","url":"https://api.github.com/users/lyzadanger","html_url":"https://github.com/lyzadanger","followers_url":"https://api.github.com/users/lyzadanger/followers","following_url":"https://api.github.com/users/lyzadanger/following{/other_user}","gists_url":"https://api.github.com/users/lyzadanger/gists{/gist_id}","starred_url":"https://api.github.com/users/lyzadanger/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lyzadanger/subscriptions","organizations_url":"https://api.github.com/users/lyzadanger/orgs","repos_url":"https://api.github.com/users/lyzadanger/repos","events_url":"https://api.github.com/users/lyzadanger/events{/privacy}","received_events_url":"https://api.github.com/users/lyzadanger/received_events","type":"User","site_admin":false},"body":"This PR addresses the remainder of https://github.com/hypothesis/product-backlog/issues/483 by enforcing URI-level group scope on `PATCH` requests to `/api/annotations` (`h.storage.update_annotation`).\r\n\r\nIt required a bit of refactoring of the `update_annotation` method to get at a few more pieces of `request` that were needed, as well as the `group_service` for group lookup.\r\n\r\nThe service will raise (i.e. return a 400) if all of the following:\r\n\r\n1. A request is made to `PATCH /api/annotations` containing a `target_uri` value (not all patches contain all fields)\r\n2. The `filter_groups_by_scope` feature flag is ON\r\n3. The annotation's group has 1 or more `scopes`\r\n4. The new `target_uri` does not match any of the group's scopes","created_at":"2018-02-27T19:43:31Z","updated_at":"2018-02-28T15:32:41Z","closed_at":null,"merged_at":null,"merge_commit_sha":"e5f2ea52f9284ed5bf4651e98cfad273b8d88339","assignee":{"login":"robertknight","id":2458,"avatar_url":"https://avatars1.githubusercontent.com/u/2458?v=4","gravatar_id":"","url":"https://api.github.com/users/robertknight","html_url":"https://github.com/robertknight","followers_url":"https://api.github.com/users/robertknight/followers","following_url":"https://api.github.com/users/robertknight/following{/other_user}","gists_url":"https://api.github.com/users/robertknight/gists{/gist_id}","starred_url":"https://api.github.com/users/robertknight/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/robertknight/subscriptions","organizations_url":"https://api.github.com/users/robertknight/orgs","repos_url":"https://api.github.com/users/robertknight/repos","events_url":"https://api.github.com/users/robertknight/events{/privacy}","received_events_url":"https://api.github.com/users/robertknight/received_events","type":"User","site_admin":false},"assignees":[{"login":"robertknight","id":2458,"avatar_url":"https://avatars1.githubusercontent.com/u/2458?v=4","gravatar_id":"","url":"https://api.github.com/users/robertknight","html_url":"https://github.com/robertknight","followers_url":"https://api.github.com/users/robertknight/followers","following_url":"https://api.github.com/users/robertknight/following{/other_user}","gists_url":"https://api.github.com/users/robertknight/gists{/gist_id}","starred_url":"https://api.github.com/users/robertknight/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/robertknight/subscriptions","organizations_url":"https://api.github.com/users/robertknight/orgs","repos_url":"https://api.github.com/users/robertknight/repos","events_url":"https://api.github.com/users/robertknight/events{/privacy}","received_events_url":"https://api.github.com/users/robertknight/received_events","type":"User","site_admin":false}],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/hypothesis/h/pulls/4835/commits","review_comments_url":"https://api.github.com/repos/hypothesis/h/pulls/4835/comments","review_comment_url":"https://api.github.com/repos/hypothesis/h/pulls/comments{/number}","comments_url":"https://api.github.com/repos/hypothesis/h/issues/4835/comments","statuses_url":"https://api.github.com/repos/hypothesis/h/statuses/94338b34cd953d2b26c6c94dc654dba469008734","head":{"label":"hypothesis:enforce-scope-on-patch","ref":"enforce-scope-on-patch","sha":"94338b34cd953d2b26c6c94dc654dba469008734","user":{"login":"hypothesis","id":1243215,"avatar_url":"https://avatars1.githubusercontent.com/u/1243215?v=4","gravatar_id":"","url":"https://api.github.com/users/hypothesis","html_url":"https://github.com/hypothesis","followers_url":"https://api.github.com/users/hypothesis/followers","following_url":"https://api.github.com/users/hypothesis/following{/other_user}","gists_url":"https://api.github.com/users/hypothesis/gists{/gist_id}","starred_url":"https://api.github.com/users/hypothesis/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hypothesis/subscriptions","organizations_url":"https://api.github.com/users/hypothesis/orgs","repos_url":"https://api.github.com/users/hypothesis/repos","events_url":"https://api.github.com/users/hypothesis/events{/privacy}","received_events_url":"https://api.github.com/users/hypothesis/received_events","type":"Organization","site_admin":false},"repo":{"id":3910945,"name":"h","full_name":"hypothesis/h","owner":{"login":"hypothesis","id":1243215,"avatar_url":"https://avatars1.githubusercontent.com/u/1243215?v=4","gravatar_id":"","url":"https://api.github.com/users/hypothesis","html_url":"https://github.com/hypothesis","followers_url":"https://api.github.com/users/hypothesis/followers","following_url":"https://api.github.com/users/hypothesis/following{/other_user}","gists_url":"https://api.github.com/users/hypothesis/gists{/gist_id}","starred_url":"https://api.github.com/users/hypothesis/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hypothesis/subscriptions","organizations_url":"https://api.github.com/users/hypothesis/orgs","repos_url":"https://api.github.com/users/hypothesis/repos","events_url":"https://api.github.com/users/hypothesis/events{/privacy}","received_events_url":"https://api.github.com/users/hypothesis/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/hypothesis/h","description":"Annotate with anyone, anywhere.","fork":false,"url":"https://api.github.com/repos/hypothesis/h","forks_url":"https://api.github.com/repos/hypothesis/h/forks","keys_url":"https://api.github.com/repos/hypothesis/h/keys{/key_id}","collaborators_url":"https://api.github.com/repos/hypothesis/h/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/hypothesis/h/teams","hooks_url":"https://api.github.com/repos/hypothesis/h/hooks","issue_events_url":"https://api.github.com/repos/hypothesis/h/issues/events{/number}","events_url":"https://api.github.com/repos/hypothesis/h/events","assignees_url":"https://api.github.com/repos/hypothesis/h/assignees{/user}","branches_url":"https://api.github.com/repos/hypothesis/h/branches{/branch}","tags_url":"https://api.github.com/repos/hypothesis/h/tags","blobs_url":"https://api.github.com/repos/hypothesis/h/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/hypothesis/h/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/hypothesis/h/git/refs{/sha}","trees_url":"https://api.github.com/repos/hypothesis/h/git/trees{/sha}","statuses_url":"https://api.github.com/repos/hypothesis/h/statuses/{sha}","languages_url":"https://api.github.com/repos/hypothesis/h/languages","stargazers_url":"https://api.github.com/repos/hypothesis/h/stargazers","contributors_url":"https://api.github.com/repos/hypothesis/h/contributors","subscribers_url":"https://api.github.com/repos/hypothesis/h/subscribers","subscription_url":"https://api.github.com/repos/hypothesis/h/subscription","commits_url":"https://api.github.com/repos/hypothesis/h/commits{/sha}","git_commits_url":"https://api.github.com/repos/hypothesis/h/git/commits{/sha}","comments_url":"https://api.github.com/repos/hypothesis/h/comments{/number}","issue_comment_url":"https://api.github.com/repos/hypothesis/h/issues/comments{/number}","contents_url":"https://api.github.com/repos/hypothesis/h/contents/{+path}","compare_url":"https://api.github.com/repos/hypothesis/h/compare/{base}...{head}","merges_url":"https://api.github.com/repos/hypothesis/h/merges","archive_url":"https://api.github.com/repos/hypothesis/h/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/hypothesis/h/downloads","issues_url":"https://api.github.com/repos/hypothesis/h/issues{/number}","pulls_url":"https://api.github.com/repos/hypothesis/h/pulls{/number}","milestones_url":"https://api.github.com/repos/hypothesis/h/milestones{/number}","notifications_url":"https://api.github.com/repos/hypothesis/h/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/hypothesis/h/labels{/name}","releases_url":"https://api.github.com/repos/hypothesis/h/releases{/id}","deployments_url":"https://api.github.com/repos/hypothesis/h/deployments","created_at":"2012-04-02T19:56:59Z","updated_at":"2018-02-27T02:24:27Z","pushed_at":"2018-02-28T15:26:15Z","git_url":"git://github.com/hypothesis/h.git","ssh_url":"git@github.com:hypothesis/h.git","clone_url":"https://github.com/hypothesis/h.git","svn_url":"https://github.com/hypothesis/h","homepage":"https://hypothes.is/","size":28716,"stargazers_count":1036,"watchers_count":1036,"language":"Python","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":243,"mirror_url":null,"archived":false,"open_issues_count":69,"license":{"key":"other","name":"Other","spdx_id":null,"url":null},"forks":243,"open_issues":69,"watchers":1036,"default_branch":"master"}},"base":{"label":"hypothesis:master","ref":"master","sha":"d99e518760fe9d956f8ef381810ccd90a4b7cf15","user":{"login":"hypothesis","id":1243215,"avatar_url":"https://avatars1.githubusercontent.com/u/1243215?v=4","gravatar_id":"","url":"https://api.github.com/users/hypothesis","html_url":"https://github.com/hypothesis","followers_url":"https://api.github.com/users/hypothesis/followers","following_url":"https://api.github.com/users/hypothesis/following{/other_user}","gists_url":"https://api.github.com/users/hypothesis/gists{/gist_id}","starred_url":"https://api.github.com/users/hypothesis/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hypothesis/subscriptions","organizations_url":"https://api.github.com/users/hypothesis/orgs","repos_url":"https://api.github.com/users/hypothesis/repos","events_url":"https://api.github.com/users/hypothesis/events{/privacy}","received_events_url":"https://api.github.com/users/hypothesis/received_events","type":"Organization","site_admin":false},"repo":{"id":3910945,"name":"h","full_name":"hypothesis/h","owner":{"login":"hypothesis","id":1243215,"avatar_url":"https://avatars1.githubusercontent.com/u/1243215?v=4","gravatar_id":"","url":"https://api.github.com/users/hypothesis","html_url":"https://github.com/hypothesis","followers_url":"https://api.github.com/users/hypothesis/followers","following_url":"https://api.github.com/users/hypothesis/following{/other_user}","gists_url":"https://api.github.com/users/hypothesis/gists{/gist_id}","starred_url":"https://api.github.com/users/hypothesis/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hypothesis/subscriptions","organizations_url":"https://api.github.com/users/hypothesis/orgs","repos_url":"https://api.github.com/users/hypothesis/repos","events_url":"https://api.github.com/users/hypothesis/events{/privacy}","received_events_url":"https://api.github.com/users/hypothesis/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/hypothesis/h","description":"Annotate with anyone, anywhere.","fork":false,"url":"https://api.github.com/repos/hypothesis/h","forks_url":"https://api.github.com/repos/hypothesis/h/forks","keys_url":"https://api.github.com/repos/hypothesis/h/keys{/key_id}","collaborators_url":"https://api.github.com/repos/hypothesis/h/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/hypothesis/h/teams","hooks_url":"https://api.github.com/repos/hypothesis/h/hooks","issue_events_url":"https://api.github.com/repos/hypothesis/h/issues/events{/number}","events_url":"https://api.github.com/repos/hypothesis/h/events","assignees_url":"https://api.github.com/repos/hypothesis/h/assignees{/user}","branches_url":"https://api.github.com/repos/hypothesis/h/branches{/branch}","tags_url":"https://api.github.com/repos/hypothesis/h/tags","blobs_url":"https://api.github.com/repos/hypothesis/h/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/hypothesis/h/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/hypothesis/h/git/refs{/sha}","trees_url":"https://api.github.com/repos/hypothesis/h/git/trees{/sha}","statuses_url":"https://api.github.com/repos/hypothesis/h/statuses/{sha}","languages_url":"https://api.github.com/repos/hypothesis/h/languages","stargazers_url":"https://api.github.com/repos/hypothesis/h/stargazers","contributors_url":"https://api.github.com/repos/hypothesis/h/contributors","subscribers_url":"https://api.github.com/repos/hypothesis/h/subscribers","subscription_url":"https://api.github.com/repos/hypothesis/h/subscription","commits_url":"https://api.github.com/repos/hypothesis/h/commits{/sha}","git_commits_url":"https://api.github.com/repos/hypothesis/h/git/commits{/sha}","comments_url":"https://api.github.com/repos/hypothesis/h/comments{/number}","issue_comment_url":"https://api.github.com/repos/hypothesis/h/issues/comments{/number}","contents_url":"https://api.github.com/repos/hypothesis/h/contents/{+path}","compare_url":"https://api.github.com/repos/hypothesis/h/compare/{base}...{head}","merges_url":"https://api.github.com/repos/hypothesis/h/merges","archive_url":"https://api.github.com/repos/hypothesis/h/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/hypothesis/h/downloads","issues_url":"https://api.github.com/repos/hypothesis/h/issues{/number}","pulls_url":"https://api.github.com/repos/hypothesis/h/pulls{/number}","milestones_url":"https://api.github.com/repos/hypothesis/h/milestones{/number}","notifications_url":"https://api.github.com/repos/hypothesis/h/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/hypothesis/h/labels{/name}","releases_url":"https://api.github.com/repos/hypothesis/h/releases{/id}","deployments_url":"https://api.github.com/repos/hypothesis/h/deployments","created_at":"2012-04-02T19:56:59Z","updated_at":"2018-02-27T02:24:27Z","pushed_at":"2018-02-28T15:26:15Z","git_url":"git://github.com/hypothesis/h.git","ssh_url":"git@github.com:hypothesis/h.git","clone_url":"https://github.com/hypothesis/h.git","svn_url":"https://github.com/hypothesis/h","homepage":"https://hypothes.is/","size":28716,"stargazers_count":1036,"watchers_count":1036,"language":"Python","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":243,"mirror_url":null,"archived":false,"open_issues_count":69,"license":{"key":"other","name":"Other","spdx_id":null,"url":null},"forks":243,"open_issues":69,"watchers":1036,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/hypothesis/h/pulls/4835"},"html":{"href":"https://github.com/hypothesis/h/pull/4835"},"issue":{"href":"https://api.github.com/repos/hypothesis/h/issues/4835"},"comments":{"href":"https://api.github.com/repos/hypothesis/h/issues/4835/comments"},"review_comments":{"href":"https://api.github.com/repos/hypothesis/h/pulls/4835/comments"},"review_comment":{"href":"https://api.github.com/repos/hypothesis/h/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/hypothesis/h/pulls/4835/commits"},"statuses":{"href":"https://api.github.com/repos/hypothesis/h/statuses/94338b34cd953d2b26c6c94dc654dba469008734"}},"author_association":"CONTRIBUTOR"}}
{"actor":{"display_login":"lyzadanger"}}