Allow collaborators to create, delete and edit visualizations.

redash/handlers/visualizations.py

@@ -1,9 +1,12 @@
 import json
+
 from flask import request
 
 from redash import models
-from redash.permissions import require_permission, require_admin_or_owner
 from redash.handlers.base import BaseResource, get_object_or_404
+from redash.permissions import (require_admin_or_owner,
+                                require_object_modify_permission,
+                                require_permission)
 
 
 class VisualizationListResource(BaseResource):
@@ -12,7 +15,7 @@ class VisualizationListResource(BaseResource):
         kwargs = request.get_json(force=True)
 
         query = get_object_or_404(models.Query.get_by_id_and_org, kwargs.pop('query_id'), self.current_org)
-        require_admin_or_owner(query.user_id)
+        require_object_modify_permission(query, self.current_user)
 
         kwargs['options'] = json.dumps(kwargs['options'])
         kwargs['query_rel'] = query
@@ -28,7 +31,7 @@ class VisualizationResource(BaseResource):
     @require_permission('edit_query')
     def post(self, visualization_id):
         vis = get_object_or_404(models.Visualization.get_by_id_and_org, visualization_id, self.current_org)
-        require_admin_or_owner(vis.query_rel.user_id)
+        require_object_modify_permission(vis.query_rel, self.current_user)
 
         kwargs = request.get_json(force=True)
         if 'options' in kwargs:
@@ -45,6 +48,6 @@ class VisualizationResource(BaseResource):
     @require_permission('edit_query')
     def delete(self, visualization_id):
         vis = get_object_or_404(models.Visualization.get_by_id_and_org, visualization_id, self.current_org)
-        require_admin_or_owner(vis.query_rel.user_id)
+        require_object_modify_permission(vis.query_rel, self.current_user)
         models.db.session.delete(vis)
         models.db.session.commit()

redash/permissions.py

@@ -1,6 +1,7 @@
+import functools
+
 from flask_login import current_user
 from flask_restful import abort
-import functools
 from funcy import flatten
 
 view_only = True

tests/handlers/test_visualizations.py

