{"action":"created","comment":{"url":"https://api.github.com/repos/CartoDB/cartodb/pulls/comments/31926173","id":31926173,"diff_hunk":"@@ -297,54 +299,80 @@ def drop_organization_user(org_id, is_owner=false)\n \n     Thread.new do\n       in_database(as: :superuser) do |database|\n-        # Drop this user privileges in every schema of the DB\n-        schemas = ['cdb', 'cdb_importer', 'cartodb', 'public', self.database_schema] +\n-          User.select(:database_schema).where(:organization_id => org_id).all.collect(&:database_schema)\n-        schemas.uniq.each do |s|\n-          if is_owner\n+        if is_owner\n+          schemas = ['cdb', 'cdb_importer', 'cartodb', 'public', self.database_schema] +\n+              User.select(:database_schema).where(:organization_id => org_id).all.collect(&:database_schema)\n+          schemas.uniq.each do |s|\n             drop_user_privileges_in_schema(s, [CartoDB::PUBLIC_DB_USER])\n-          else\n-            drop_user_privileges_in_schema(s)\n           end\n         end\n \n-        # Only remove publicuser from current user schema (and if owner, already done above)\n-        unless is_owner\n-          drop_user_privileges_in_schema(self.database_schema, [CartoDB::PUBLIC_DB_USER])\n-        end\n-\n-        # Drop user quota function\n-        database.run(%Q{ DROP FUNCTION IF EXISTS \"#{self.database_schema}\"._CDB_UserQuotaInBytes()})\n         # If user is in an organization should never have public schema, so to be safe check\n-        drop_all_functions_from_schema(self.database_schema)\n         unless self.database_schema == 'public'\n           # Must drop all functions before or it will fail\n-          database.run(%Q{ DROP SCHEMA IF EXISTS \"#{self.database_schema}\" })\n+          database.run(%Q{ DROP SCHEMA IF EXISTS \"#{self.database_schema}\" CASCADE })","path":"app/models/user.rb","position":51,"original_position":51,"commit_id":"3540f274b8fa0eed34dde9b438500a28e841df9c","original_commit_id":"3540f274b8fa0eed34dde9b438500a28e841df9c","user":{"login":"Kartones","id":2085449,"avatar_url":"https://avatars.githubusercontent.com/u/2085449?v=3","gravatar_id":"","url":"https://api.github.com/users/Kartones","html_url":"https://github.com/Kartones","followers_url":"https://api.github.com/users/Kartones/followers","following_url":"https://api.github.com/users/Kartones/following{/other_user}","gists_url":"https://api.github.com/users/Kartones/gists{/gist_id}","starred_url":"https://api.github.com/users/Kartones/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Kartones/subscriptions","organizations_url":"https://api.github.com/users/Kartones/orgs","repos_url":"https://api.github.com/users/Kartones/repos","events_url":"https://api.github.com/users/Kartones/events{/privacy}","received_events_url":"https://api.github.com/users/Kartones/received_events","type":"User","site_admin":false},"body":"uhhh, no no. No cascade as we can screw up many things.\r\n\r\nMany of the additional logic of user deletion comes from having to manually simulate a cascade by dropping things, trying again, etc. If we need to check PG logs to see \"new objects to destroy\" after this permission changes lets do it, but I'm against a DROP CASCADE as we can wipe out critical stuff, shared functions, etc.\r\n\r\nEither that or @javisantana or @lbosque agree to this decision and we assume we can remove unexpected things, but in the past the restriction was there of never doing a CASCADE.","created_at":"2015-06-08T15:26:39Z","updated_at":"2015-06-08T15:26:39Z","html_url":"https://github.com/CartoDB/cartodb/pull/3881#discussion_r31926173","pull_request_url":"https://api.github.com/repos/CartoDB/cartodb/pulls/3881","_links":{"self":{"href":"https://api.github.com/repos/CartoDB/cartodb/pulls/comments/31926173"},"html":{"href":"https://github.com/CartoDB/cartodb/pull/3881#discussion_r31926173"},"pull_request":{"href":"https://api.github.com/repos/CartoDB/cartodb/pulls/3881"}}},"pull_request":{"url":"https://api.github.com/repos/CartoDB/cartodb/pulls/3881","id":36634905,"html_url":"https://github.com/CartoDB/cartodb/pull/3881","diff_url":"https://github.com/CartoDB/cartodb/pull/3881.diff","patch_url":"https://github.com/CartoDB/cartodb/pull/3881.patch","issue_url":"https://api.github.com/repos/CartoDB/cartodb/issues/3881","number":3881,"state":"open","locked":false,"title":"[NOMERGE] 3859 org permission based on roles","user":{"login":"juanignaciosl","id":932968,"avatar_url":"https://avatars.githubusercontent.com/u/932968?v=3","gravatar_id":"","url":"https://api.github.com/users/juanignaciosl","html_url":"https://github.com/juanignaciosl","followers_url":"https://api.github.com/users/juanignaciosl/followers","following_url":"https://api.github.com/users/juanignaciosl/following{/other_user}","gists_url":"https://api.github.com/users/juanignaciosl/gists{/gist_id}","starred_url":"https://api.github.com/users/juanignaciosl/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/juanignaciosl/subscriptions","organizations_url":"https://api.github.com/users/juanignaciosl/orgs","repos_url":"https://api.github.com/users/juanignaciosl/repos","events_url":"https://api.github.com/users/juanignaciosl/events{/privacy}","received_events_url":"https://api.github.com/users/juanignaciosl/received_events","type":"User","site_admin":false},"body":"@Kartones , @javisantana CR this, please. Marked as NOMERGE until agreement, after talking about implications on other projects. This fixes #3859.\r\n\r\n@iriberri could you pass QA to this, please? It's deployed on ded02.\r\n\r\n### Permission changes\r\n\r\nThis changes the global permission granting model of organizations, from user-centric to role-centric: previously users where granted/revoked themselves, now permission is granted to the role all organization users share. The key for understanding the changes is comparing the [new `setup_schema` method](https://github.com/CartoDB/cartodb/pull/3881/files#diff-4676c008b11a5480d73d4a6de01e45b9R1564) and [`set_user_privileges`](https://github.com/CartoDB/cartodb/pull/3881/files#diff-4676c008b11a5480d73d4a6de01e45b9R1712). For organization users now many \"set privileges\" methods are not called. That \"granting to users\" has changed to [\"granting to organization role\"](https://github.com/CartoDB/cartodb/pull/3881/files#diff-4676c008b11a5480d73d4a6de01e45b9R1573). [That role, `CDB_Organization_Member_Group_Role_Member_Name()` is assigned to users on user creation](https://github.com/CartoDB/cartodb-postgresql/blob/9114d4e463c8664c1fb31e3bc538ce96c0dd0771/scripts-available/CDB_Organizations.sql#L24), it's not \"new\", so this is functionally equivalent.\r\n\r\n### What to test\r\n\r\nAnything related to creation/deletion/sharing of organizations, users and datasets/maps. For example, there's no need at all to test every editor option, but you need to test whether organization and user management works fine, and if a user can/cannot view/edit something he should. Similarly, no need to test importing formats, only one importer is enough. SQL and Maps API should also be tested. Normal user management logic shouldn't have changed, but you should also test that this is true.","created_at":"2015-06-01T14:38:15Z","updated_at":"2015-06-08T15:26:40Z","closed_at":null,"merged_at":null,"merge_commit_sha":"1b9c0c012f5582e4ba060259a7780710eea3bc1f","assignee":{"login":"juanignaciosl","id":932968,"avatar_url":"https://avatars.githubusercontent.com/u/932968?v=3","gravatar_id":"","url":"https://api.github.com/users/juanignaciosl","html_url":"https://github.com/juanignaciosl","followers_url":"https://api.github.com/users/juanignaciosl/followers","following_url":"https://api.github.com/users/juanignaciosl/following{/other_user}","gists_url":"https://api.github.com/users/juanignaciosl/gists{/gist_id}","starred_url":"https://api.github.com/users/juanignaciosl/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/juanignaciosl/subscriptions","organizations_url":"https://api.github.com/users/juanignaciosl/orgs","repos_url":"https://api.github.com/users/juanignaciosl/repos","events_url":"https://api.github.com/users/juanignaciosl/events{/privacy}","received_events_url":"https://api.github.com/users/juanignaciosl/received_events","type":"User","site_admin":false},"milestone":null,"commits_url":"https://api.github.com/repos/CartoDB/cartodb/pulls/3881/commits","review_comments_url":"https://api.github.com/repos/CartoDB/cartodb/pulls/3881/comments","review_comment_url":"https://api.github.com/repos/CartoDB/cartodb/pulls/comments{/number}","comments_url":"https://api.github.com/repos/CartoDB/cartodb/issues/3881/comments","statuses_url":"https://api.github.com/repos/CartoDB/cartodb/statuses/3540f274b8fa0eed34dde9b438500a28e841df9c","head":{"label":"CartoDB:3859-org_permission_based_on_roles","ref":"3859-org_permission_based_on_roles","sha":"3540f274b8fa0eed34dde9b438500a28e841df9c","user":{"login":"CartoDB","id":1799254,"avatar_url":"https://avatars.githubusercontent.com/u/1799254?v=3","gravatar_id":"","url":"https://api.github.com/users/CartoDB","html_url":"https://github.com/CartoDB","followers_url":"https://api.github.com/users/CartoDB/followers","following_url":"https://api.github.com/users/CartoDB/following{/other_user}","gists_url":"https://api.github.com/users/CartoDB/gists{/gist_id}","starred_url":"https://api.github.com/users/CartoDB/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/CartoDB/subscriptions","organizations_url":"https://api.github.com/users/CartoDB/orgs","repos_url":"https://api.github.com/users/CartoDB/repos","events_url":"https://api.github.com/users/CartoDB/events{/privacy}","received_events_url":"https://api.github.com/users/CartoDB/received_events","type":"Organization","site_admin":false},"repo":{"id":4909355,"name":"cartodb","full_name":"CartoDB/cartodb","owner":{"login":"CartoDB","id":1799254,"avatar_url":"https://avatars.githubusercontent.com/u/1799254?v=3","gravatar_id":"","url":"https://api.github.com/users/CartoDB","html_url":"https://github.com/CartoDB","followers_url":"https://api.github.com/users/CartoDB/followers","following_url":"https://api.github.com/users/CartoDB/following{/other_user}","gists_url":"https://api.github.com/users/CartoDB/gists{/gist_id}","starred_url":"https://api.github.com/users/CartoDB/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/CartoDB/subscriptions","organizations_url":"https://api.github.com/users/CartoDB/orgs","repos_url":"https://api.github.com/users/CartoDB/repos","events_url":"https://api.github.com/users/CartoDB/events{/privacy}","received_events_url":"https://api.github.com/users/CartoDB/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/CartoDB/cartodb","description":"Geospatial database for the cloud","fork":false,"url":"https://api.github.com/repos/CartoDB/cartodb","forks_url":"https://api.github.com/repos/CartoDB/cartodb/forks","keys_url":"https://api.github.com/repos/CartoDB/cartodb/keys{/key_id}","collaborators_url":"https://api.github.com/repos/CartoDB/cartodb/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/CartoDB/cartodb/teams","hooks_url":"https://api.github.com/repos/CartoDB/cartodb/hooks","issue_events_url":"https://api.github.com/repos/CartoDB/cartodb/issues/events{/number}","events_url":"https://api.github.com/repos/CartoDB/cartodb/events","assignees_url":"https://api.github.com/repos/CartoDB/cartodb/assignees{/user}","branches_url":"https://api.github.com/repos/CartoDB/cartodb/branches{/branch}","tags_url":"https://api.github.com/repos/CartoDB/cartodb/tags","blobs_url":"https://api.github.com/repos/CartoDB/cartodb/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/CartoDB/cartodb/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/CartoDB/cartodb/git/refs{/sha}","trees_url":"https://api.github.com/repos/CartoDB/cartodb/git/trees{/sha}","statuses_url":"https://api.github.com/repos/CartoDB/cartodb/statuses/{sha}","languages_url":"https://api.github.com/repos/CartoDB/cartodb/languages","stargazers_url":"https://api.github.com/repos/CartoDB/cartodb/stargazers","contributors_url":"https://api.github.com/repos/CartoDB/cartodb/contributors","subscribers_url":"https://api.github.com/repos/CartoDB/cartodb/subscribers","subscription_url":"https://api.github.com/repos/CartoDB/cartodb/subscription","commits_url":"https://api.github.com/repos/CartoDB/cartodb/commits{/sha}","git_commits_url":"https://api.github.com/repos/CartoDB/cartodb/git/commits{/sha}","comments_url":"https://api.github.com/repos/CartoDB/cartodb/comments{/number}","issue_comment_url":"https://api.github.com/repos/CartoDB/cartodb/issues/comments{/number}","contents_url":"https://api.github.com/repos/CartoDB/cartodb/contents/{+path}","compare_url":"https://api.github.com/repos/CartoDB/cartodb/compare/{base}...{head}","merges_url":"https://api.github.com/repos/CartoDB/cartodb/merges","archive_url":"https://api.github.com/repos/CartoDB/cartodb/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/CartoDB/cartodb/downloads","issues_url":"https://api.github.com/repos/CartoDB/cartodb/issues{/number}","pulls_url":"https://api.github.com/repos/CartoDB/cartodb/pulls{/number}","milestones_url":"https://api.github.com/repos/CartoDB/cartodb/milestones{/number}","notifications_url":"https://api.github.com/repos/CartoDB/cartodb/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/CartoDB/cartodb/labels{/name}","releases_url":"https://api.github.com/repos/CartoDB/cartodb/releases{/id}","created_at":"2012-07-05T15:23:45Z","updated_at":"2015-06-07T17:04:24Z","pushed_at":"2015-06-08T15:14:25Z","git_url":"git://github.com/CartoDB/cartodb.git","ssh_url":"git@github.com:CartoDB/cartodb.git","clone_url":"https://github.com/CartoDB/cartodb.git","svn_url":"https://github.com/CartoDB/cartodb","homepage":"www.cartodb.com","size":472557,"stargazers_count":817,"watchers_count":817,"language":"JavaScript","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":193,"mirror_url":null,"open_issues_count":593,"forks":193,"open_issues":593,"watchers":817,"default_branch":"master"}},"base":{"label":"CartoDB:master","ref":"master","sha":"a9a3b3c79b369646b3a59b02c173d54af0222112","user":{"login":"CartoDB","id":1799254,"avatar_url":"https://avatars.githubusercontent.com/u/1799254?v=3","gravatar_id":"","url":"https://api.github.com/users/CartoDB","html_url":"https://github.com/CartoDB","followers_url":"https://api.github.com/users/CartoDB/followers","following_url":"https://api.github.com/users/CartoDB/following{/other_user}","gists_url":"https://api.github.com/users/CartoDB/gists{/gist_id}","starred_url":"https://api.github.com/users/CartoDB/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/CartoDB/subscriptions","organizations_url":"https://api.github.com/users/CartoDB/orgs","repos_url":"https://api.github.com/users/CartoDB/repos","events_url":"https://api.github.com/users/CartoDB/events{/privacy}","received_events_url":"https://api.github.com/users/CartoDB/received_events","type":"Organization","site_admin":false},"repo":{"id":4909355,"name":"cartodb","full_name":"CartoDB/cartodb","owner":{"login":"CartoDB","id":1799254,"avatar_url":"https://avatars.githubusercontent.com/u/1799254?v=3","gravatar_id":"","url":"https://api.github.com/users/CartoDB","html_url":"https://github.com/CartoDB","followers_url":"https://api.github.com/users/CartoDB/followers","following_url":"https://api.github.com/users/CartoDB/following{/other_user}","gists_url":"https://api.github.com/users/CartoDB/gists{/gist_id}","starred_url":"https://api.github.com/users/CartoDB/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/CartoDB/subscriptions","organizations_url":"https://api.github.com/users/CartoDB/orgs","repos_url":"https://api.github.com/users/CartoDB/repos","events_url":"https://api.github.com/users/CartoDB/events{/privacy}","received_events_url":"https://api.github.com/users/CartoDB/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/CartoDB/cartodb","description":"Geospatial database for the cloud","fork":false,"url":"https://api.github.com/repos/CartoDB/cartodb","forks_url":"https://api.github.com/repos/CartoDB/cartodb/forks","keys_url":"https://api.github.com/repos/CartoDB/cartodb/keys{/key_id}","collaborators_url":"https://api.github.com/repos/CartoDB/cartodb/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/CartoDB/cartodb/teams","hooks_url":"https://api.github.com/repos/CartoDB/cartodb/hooks","issue_events_url":"https://api.github.com/repos/CartoDB/cartodb/issues/events{/number}","events_url":"https://api.github.com/repos/CartoDB/cartodb/events","assignees_url":"https://api.github.com/repos/CartoDB/cartodb/assignees{/user}","branches_url":"https://api.github.com/repos/CartoDB/cartodb/branches{/branch}","tags_url":"https://api.github.com/repos/CartoDB/cartodb/tags","blobs_url":"https://api.github.com/repos/CartoDB/cartodb/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/CartoDB/cartodb/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/CartoDB/cartodb/git/refs{/sha}","trees_url":"https://api.github.com/repos/CartoDB/cartodb/git/trees{/sha}","statuses_url":"https://api.github.com/repos/CartoDB/cartodb/statuses/{sha}","languages_url":"https://api.github.com/repos/CartoDB/cartodb/languages","stargazers_url":"https://api.github.com/repos/CartoDB/cartodb/stargazers","contributors_url":"https://api.github.com/repos/CartoDB/cartodb/contributors","subscribers_url":"https://api.github.com/repos/CartoDB/cartodb/subscribers","subscription_url":"https://api.github.com/repos/CartoDB/cartodb/subscription","commits_url":"https://api.github.com/repos/CartoDB/cartodb/commits{/sha}","git_commits_url":"https://api.github.com/repos/CartoDB/cartodb/git/commits{/sha}","comments_url":"https://api.github.com/repos/CartoDB/cartodb/comments{/number}","issue_comment_url":"https://api.github.com/repos/CartoDB/cartodb/issues/comments{/number}","contents_url":"https://api.github.com/repos/CartoDB/cartodb/contents/{+path}","compare_url":"https://api.github.com/repos/CartoDB/cartodb/compare/{base}...{head}","merges_url":"https://api.github.com/repos/CartoDB/cartodb/merges","archive_url":"https://api.github.com/repos/CartoDB/cartodb/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/CartoDB/cartodb/downloads","issues_url":"https://api.github.com/repos/CartoDB/cartodb/issues{/number}","pulls_url":"https://api.github.com/repos/CartoDB/cartodb/pulls{/number}","milestones_url":"https://api.github.com/repos/CartoDB/cartodb/milestones{/number}","notifications_url":"https://api.github.com/repos/CartoDB/cartodb/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/CartoDB/cartodb/labels{/name}","releases_url":"https://api.github.com/repos/CartoDB/cartodb/releases{/id}","created_at":"2012-07-05T15:23:45Z","updated_at":"2015-06-07T17:04:24Z","pushed_at":"2015-06-08T15:14:25Z","git_url":"git://github.com/CartoDB/cartodb.git","ssh_url":"git@github.com:CartoDB/cartodb.git","clone_url":"https://github.com/CartoDB/cartodb.git","svn_url":"https://github.com/CartoDB/cartodb","homepage":"www.cartodb.com","size":472557,"stargazers_count":817,"watchers_count":817,"language":"JavaScript","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":193,"mirror_url":null,"open_issues_count":593,"forks":193,"open_issues":593,"watchers":817,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/CartoDB/cartodb/pulls/3881"},"html":{"href":"https://github.com/CartoDB/cartodb/pull/3881"},"issue":{"href":"https://api.github.com/repos/CartoDB/cartodb/issues/3881"},"comments":{"href":"https://api.github.com/repos/CartoDB/cartodb/issues/3881/comments"},"review_comments":{"href":"https://api.github.com/repos/CartoDB/cartodb/pulls/3881/comments"},"review_comment":{"href":"https://api.github.com/repos/CartoDB/cartodb/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/CartoDB/cartodb/pulls/3881/commits"},"statuses":{"href":"https://api.github.com/repos/CartoDB/cartodb/statuses/3540f274b8fa0eed34dde9b438500a28e841df9c"}}}}