Merge pull request #2563 from smilerz/test_fixes

fixed rating sort order and updated tests

cookbook/helper/recipe_search.py

@@ -32,6 +32,9 @@ class RecipeSearch():
             if custom_filter:
                 self._params = {**json.loads(custom_filter.search)}
                 self._original_params = {**(params or {})}
+                # json.loads casts rating as an integer, expecting string
+                if isinstance(self._params.get('rating', None), int):
+                    self._params['rating'] = str(self._params['rating'])
             else:
                 self._params = {**(params or {})}
         else:
@@ -434,22 +437,21 @@ class RecipeSearch():
 
     def rating_filter(self, rating=None):
         if rating or self._sort_includes('rating'):
-            lessthan = self._sort_includes('-rating') or '-' in (rating or [])
-            if lessthan:
-                default = 0
-            else:
+            lessthan = '-' in (rating or [])
+            reverse = 'rating' in (self._sort_order or []) and '-rating' not in (self._sort_order or [])
+            if lessthan or reverse:
                 default = 100
+            else:
+                default = 0
             # TODO make ratings a settings user-only vs all-users
-            self._queryset = self._queryset.annotate(rating=Round(Avg(Case(When(
-                cooklog__created_by=self._request.user, then='cooklog__rating'), default=default))))
+            self._queryset = self._queryset.annotate(rating=Round(Avg(Case(When(cooklog__created_by=self._request.user, then='cooklog__rating'), default=default))))
         if rating is None:
             return
 
         if rating == '0':
             self._queryset = self._queryset.filter(rating=0)
         elif lessthan:
-            self._queryset = self._queryset.filter(
-                rating__lte=int(rating[1:])).exclude(rating=0)
+            self._queryset = self._queryset.filter(rating__lte=int(rating[1:])).exclude(rating=0)
         else:
             self._queryset = self._queryset.filter(rating__gte=int(rating))
 

cookbook/tests/api/test_api_food.py

@@ -475,6 +475,7 @@ def test_root_filter(obj_tree_1, obj_2, obj_3, u1_s1):
     with scope(space=obj_tree_1.space):
         # for some reason the 'path' attribute changes between the factory and the test when using both obj_tree and obj
         obj_tree_1 = Food.objects.get(id=obj_tree_1.id)
+        obj_2 = Food.objects.get(id=obj_2.id)
         parent = obj_tree_1.get_parent()
 
     # should return root objects in the space (obj_1, obj_2), ignoring query filters
@@ -499,17 +500,16 @@ def test_tree_filter(obj_tree_1, obj_2, obj_3, u1_s1):
     with scope(space=obj_tree_1.space):
         # for some reason the 'path' attribute changes between the factory and the test when using both obj_tree and obj
         obj_tree_1 = Food.objects.get(id=obj_tree_1.id)
+        obj_2 = Food.objects.get(id=obj_2.id)
         parent = obj_tree_1.get_parent()
         obj_2.move(parent, node_location)
         obj_2 = Food.objects.get(id=obj_2.id)
-        parent = Food.objects.get(id=parent.id)
 
-    # should return full tree starting at parent (obj_tree_1, obj_2), ignoring query filters
-    response = json.loads(
-        u1_s1.get(f'{reverse(LIST_URL)}?tree={parent.id}').content)
+    # should return full tree starting at, but excluding parent (obj_tree_1, obj_2), ignoring query filters
+    response = json.loads(u1_s1.get(f'{reverse(LIST_URL)}?tree={parent.id}').content)
     assert response['count'] == 4
-    response = json.loads(u1_s1.get(
-        f'{reverse(LIST_URL)}?tree={parent.id}&query={obj_2.name[4:]}').content)
+    # filtering is ignored - should return identical results as ?tree=x
+    response = json.loads(u1_s1.get(f'{reverse(LIST_URL)}?tree={parent.id}&query={obj_2.name[4:]}').content)
     assert response['count'] == 4
 
 

cookbook/tests/api/test_api_supermarket.py

@@ -48,7 +48,7 @@ def test_list_filter(obj_1, obj_2, u1_s1):
     assert r.status_code == 200
     response = json.loads(r.content)
     assert len(response) == 2
-    assert response[0]['name'] == obj_1.name
+    # assert response[0]['name'] == obj_1.name   # assuming an order when it's not always valid
 
     response = json.loads(u1_s1.get(f'{reverse(LIST_URL)}?limit=1').content)
     assert len(response) == 1