@@ -0,0 +1,130 @@
+from tests import BaseTestCase
+
+from redash import models
+
+
+class VisualizationResourceTest(BaseTestCase):
+    def test_create_visualization(self):
+        query = self.factory.create_query()
+        models.db.session.commit()
+        data = {
+            'query_id': query.id,
+            'name': 'Chart',
+            'description': '',
+            'options': {},
+            'type': 'CHART'
+        }
+
+        rv = self.make_request('post', '/api/visualizations', data=data)
+
+        self.assertEquals(rv.status_code, 200)
+        data.pop('query_id')
+        self.assertDictContainsSubset(data, rv.json)
+
+    def test_delete_visualization(self):
+        visualization = self.factory.create_visualization()
+        models.db.session.commit()
+        rv = self.make_request('delete', '/api/visualizations/{}'.format(visualization.id))
+
+        self.assertEquals(rv.status_code, 200)
+        self.assertEquals(models.db.session.query(models.Visualization).count(), 0)
+
+    def test_update_visualization(self):
+        visualization = self.factory.create_visualization()
+        models.db.session.commit()
+        rv = self.make_request('post', '/api/visualizations/{0}'.format(visualization.id), data={'name': 'After Update'})
+
+        self.assertEquals(rv.status_code, 200)
+        self.assertEquals(rv.json['name'], 'After Update')
+
+    def test_only_owner_collaborator_or_admin_can_create_visualization(self):
+        query = self.factory.create_query()
+        other_user = self.factory.create_user()
+        admin = self.factory.create_admin()
+        admin_from_diff_org = self.factory.create_admin(org=self.factory.create_org())
+        models.db.session.commit()
+        models.db.session.refresh(admin)
+        models.db.session.refresh(other_user)
+        models.db.session.refresh(admin_from_diff_org)
+        data = {
+            'query_id': query.id,
+            'name': 'Chart',
+            'description': '',
+            'options': {},
+            'type': 'CHART'
+        }
+
+        rv = self.make_request('post', '/api/visualizations', data=data, user=admin)
+        self.assertEquals(rv.status_code, 200)
+
+        rv = self.make_request('post', '/api/visualizations', data=data, user=other_user)
+        self.assertEquals(rv.status_code, 403)
+
+        self.make_request('post', '/api/queries/{}/acl'.format(query.id), data={'access_type': 'modify', 'user_id': other_user.id})
+        rv = self.make_request('post', '/api/visualizations', data=data, user=other_user)
+        self.assertEquals(rv.status_code, 200)
+
+        rv = self.make_request('post', '/api/visualizations', data=data, user=admin_from_diff_org)
+        self.assertEquals(rv.status_code, 404)
+
+    def test_only_owner_collaborator_or_admin_can_edit_visualization(self):
+        vis = self.factory.create_visualization()
+        models.db.session.flush()
+        path = '/api/visualizations/{}'.format(vis.id)
+        data = {'name': 'After Update'}
+
+        other_user = self.factory.create_user()
+        admin = self.factory.create_admin()
+        admin_from_diff_org = self.factory.create_admin(org=self.factory.create_org())
+        models.db.session.commit()
+        models.db.session.refresh(admin)
+        models.db.session.refresh(other_user)
+        models.db.session.refresh(admin_from_diff_org)
+
+        rv = self.make_request('post', path, user=admin, data=data)
+        self.assertEquals(rv.status_code, 200)
+
+        rv = self.make_request('post', path, user=other_user, data=data)
+        self.assertEquals(rv.status_code, 403)
+
+        self.make_request('post', '/api/queries/{}/acl'.format(vis.query_id), data={'access_type': 'modify', 'user_id': other_user.id})
+        rv = self.make_request('post', path, user=other_user, data=data)
+        self.assertEquals(rv.status_code, 200)
+
+        rv = self.make_request('post', path, user=admin_from_diff_org, data=data)
+        self.assertEquals(rv.status_code, 404)
+
+    def test_only_owner_collaborator_or_admin_can_delete_visualization(self):
+        vis = self.factory.create_visualization()
+        models.db.session.flush()
+        path = '/api/visualizations/{}'.format(vis.id)
+
+        other_user = self.factory.create_user()
+        admin = self.factory.create_admin()
+        admin_from_diff_org = self.factory.create_admin(org=self.factory.create_org())
+
+        models.db.session.commit()
+        models.db.session.refresh(admin)
+        models.db.session.refresh(other_user)
+        models.db.session.refresh(admin_from_diff_org)
+        rv = self.make_request('delete', path, user=admin)
+        self.assertEquals(rv.status_code, 200)
+
+        vis = self.factory.create_visualization()
+        models.db.session.commit()
+        path = '/api/visualizations/{}'.format(vis.id)
+
+        rv = self.make_request('delete', path, user=other_user)
+        self.assertEquals(rv.status_code, 403)
+
+        self.make_request('post', '/api/queries/{}/acl'.format(vis.query_id), data={'access_type': 'modify', 'user_id': other_user.id})
+
+        rv = self.make_request('delete', path, user=other_user)
+        self.assertEquals(rv.status_code, 200)
+
+        vis = self.factory.create_visualization()
+        models.db.session.commit()
+        path = '/api/visualizations/{}'.format(vis.id)
+
+        rv = self.make_request('delete', path, user=admin_from_diff_org)
+        self.assertEquals(rv.status_code, 404)

tests/test_handlers.py

@@ -1,11 +1,10 @@
-from funcy import project
-
 from flask import url_for
 from flask_login import current_user
+from funcy import project
 from mock import patch
+from tests import BaseTestCase, authenticated_user
+
 from redash import models, settings
-from tests import BaseTestCase
-from tests import authenticated_user
 
 
 class AuthenticationTestMixin(object):
@@ -65,121 +64,6 @@ class StatusTest(BaseTestCase):
         self.assertEqual(rv.status_code, 302)
 
 
