Răsfoiți Sursa

HUE-2039 [beeswax] Remove the need to clone a shared query

A non shared design can't ne seen or modified by someone else without the
write perm.
A shared design can be executed in auto mode and will appear in the history
of the user.
A shared design can be saved as a new design.
Adding tests, refactored a bit the view to remove some logic from the view.
Romain Rigaux 11 ani în urmă
părinte
comite
965f9ca248

+ 4 - 6
apps/beeswax/src/beeswax/models.py

@@ -15,7 +15,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import copy
 import base64
 import datetime
 import logging
@@ -253,12 +252,11 @@ class SavedQuery(models.Model):
       pass
 
   def clone(self):
-    """clone() -> A new SavedQuery with a deep copy of the same data"""
     design = SavedQuery(type=self.type, owner=self.owner)
-    design.data = copy.deepcopy(self.data)
-    design.name = copy.deepcopy(self.name)
-    design.desc = copy.deepcopy(self.desc)
-    design.is_auto = copy.deepcopy(self.is_auto)
+    design.data = self.data
+    design.name = self.name
+    design.desc = self.desc
+    design.is_auto = self.is_auto
     return design
 
   @classmethod

+ 2 - 0
apps/beeswax/src/beeswax/templates/execute.mako

@@ -273,7 +273,9 @@ ${layout.menubar(section='query')}
             <button data-bind="click: tryExecuteNextStatement, visible: !$root.design.isFinished()" type="button" class="btn btn-primary disable-feedback" tabindex="2">${_('Next')}</button>
             <button data-bind="click: tryExecuteQuery, visible: !$root.design.isFinished()" type="button" id="executeQuery" class="btn btn-primary disable-feedback" tabindex="2">${_('Restart')}</button>
 
+            % if can_edit:
             <button data-bind="click: trySaveDesign, css: {'hide': !$root.design.id() || $root.design.id() == -1}" type="button" class="btn hide">${_('Save')}</button>
+            % endif
             <button data-bind="click: saveAsModal" type="button" class="btn">${_('Save as...')}</button>
             <button data-bind="click: tryExplainQuery, visible: $root.canExecute" type="button" id="explainQuery" class="btn">${_('Explain')}</button>
             &nbsp; ${_('or create a')} &nbsp;

+ 7 - 9
apps/beeswax/src/beeswax/templates/list_designs.mako

@@ -89,24 +89,20 @@ ${ layout.menubar(section='saved queries') }
     <tbody>
       % for design in page.object_list:
         <%
-          may_edit = user == design.owner
+          may_edit = design.doc.get().can_write(user)
         %>
       <tr>
         <td data-row-selector-exclude="true">
           <div class="hueCheckbox savedCheck fa"
-            % if may_edit:
               data-edit-url="${ url(app_name + ':execute_design', design_id=design.id) }"
-              data-delete-name="${ design.id }"
               data-history-url="${ url(app_name + ':list_query_history') }?q-design_id=${design.id}"
+            % if may_edit:
+              data-delete-name="${ design.id }"
             % endif
             data-clone-url="${ url(app_name + ':clone_design', design_id=design.id) }" data-row-selector-exclude="true"></div>
         </td>
         <td>
-        % if may_edit:
           <a href="${ url(app_name + ':execute_design', design_id=design.id) }" data-row-selector="true">${ force_unicode(design.name) }</a>
-        % else:
-          ${ force_unicode(design.name) }
-        % endif
         </td>
         <td>
         % if design.desc:
