{"action":"created","issue":{"url":"https://api.github.com/repos/civicrm/civicrm-core/issues/12995","repository_url":"https://api.github.com/repos/civicrm/civicrm-core","labels_url":"https://api.github.com/repos/civicrm/civicrm-core/issues/12995/labels{/name}","comments_url":"https://api.github.com/repos/civicrm/civicrm-core/issues/12995/comments","events_url":"https://api.github.com/repos/civicrm/civicrm-core/issues/12995/events","html_url":"https://github.com/civicrm/civicrm-core/pull/12995","id":373262952,"node_id":"MDExOlB1bGxSZXF1ZXN0MjI1MjMzODQx","number":12995,"title":"dev/core/454 rationalise activity case permissions & code","user":{"login":"eileenmcnaughton","id":336308,"node_id":"MDQ6VXNlcjMzNjMwOA==","avatar_url":"https://avatars2.githubusercontent.com/u/336308?v=4","gravatar_id":"","url":"https://api.github.com/users/eileenmcnaughton","html_url":"https://github.com/eileenmcnaughton","followers_url":"https://api.github.com/users/eileenmcnaughton/followers","following_url":"https://api.github.com/users/eileenmcnaughton/following{/other_user}","gists_url":"https://api.github.com/users/eileenmcnaughton/gists{/gist_id}","starred_url":"https://api.github.com/users/eileenmcnaughton/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/eileenmcnaughton/subscriptions","organizations_url":"https://api.github.com/users/eileenmcnaughton/orgs","repos_url":"https://api.github.com/users/eileenmcnaughton/repos","events_url":"https://api.github.com/users/eileenmcnaughton/events{/privacy}","received_events_url":"https://api.github.com/users/eileenmcnaughton/received_events","type":"User","site_admin":false},"labels":[{"id":246037889,"node_id":"MDU6TGFiZWwyNDYwMzc4ODk=","url":"https://api.github.com/repos/civicrm/civicrm-core/labels/master","name":"master","color":"0052cc","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":4,"created_at":"2018-10-24T01:03:28Z","updated_at":"2018-12-13T14:19:33Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/civicrm/civicrm-core/pulls/12995","html_url":"https://github.com/civicrm/civicrm-core/pull/12995","diff_url":"https://github.com/civicrm/civicrm-core/pull/12995.diff","patch_url":"https://github.com/civicrm/civicrm-core/pull/12995.patch"},"body":"Overview\r\n----------------------------------------\r\nPer https://lab.civicrm.org/dev/core/issues/454 I have made access activities associated with cases consistent between the UI & various other places - in practice making 'access my cases and activities' OR 'access all cases and activities' sufficient to pass the CiviCase component check.\r\n\r\nBefore\r\n----------------------------------------\r\nVia api 'administer CiviCase' required for accessing case activities, on top of 'access my cases and activities' OR 'access all cases and activities'  as required in the UI\r\n\r\nAfter\r\n----------------------------------------\r\n 'access my cases and activities' OR 'access all cases and activities'  as required in the UI also required in CiviCase\r\n\r\nTechnical Details\r\n----------------------------------------\r\n\r\nThis is a refactor step - not the final destination. I want to move from loading\r\nall activities & then checking if each one can be accessed to determining a list\r\nof activity types and then adding that filter to the sql query. 2 reasons\r\n- performance\r\n- because getcount actually hard fails when permissions are applied currently :-(\r\n\r\nIn this step I note the function activityComponents looks like it expected\r\nthe concept of showActivitiesInCore by components to take off but in\r\npractice it is hard coded to 0 for CiviCase & CiviCase only. Oddly there is\r\nacually handling for CiviCase in this function in a place unreachable\r\nwithout the changes in this patch. In addition the few places that call this\r\nfunction also do weird CiviCase handling. I've opted for relatively low\r\nintervention since I think most places that call this function\r\nare likely to change themselves in the short-medium term.\r\n\r\n\r\nComments\r\n----------------------------------------\r\nI think this would be easier to review if https://github.com/civicrm/civicrm-core/pull/12994 were merged first (it is incorporated in this but I would rebase it out if it were merged)"},"comment":{"url":"https://api.github.com/repos/civicrm/civicrm-core/issues/comments/446984469","html_url":"https://github.com/civicrm/civicrm-core/pull/12995#issuecomment-446984469","issue_url":"https://api.github.com/repos/civicrm/civicrm-core/issues/12995","id":446984469,"node_id":"MDEyOklzc3VlQ29tbWVudDQ0Njk4NDQ2OQ==","user":{"login":"aydun","id":2730045,"node_id":"MDQ6VXNlcjI3MzAwNDU=","avatar_url":"https://avatars0.githubusercontent.com/u/2730045?v=4","gravatar_id":"","url":"https://api.github.com/users/aydun","html_url":"https://github.com/aydun","followers_url":"https://api.github.com/users/aydun/followers","following_url":"https://api.github.com/users/aydun/following{/other_user}","gists_url":"https://api.github.com/users/aydun/gists{/gist_id}","starred_url":"https://api.github.com/users/aydun/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/aydun/subscriptions","organizations_url":"https://api.github.com/users/aydun/orgs","repos_url":"https://api.github.com/users/aydun/repos","events_url":"https://api.github.com/users/aydun/events{/privacy}","received_events_url":"https://api.github.com/users/aydun/received_events","type":"User","site_admin":false},"created_at":"2018-12-13T14:19:33Z","updated_at":"2018-12-13T14:19:33Z","author_association":"CONTRIBUTOR","body":"@eileenmcnaughton As I read it, any one of the three permissions was sufficient to make `isContactPermittedAccessToCaseActivity()` return `TRUE`, and the change removes 'administer CiviCase' from that list.  \r\n\r\nI guess it comes down to what the 'administer' permission should provide: is it an overriding super-permission in the root tradition, or should it mean 'can configure the CiviCase component'? \r\n\r\nI'm not sufficiently up to speed with the nuances of other 'administer' permissions, but by itself this seems a reasonable change since anyone needing to access all cases and activities can be granted the, erm, 'access all cases and activities' permission."}}
{"actor":{"display_login":"aydun"}}