Fix performance issues for shortcut menu

3 files changed, 117 insertions, 18 deletions

diff --git a/ishtar_common/models.py b/ishtar_common/models.py
index 6fb7852ae..7015c70a5 100644
--- a/ishtar_common/models.py
+++ b/ishtar_common/models.py
@@ -226,7 +226,7 @@ class OwnPerms:
         query = self.get_query_owns(user)
         if not query:
             return False
-        query = query & Q(pk=self.pk)
+        query &= Q(pk=self.pk)
         return self.__class__.objects.filter(query).count()
 
     @classmethod
@@ -240,28 +240,57 @@ class OwnPerms:
         return cls.objects.filter(query).count()
 
     @classmethod
-    def get_owns(cls, user, replace_query={}, limit=None):
+    def _return_get_owns(cls, owns, values, get_short_menu_class,
+                         label_key='cached_label'):
+        if not values:
+            if not get_short_menu_class:
+                return sorted(owns, key=lambda x: getattr(x, label_key))
+            return sorted(owns, key=lambda x: getattr(x[0], label_key))
+        if not get_short_menu_class:
+            return sorted(owns, key=lambda x: x[label_key])
+        return sorted(owns, key=lambda x: x[0][label_key])
+
+    @classmethod
+    def get_owns(cls, user, replace_query={}, limit=None, values=None,
+                 get_short_menu_class=False):
         """
         Get Own items
         """
         if isinstance(user, User):
             user = IshtarUser.objects.get(user_ptr=user)
         if user.is_anonymous():
-            return cls.objects.filter(pk__isnull=True)
+            returned = cls.objects.filter(pk__isnull=True)
+            if values:
+                returned = []
+            return returned
         items = []
         if hasattr(cls, 'BASKET_MODEL'):
             items = list(cls.BASKET_MODEL.objects.filter(user=user).all())
         query = cls.get_query_owns(user)
         if not query and not replace_query:
-            return cls.objects.filter(pk__isnull=True)
+            returned = cls.objects.filter(pk__isnull=True)
+            if values:
+                returned = []
+            return returned
         if query:
             q = cls.objects.filter(query)
         if replace_query:
             q = cls.objects.filter(replace_query)
+        if values:
+            q = q.values(*values)
         if limit:
             items += list(q.order_by('-pk')[:limit])
         else:
             items += list(q.order_by(*cls._meta.ordering).all())
+        if get_short_menu_class:
+            if values:
+                if 'id' not in values:
+                    raise NotImplementedError(
+                        "Call of get_owns with get_short_menu_class option and"
+                        " no 'id' in values is not implemented")
+                items = [(i, cls.get_short_menu_class(i['id'])) for i in items]
+            else:
+                items = [(i, cls.get_short_menu_class(i.pk)) for i in items]
         return items
 
 
@@ -643,7 +672,8 @@ class Basket(models.Model):
     def __unicode__(self):
         return self.label
 
-    def get_short_menu_class(self):
+    @classmethod
+    def get_short_menu_class(cls, pk):
         return 'basket'
 
     @property
@@ -968,7 +998,8 @@ def post_delete_record_relation(sender, instance, **kwargs):
 
 
 class ShortMenuItem(object):
-    def get_short_menu_class(self):
+    @classmethod
+    def get_short_menu_class(cls, pk):
         return ''
 
 
diff --git a/ishtar_common/tests.py b/ishtar_common/tests.py
index 10584e4f2..cdf6ce330 100644
--- a/ishtar_common/tests.py
+++ b/ishtar_common/tests.py
@@ -334,6 +334,70 @@ class MergeTest(TestCase):
                          init_mc)
 
 