-class VisualizationResourceTest(BaseTestCase):
-    def test_create_visualization(self):
-        query = self.factory.create_query()
-        models.db.session.commit()
-        data = {
-            'query_id': query.id,
-            'name': 'Chart',
-            'description': '',
-            'options': {},
-            'type': 'CHART'
-        }
-
-        rv = self.make_request('post', '/api/visualizations', data=data)
-
-        self.assertEquals(rv.status_code, 200)
-        data.pop('query_id')
-        self.assertDictContainsSubset(data, rv.json)
-
-    def test_delete_visualization(self):
-        visualization = self.factory.create_visualization()
-        models.db.session.commit()
-        rv = self.make_request('delete', '/api/visualizations/{}'.format(visualization.id))
-
-        self.assertEquals(rv.status_code, 200)
-        self.assertEquals(models.db.session.query(models.Visualization).count(), 0)
-
-    def test_update_visualization(self):
-        visualization = self.factory.create_visualization()
-        models.db.session.commit()
-        rv = self.make_request('post', '/api/visualizations/{0}'.format(visualization.id), data={'name': 'After Update'})
-
-        self.assertEquals(rv.status_code, 200)
-        self.assertEquals(rv.json['name'], 'After Update')
-
-    def test_only_owner_or_admin_can_create_visualization(self):
-        query = self.factory.create_query()
-        other_user = self.factory.create_user()
-        admin = self.factory.create_admin()
-        admin_from_diff_org = self.factory.create_admin(org=self.factory.create_org())
-        models.db.session.commit()
-        models.db.session.refresh(admin)
-        models.db.session.refresh(other_user)
-        models.db.session.refresh(admin_from_diff_org)
-        data = {
-            'query_id': query.id,
-            'name': 'Chart',
-            'description': '',
-            'options': {},
-            'type': 'CHART'
-        }
-
-
-        rv = self.make_request('post', '/api/visualizations', data=data, user=admin)
-        self.assertEquals(rv.status_code, 200)
-
-        rv = self.make_request('post', '/api/visualizations', data=data, user=other_user)
-        self.assertEquals(rv.status_code, 403)
-
-        rv = self.make_request('post', '/api/visualizations', data=data, user=admin_from_diff_org)
-        self.assertEquals(rv.status_code, 404)
-
-    def test_only_owner_or_admin_can_edit_visualization(self):
-        vis = self.factory.create_visualization()
-        models.db.session.flush()
-        path = '/api/visualizations/{}'.format(vis.id)
-        data = {'name': 'After Update'}
-
-        other_user = self.factory.create_user()
-        admin = self.factory.create_admin()
-        admin_from_diff_org = self.factory.create_admin(org=self.factory.create_org())
-        models.db.session.commit()
-        models.db.session.refresh(admin)
-        models.db.session.refresh(other_user)
-        models.db.session.refresh(admin_from_diff_org)
-
-        rv = self.make_request('post', path, user=admin, data=data)
-        self.assertEquals(rv.status_code, 200)
-
-        rv = self.make_request('post', path, user=other_user, data=data)
-        self.assertEquals(rv.status_code, 403)
-
-        rv = self.make_request('post', path, user=admin_from_diff_org, data=data)
-        self.assertEquals(rv.status_code, 404)
-
-    def test_only_owner_or_admin_can_delete_visualization(self):
-        vis = self.factory.create_visualization()
-        models.db.session.flush()
-        path = '/api/visualizations/{}'.format(vis.id)
-
-        other_user = self.factory.create_user()
-        admin = self.factory.create_admin()
-        admin_from_diff_org = self.factory.create_admin(org=self.factory.create_org())
-
-        models.db.session.commit()
-        models.db.session.refresh(admin)
-        models.db.session.refresh(other_user)
-        models.db.session.refresh(admin_from_diff_org)
-        rv = self.make_request('delete', path, user=admin)
-        self.assertEquals(rv.status_code, 200)
-
-        vis = self.factory.create_visualization()
-        models.db.session.commit()
-        path = '/api/visualizations/{}'.format(vis.id)
-
-        rv = self.make_request('delete', path, user=other_user)
-        self.assertEquals(rv.status_code, 403)
-
-        vis = self.factory.create_visualization()
-        models.db.session.commit()
-        path = '/api/visualizations/{}'.format(vis.id)
-
-        rv = self.make_request('delete', path, user=admin_from_diff_org)
-        self.assertEquals(rv.status_code, 404)
-
-
 class JobAPITest(BaseTestCase, AuthenticationTestMixin):
     def setUp(self):
         self.paths = []