@@ -207,7 +203,7 @@ ${ layout.menubar(section='saved queries') }
     function toggleActions() {
       $(".toolbarBtn").attr("disabled", "disabled");
 
-      var selector = $(".hueCheckbox[checked='checked']");
+      var selector = $(".hueCheckbox[checked='checked']:not(.selectAll)");
       if (selector.length == 1) {
         if (selector.data("edit-url")) {
           $("#editBtn").removeAttr("disabled").click(function () {
@@ -225,7 +221,9 @@ ${ layout.menubar(section='saved queries') }
           });
         }
       }
-      if (selector.length >= 1) {
+
+      var can_delete = $(".hueCheckbox[checked='checked'][data-delete-name]");
+      if (can_delete.length > 0 && can_delete.length == selector.length) {
         $("#trashQueryBtn").removeAttr("disabled");
         $("#trashQueryCaretBtn").removeAttr("disabled");
       }

+ 62 - 1
apps/beeswax/src/beeswax/tests.py

@@ -51,7 +51,7 @@ import beeswax.views
 
 from beeswax import conf, hive_site
 from beeswax.conf import HIVE_SERVER_HOST
-from beeswax.views import collapse_whitespace
+from beeswax.views import collapse_whitespace, _save_design
 from beeswax.test_base import make_query, wait_for_query_to_finish, verify_history, get_query_server_config,\
   HIVE_SERVER_TEST_PORT, fetch_query_result_data
 from beeswax.design import hql_query, strip_trailing_semicolon
@@ -63,6 +63,7 @@ from beeswax.server.hive_server2_lib import HiveServerClient,\
   PartitionValueCompatible, HiveServerTable
 from beeswax.test_base import BeeswaxSampleProvider
 from beeswax.hive_site import get_metastore
+from desktop.lib.exceptions_renderable import PopupException
 
 
 
@@ -1840,6 +1841,9 @@ class TestWithMockedServer(object):
     dbms.HiveServer2Dbms = MockDbms
 
     self.client = make_logged_in_client(is_superuser=False)
+    self.client_not_me = make_logged_in_client(username='not_me', is_superuser=False, groupname='test')
+    self.user = User.objects.get(username='test')
+    self.user_not_me = User.objects.get(username='not_me')
     grant_access("test", "test", "beeswax")
 
   def tearDown(self):
@@ -1892,6 +1896,63 @@ class TestWithMockedServer(object):
     ids_page_1 = set([query.id for query in resp.context['page'].object_list])
     assert_equal(0, sum([query_id in ids_page_1 for query_id in ids]))
 
+  def test_save_design(self):
+    response = _make_query(self.client, 'SELECT', submission_type='Save', name='My Name 1', desc='My Description')
+    content = json.loads(response.content)
+    design_id = content['design_id']
+
+    design = SavedQuery.objects.get(id=design_id)
+    design_obj = hql_query('SELECT')
+
+    # Save his own query
+    saved_design = _save_design(user=self.user, design=design, type_=HQL, design_obj=design_obj, explicit_save=True, name='test_save_design', desc='test_save_design desc')
+    assert_equal('test_save_design', saved_design.name)
+    assert_equal('test_save_design desc', saved_design.desc)
+    assert_equal('test_save_design', saved_design.doc.get().name)
+    assert_equal('test_save_design desc', saved_design.doc.get().description)
+    assert_false(saved_design.doc.get().is_historic())
+
+    # Execute it as auto
+    saved_design = _save_design(user=self.user, design=design, type_=HQL, design_obj=design_obj, explicit_save=False, name='test_save_design', desc='test_save_design desc')
+    assert_equal('test_save_design (new)', saved_design.name)
+    assert_equal('test_save_design desc', saved_design.desc)
+    assert_equal('test_save_design (new)', saved_design.doc.get().name)
+    assert_equal('test_save_design desc', saved_design.doc.get().description)
+    assert_true(saved_design.doc.get().is_historic())
+
+    # not_me user can't modify it
+    try:
+      _save_design(user=self.user_not_me, design=design, type_=HQL, design_obj=design_obj, explicit_save=True, name='test_save_design', desc='test_save_design desc')
+      assert_true(False, 'not_me is not allowed')
+    except PopupException:
+      pass
+
+    saved_design.doc.get().share_to_default()
+
+    try:
+      _save_design(user=self.user_not_me, design=design, type_=HQL, design_obj=design_obj, explicit_save=True, name='test_save_design', desc='test_save_design desc')
+      assert_true(False, 'not_me is not allowed')
+    except PopupException:
+      pass
+
+    # not_me can execute it as auto
+    saved_design = _save_design(user=self.user_not_me, design=design, type_=HQL, design_obj=design_obj, explicit_save=False, name='test_save_design', desc='test_save_design desc')
+    assert_equal('test_save_design (new)', saved_design.name)
+    assert_equal('test_save_design desc', saved_design.desc)
+    assert_equal('test_save_design (new)', saved_design.doc.get().name)
+    assert_equal('test_save_design desc', saved_design.doc.get().description)
+    assert_true(saved_design.doc.get().is_historic())
+
+    # not_me can save as a new design
+    design = SavedQuery(owner=self.user_not_me, type=HQL)
+
+    saved_design = _save_design(user=self.user_not_me, design=design, type_=HQL, design_obj=design_obj, explicit_save=True, name='test_save_design', desc='test_save_design desc')
+    assert_equal('test_save_design', saved_design.name)
+    assert_equal('test_save_design desc', saved_design.desc)
+    assert_equal('test_save_design', saved_design.doc.get().name)
+    assert_equal('test_save_design desc', saved_design.doc.get().description)
+    assert_false(saved_design.doc.get().is_historic())
+
 
 class TestDesign():
 

+ 20 - 20
apps/beeswax/src/beeswax/views.py

@@ -76,7 +76,7 @@ def save_design(request, form, type_, design, explicit_save):
   Need to return a SavedQuery because we may end up with a different one.
   Assumes that form.saveform is the SaveForm, and that it is valid.
   """
-  authorized_get_design(request, design.id, owner_only=True)
+  authorized_get_design(request, design.id)
   assert form.saveform.is_valid()
   sub_design_form = form # Beeswax/Impala case
 
@@ -91,22 +91,30 @@ def save_design(request, form, type_, design, explicit_save):
   else:
     raise ValueError(_('Invalid design type %(type)s') % {'type': type_})
 
+  design_obj = design_cls(sub_design_form, query_type=type_)
+  name = form.saveform.cleaned_data['name']
+  desc = form.saveform.cleaned_data['desc']
+
+  return _save_design(request.user, design, type_, design_obj, explicit_save, name, desc)
+
+
+def _save_design(user, design, type_, design_obj, explicit_save, name=None, desc=None):
   # Design here means SavedQuery
   old_design = design
-  design_obj = design_cls(sub_design_form, query_type=type_)
   new_data = design_obj.dumps()
 
   # Auto save if (1) the user didn't click "save", and (2) the data is different.
-  # Don't generate an auto-saved design if the user didn't change anything
-  if explicit_save:
-    design.name = form.saveform.cleaned_data['name']
-    design.desc = form.saveform.cleaned_data['desc']
+  # Create an history design if the user is executing a shared design.
+  # Don't generate an auto-saved design if the user didn't change anything.
+  if explicit_save and (not design.doc.exists() or design.doc.get().can_write_or_exception(user)):
+    design.name = name
+    design.desc = desc
     design.is_auto = False
   elif design_obj != old_design.get_design():
     # Auto save iff the data is different
     if old_design.id is not None:
       # Clone iff the parent design isn't a new unsaved model
-      design = old_design.clone()
+      design = old_design.clone(new_owner=user)
       if not old_design.is_auto:
         design.name = old_design.name + models.SavedQuery.AUTO_DESIGN_SUFFIX
     else:
@@ -175,16 +183,7 @@ def clone_design(request, design_id):
     LOG.error('Cannot clone non-existent design %s' % (design_id,))
     return list_designs(request)
 
-  copy = design.clone()
-  copy_doc = design.doc.get().copy()
-  copy.name = design.name + ' (copy)'
-  copy.owner = request.user
-  copy.save()
-
-  copy_doc.owner = copy.owner
-  copy_doc.name = copy.name
-  copy_doc.save()
-  copy.doc.add(copy_doc)
+  copy = design.clone(request.user)
 
   messages.info(request, _('Copied design: %(name)s') % {'name': design.name})
 
@@ -404,6 +403,7 @@ def execute_query(request, design_id=None, query_history_id=None):
     'query_history': query_history,
     'autocomplete_base_url': reverse(get_app_name(request) + ':api_autocomplete_databases', kwargs={}),
     'can_edit_name': design and design.id and not design.is_auto,
+    'can_edit': design and design.id and design.doc.get().can_write(request.user),
     'action': action,
     'on_success_url': request.GET.get('on_success_url'),
     'has_metastore': 'metastore' in get_apps_dict(request.user)
@@ -460,7 +460,7 @@ def view_results(request, id, first_row=0):
     else:
       results = db.fetch(handle, start_over, 100)
       data = []
-      
+
       # Materialize and HTML escape results
       # TODO: use Number + list comprehension
       for row in results.rows():
@@ -630,11 +630,10 @@ def massage_columns_for_json(cols):
     })
   return massaged_cols
 
-
-# owner_only is deprecated
 def authorized_get_design(request, design_id, owner_only=False, must_exist=False):
   if design_id is None and not must_exist:
     return None
+
   try:
     design = SavedQuery.objects.get(id=design_id)
   except SavedQuery.DoesNotExist:
@@ -653,6 +652,7 @@ def authorized_get_design(request, design_id, owner_only=False, must_exist=False
 def authorized_get_query_history(request, query_history_id, owner_only=False, must_exist=False):
   if query_history_id is None and not must_exist:
     return None
+
   try:
     query_history = QueryHistory.get(id=query_history_id)
   except QueryHistory.DoesNotExist:

+ 1 - 4
desktop/core/src/desktop/models.py

@@ -414,8 +414,6 @@ class Document(models.Model):
   def copy(self, name=None, owner=None):
     copy_doc = self
 
-    tags = self.tags.all() # Don't copy tags
-
     copy_doc.pk = None
     copy_doc.id = None
     if name is not None:
@@ -424,8 +422,7 @@ class Document(models.Model):
       copy_doc.owner = owner
     copy_doc.save()
 
-    #tags = filter(lambda tag: tag.tag != DocumentTag.EXAMPLE, tags)
-    #if not tags:
+    # Don't copy tags
     default_tag = DocumentTag.objects.get_default_tag(copy_doc.owner)
     tags = [default_tag]
     copy_doc.tags.add(*tags)