+class ShortMenuTest(TestCase):
+    def setUp(self):
+        from archaeological_operations.models import OperationType
+        self.username = 'username666'
+        self.password = 'dcbqj7xnjkxnjsknx!@%'
+        self.user = User.objects.create_superuser(
+            self.username, "nomail@nomail.com", self.password)
+        self.other_user = User.objects.create_superuser(
+            'John', "nomail@nomail.com", self.password)
+        self.ope_type = OperationType.objects.create()
+
+    def testNotConnected(self):
+        c = Client()
+        response = c.get(reverse('shortcut-menu'))
+        # no content if not logged
+        self.assertFalse("shortcut-menu" in response.content)
+        c = Client()
+        c.login(username=self.username, password=self.password)
+        # no content because the user owns no object
+        response = c.get(reverse('shortcut-menu'))
+        self.assertFalse("shortcut-menu" in response.content)
+        from archaeological_operations.models import Operation
+        Operation.objects.create(
+            operation_type=self.ope_type,
+            history_modifier=self.user)
+        # content is here
+        response = c.get(reverse('shortcut-menu'))
+        self.assertTrue("shortcut-menu" in response.content)
+
+    def testOperation(self):
+        from archaeological_operations.models import Operation
+        c = Client()
+        c.login(username=self.username, password=self.password)
+        ope = Operation.objects.create(
+            operation_type=self.ope_type,
+            history_modifier=self.other_user,
+            year=2042, operation_code=54
+        )
+        # not available at first
+        response = c.get(reverse('shortcut-menu'))
+        self.assertFalse(str(ope.cached_label) in response.content)
+
+        # available because is the creator
+        ope.history_creator = self.user
+        ope.save()
+        response = c.get(reverse('shortcut-menu'))
+        self.assertTrue(str(ope.cached_label) in response.content)
+
+        # available because is in charge
+        ope.history_creator = self.other_user
+        ope.in_charge = self.user.ishtaruser.person
+        ope.save()
+        response = c.get(reverse('shortcut-menu'))
+        self.assertTrue(str(ope.cached_label) in response.content)
+
+        # available because is the scientist
+        ope.history_creator = self.other_user
+        ope.in_charge = None
+        ope.scientist = self.user.ishtaruser.person
+        ope.save()
+        response = c.get(reverse('shortcut-menu'))
+        self.assertTrue(str(ope.cached_label) in response.content)
+
+
 class ImportTest(TestCase):
     def testDeleteRelated(self):
         town = models.Town.objects.create(name='my-test')
diff --git a/ishtar_common/views.py b/ishtar_common/views.py
index b0817fc59..dbbc3d538 100644
--- a/ishtar_common/views.py
+++ b/ishtar_common/views.py
@@ -256,7 +256,6 @@ def shortcut_menu(request):
             model_name = model.SLUG
             current = model_name in request.session \
                 and request.session[model_name]
-
             dct['menu'].append((
                 lbl, model_name, current or 0,
                 JQueryAutoComplete(
@@ -275,23 +274,28 @@ def shortcut_menu(request):
         cls = ''
         current = model_name in request.session and request.session[model_name]
         items = []
-        for item in model.get_owns(request.user,
-                                   menu_filtr=current_selected_item,
-                                   limit=100):
-            pk = unicode(item.pk)
-            if item.IS_BASKET:
+        current_items = []
+        for item, shortmenu_class in model.get_owns(
+                request.user, menu_filtr=current_selected_item, limit=100,
+                values=['id', 'cached_label'], get_short_menu_class=True):
+            pk = unicode(item['id'])
+            if shortmenu_class == 'basket':
                 pk = "basket-" + pk
+            # prevent duplicates
+            if pk in current_items:
+                continue
+            current_items.append(pk)
             selected = pk == current
             if selected:
-                cls = item.get_short_menu_class()
-                new_selected_item = item
-            items.append((pk, shortify(unicode(item), 60),
-                          selected, item.get_short_menu_class()))
+                cls = shortmenu_class
+                new_selected_item = pk
+            items.append((pk, shortify(item['cached_label'], 60),
+                          selected, shortmenu_class))
         # selected is not in owns - add it to the list
         if not new_selected_item and current:
             try:
                 item = model.objects.get(pk=int(current))
-                new_selected_item = item
+                new_selected_item = item.pk
                 items.append((item.pk, shortify(unicode(item), 60),
                               True, item.get_short_menu_class()))
             except (model.DoesNotExist, ValueError):
@@ -323,7 +327,7 @@ def get_current_items(request):
 
 def unpin(request, item_type):
     if item_type not in ('find', 'contextrecord', 'operation', 'file',
-                          'treatment', 'treatmentfile'):
+                         'treatment', 'treatmentfile'):
         logger.warning("unpin unknow type: {}".format(item_type))
         return HttpResponse('nok')
     request.session['treatment'] = ''