reworked MergeMixin to handle non-trees

cookbook/models.py

@@ -104,8 +104,7 @@ class TreeModel(MP_Node):
 
     class Meta:
         abstract = True
-        
+
-        
 
 class PermissionModelMixin:
     @staticmethod

cookbook/serializer.py

@@ -118,8 +118,10 @@ class UserFileSerializer(serializers.ModelSerializer):
         except TypeError:
             current_file_size_mb = 0
 
-        if (validated_data['file'].size / 1000 / 1000 + current_file_size_mb - 5) > self.context[
+        if (
-            'request'].space.max_file_storage_mb != 0:
+            (validated_data['file'].size / 1000 / 1000 + current_file_size_mb - 5)
+            > self.context['request'].space.max_file_storage_mb != 0
+        ):
             raise ValidationError(_('You have reached your file upload limit.'))
 
     def create(self, validated_data):
@@ -241,10 +243,24 @@ class KeywordSerializer(UniqueFieldsMixin, serializers.ModelSerializer):
 
 
 class UnitSerializer(UniqueFieldsMixin, serializers.ModelSerializer):
+    image = serializers.SerializerMethodField('get_image')
+    numrecipe = serializers.SerializerMethodField('count_recipes')
+
+    def get_image(self, obj):
+        recipes = Recipe.objects.filter(steps__ingredients__unit=obj, space=obj.space).exclude(image__isnull=True).exclude(image__exact='')
+
+        if len(recipes) != 0:
+            return random.choice(recipes).image.url
+        else:
+            return None
+
+    def count_recipes(self, obj):
+        return Recipe.objects.filter(steps__ingredients__unit=obj, space=obj.space).count()
 
     def create(self, validated_data):
-        obj, created = Unit.objects.get_or_create(name=validated_data['name'].strip(),
+        validated_data['name'] = validated_data['name'].strip()
-                                                  space=self.context['request'].space)
+        validated_data['space'] = self.context['request'].space
+        obj, created = Unit.objects.get_or_create(**validated_data)
         return obj
 
     def update(self, instance, validated_data):
@@ -253,8 +269,8 @@ class UnitSerializer(UniqueFieldsMixin, serializers.ModelSerializer):
 
     class Meta:
         model = Unit
-        fields = ('id', 'name', 'description')
+        fields = ('id', 'name', 'description', 'numrecipe', 'image')
-        read_only_fields = ('id',)
+        read_only_fields = ('id', 'numrecipe', 'image')
 
 
 class SupermarketCategorySerializer(UniqueFieldsMixin, WritableNestedModelSerializer):

cookbook/templates/base.html

@@ -97,6 +97,9 @@
                     <a class="dropdown-item" href="{% url 'list_food' %}"><i
                             class="fas fa-leaf fa-fw"></i> {% trans 'Ingredients' %}
                     </a>
+                    <a class="dropdown-item" href="{% url 'list_unit' %}"><i
+                            class="fas fa-balance-scale fa-fw"></i> {% trans 'Units' %}
+                    </a>
                     <a class="dropdown-item" href="{% url 'view_supermarket' %}"><i
                             class="fas fa-store-alt fa-fw"></i> {% trans 'Supermarket' %}
                     </a>
@@ -114,6 +117,28 @@
                             class="fas fa-edit fa-fw"></i> {% trans 'Batch Edit' %}</a>
                 </div>
             </li>
+            {% comment %} should food and units be moved to the keyword dropdown and rename that?
+                    feels like the original intent of the organization of these menus has shifted maybe something like this?{% endcomment %}
+            <li class="nav-item dropdown {% if request.resolver_match.url_name in 'list_food,list_unit,list_keyword,data_batch_edit' %}active{% endif %}">
+                <a class="nav-link dropdown-toggle" href="#" id="navbarDropdownMenuLink" data-toggle="dropdown"
+                   aria-haspopup="true" aria-expanded="false">
+                    <i class="fas fa-blender"></i> {% trans 'Gadgets' %}
+                </a>
+                <div class="dropdown-menu" aria-labelledby="navbarDropdownMenuLink">
+                    <a class="dropdown-item" href="{% url 'list_food' %}"><i
+                            class="fas fa-leaf fa-fw"></i> {% trans 'Ingredients' %}
+                    </a>
+                    <a class="dropdown-item" href="{% url 'list_unit' %}"><i
+                            class="fas fa-balance-scale fa-fw"></i> {% trans 'Units' %}
+                    </a>
+                    <a class="dropdown-item" href="{% url 'list_keyword' %}"><i
+                            class="fas fa-tags fa-fw"></i> {% trans 'Keyword' %}</a>
+                    {% comment %} does this funcionatliy need moved to the keyword list page? 
+                    the Recipe Search might be better? {% endcomment %}
+                    <a class="dropdown-item" href="{% url 'data_batch_edit' %}"><i
+                            class="fas fa-edit fa-fw"></i> {% trans 'Batch Edit' %}</a>
+                </div>
+            </li>
             <li class="nav-item dropdown {% if request.resolver_match.url_name in 'list_storage,data_sync,list_recipe_import,list_sync_log,data_stats,edit_food,edit_storage' %}active{% endif %}">
                 <a class="nav-link dropdown-toggle" href="#" id="navbarDropdownMenuLink" data-toggle="dropdown"
                    aria-haspopup="true" aria-expanded="false"><i class="fas fa-database"></i> {% trans 'Storage Data' %}
@@ -129,8 +154,8 @@
                             class="fas fa-history fa-fw"></i> {% trans 'Discovery Log' %}</a>
                     <a class="dropdown-item" href="{% url 'data_stats' %}"><i
                             class="fas fa-chart-line fa-fw"></i> {% trans 'Statistics' %}</a>
-                    <a class="dropdown-item" href="{% url 'edit_food' %}"><i
+                    {% comment %} <a class="dropdown-item" href="{% url 'edit_food' %}"><i
-                            class="fas fa-balance-scale fa-fw"></i> {% trans 'Units & Ingredients' %}</a>
+                            class="fas fa-balance-scale fa-fw"></i> {% trans 'Units & Ingredients' %}</a> {% endcomment %}
                     <a class="dropdown-item" href="{% url 'data_import_url' %}"><i
                             class="fas fa-file-import"></i> {% trans 'Import Recipe' %}</a>
 

cookbook/templates/forms/edit_internal_recipe.html

@@ -760,7 +760,7 @@
                 searchUnits: function (query) {
                     this.units_loading = true
                     this.$http.get("{% url 'api:unit-list' %}" + '?query=' + query + '&limit=10').then((response) => {
-                        this.units = response.data;
+                        this.units = response.data.results;
 
                         if (this.recipe !== undefined) {
                             for (let s of this.recipe.steps) {

cookbook/templates/shopping_list.html

@@ -908,7 +908,7 @@
                 searchKeywords: function (query) {
                     this.keywords_loading = true
                     this.$http.get("{% url 'api:keyword-list' %}" + '?query=' + query + '&limit=10').then((response) => {
-                        this.keywords = response.data;
+                        this.keywords = response.data.results;
                         this.keywords_loading = false
                     }).catch((err) => {
                         console.log(err)
@@ -919,7 +919,7 @@
                 searchUnits: function (query) { //TODO move to central component
                     this.units_loading = true
                     this.$http.get("{% url 'api:unit-list' %}" + '?query=' + query + '&limit=10').then((response) => {
-                        this.units = response.data;
+                        this.units = response.data.results;
                         this.units_loading = false
                     }).catch((err) => {
                         this.makeToast(gettext('Error'), gettext("There was an error loading a resource!") + err.bodyText, 'danger')
@@ -928,7 +928,7 @@
                 searchFoods: function (query) { //TODO move to central component
                     this.foods_loading = true
                     this.$http.get("{% url 'api:food-list' %}" + '?query=' + query + '&limit=10').then((response) => {
-                        this.foods = response.data
+                        this.foods = response.data.results
                         this.foods_loading = false
                     }).catch((err) => {
                         this.makeToast(gettext('Error'), gettext("There was an error loading a resource!") + err.bodyText, 'danger')

cookbook/tests/api/test_api_food.py

@@ -273,7 +273,7 @@ def test_integrity(u1_s1, recipe_1_s1):
         )
     )
     assert r.status_code == 204
-    
+
     with scopes_disabled():
         assert Food.objects.count() == 9
         assert Ingredient.objects.count() == 9
@@ -327,7 +327,6 @@ def test_move(u1_s1, obj_1, obj_1_1, obj_1_1_1, obj_2, obj_3, space_1):
         assert Food.find_problems() == ([], [], [], [], [])
 
 
-# this seems overly long - should it be broken into smaller pieces?
 def test_merge(
     u1_s1,
     obj_1, obj_1_1, obj_1_1_1, obj_2, obj_3,

cookbook/tests/api/test_api_unit.py

@@ -1,13 +1,20 @@
 import json
-
 import pytest
+import uuid
+
+from django.contrib import auth
 from django.urls import reverse
 from django_scopes import scopes_disabled
 
-from cookbook.models import Food, Unit
+from cookbook.models import Food, Ingredient, ShoppingList, ShoppingListEntry, Unit
 
 LIST_URL = 'api:unit-list'
 DETAIL_URL = 'api:unit-detail'
+MERGE_URL = 'api:unit-merge'
+
+
+def random_food(space_1, u1_s1):
+    return Food.objects.get_or_create(name=str(uuid.uuid4()), space=space_1)[0]
 
 
 @pytest.fixture()
@@ -20,6 +27,47 @@ def obj_2(space_1):
     return Unit.objects.get_or_create(name='test_2', space=space_1)[0]
 
 
+@pytest.fixture
+def obj_3(space_2):
+    return Unit.objects.get_or_create(name='test_3', space=space_2)[0]
+
+
+@pytest.fixture()
+def ing_1_s1(obj_1, space_1, u1_s1):
+    return Ingredient.objects.create(unit=obj_1, food=random_food(space_1, u1_s1), space=space_1)
+
+
+@pytest.fixture()
+def ing_2_s1(obj_2, space_1, u1_s1):
+    return Ingredient.objects.create(unit=obj_2, food=random_food(space_1, u1_s1), space=space_1)
+
+
+@pytest.fixture()
+def ing_3_s2(obj_3, space_2, u2_s2):
+    return Ingredient.objects.create(unit=obj_3, food=random_food(space_2, u2_s2), space=space_2)
+
+
+@pytest.fixture()
+def sle_1_s1(obj_1, u1_s1, space_1):
+    e = ShoppingListEntry.objects.create(unit=obj_1, food=random_food(space_1, u1_s1))
+    s = ShoppingList.objects.create(created_by=auth.get_user(u1_s1), space=space_1, )
+    s.entries.add(e)
+    return e
+
+
+@pytest.fixture()
+def sle_2_s1(obj_2, u1_s1, space_1):
+    return ShoppingListEntry.objects.create(unit=obj_2, food=random_food(space_1, u1_s1))
+
+
+@pytest.fixture()
+def sle_3_s2(obj_3, u2_s2, space_2):
+    e = ShoppingListEntry.objects.create(unit=obj_3, food=random_food(space_2, u2_s2))
+    s = ShoppingList.objects.create(created_by=auth.get_user(u2_s2), space=space_2)
+    s.entries.add(e)
+    return e
+
+
 @pytest.mark.parametrize("arg", [
     ['a_u', 403],
     ['g1_s1', 403],
@@ -32,33 +80,31 @@ def test_list_permission(arg, request):
 
 
 def test_list_space(obj_1, obj_2, u1_s1, u1_s2, space_2):
-    assert len(json.loads(u1_s1.get(reverse(LIST_URL)).content)) == 2
+    assert json.loads(u1_s1.get(reverse(LIST_URL)).content)['count'] == 2
-    assert len(json.loads(u1_s2.get(reverse(LIST_URL)).content)) == 0
+    assert json.loads(u1_s2.get(reverse(LIST_URL)).content)['count'] == 0
 
     obj_1.space = space_2
     obj_1.save()
 
-    assert len(json.loads(u1_s1.get(reverse(LIST_URL)).content)) == 1
+    assert json.loads(u1_s1.get(reverse(LIST_URL)).content)['count'] == 1
-    assert len(json.loads(u1_s2.get(reverse(LIST_URL)).content)) == 1
+    assert json.loads(u1_s2.get(reverse(LIST_URL)).content)['count'] == 1
 
 
 def test_list_filter(obj_1, obj_2, u1_s1):
     r = u1_s1.get(reverse(LIST_URL))
     assert r.status_code == 200
     response = json.loads(r.content)
-    assert len(response) == 2
+    assert response['count'] == 2
-    # unit model isn't sorted - this isn't a stable test
-    # assert response[0]['name'] == obj_1.name
 
     response = json.loads(u1_s1.get(f'{reverse(LIST_URL)}?limit=1').content)
-    assert len(response) == 1
+    assert response['count'] == 1
 
     response = json.loads(u1_s1.get(f'{reverse(LIST_URL)}?query=chicken').content)
-    assert len(response) == 0
+    assert response['count'] == 0
 
     response = json.loads(u1_s1.get(f'{reverse(LIST_URL)}?query={obj_1.name[4:]}').content)
-    assert len(response) == 1
+    assert response['count'] == 1
-    assert response[0]['name'] == obj_1.name
+    assert response['results'][0]['name'] == obj_1.name
 
 
 @pytest.mark.parametrize("arg", [
@@ -148,3 +194,60 @@ def test_delete(u1_s1, u1_s2, obj_1):
     assert r.status_code == 204
     with scopes_disabled():
         assert Food.objects.count() == 0
+
+
+def test_merge(
+    u1_s1,
+    obj_1, obj_2, obj_3,
+    ing_1_s1, ing_2_s1, ing_3_s2,
+    sle_1_s1, sle_2_s1, sle_3_s2,
+    space_1
+):
+    with scopes_disabled():
+        assert Unit.objects.filter(space=space_1).count() == 2
+        assert obj_1.ingredient_set.count() == 1
+        assert obj_2.ingredient_set.count() == 1
+        assert obj_3.ingredient_set.count() == 1
+        assert obj_1.shoppinglistentry_set.count() == 1
+        assert obj_2.shoppinglistentry_set.count() == 1
+        assert obj_3.shoppinglistentry_set.count() == 1
+
+    # merge Unit with ingredient/shopping list entry with another Unit, only HTTP put method should work
+    url = reverse(MERGE_URL, args=[obj_1.id, obj_2.id])
+    r = u1_s1.get(url)
+    assert r.status_code == 405
+    r = u1_s1.post(url)
+    assert r.status_code == 405
+    r = u1_s1.delete(url)
+    assert r.status_code == 405
+    r = u1_s1.put(url)
+    assert r.status_code == 200
+    with scopes_disabled():
+        assert Unit.objects.filter(pk=obj_1.id).count() == 0
+        assert obj_2.ingredient_set.count() == 2
+        assert obj_3.ingredient_set.count() == 1
+
+        assert obj_2.shoppinglistentry_set.count() == 2
+        assert obj_3.shoppinglistentry_set.count() == 1
+
+    # attempt to merge with non-existent parent
+    r = u1_s1.put(
+        reverse(MERGE_URL, args=[obj_2.id, 9999])
+    )
+    assert r.status_code == 404
+
+    # attempt to move to wrong space
+    r = u1_s1.put(
+        reverse(MERGE_URL, args=[obj_2.id, obj_3.id])
+    )
+    assert r.status_code == 404
+
+    # attempt to merge with self
+    r = u1_s1.put(
+        reverse(MERGE_URL, args=[obj_2.id, obj_2.id])
+    )
+    assert r.status_code == 403
+
+    # run diagnostic to find problems - none should be found
+    with scopes_disabled():
+        assert Food.find_problems() == ([], [], [], [], [])

cookbook/urls.py

@@ -10,7 +10,7 @@ from cookbook.helper import dal
 
 from .models import (Comment, Food, InviteLink, Keyword, MealPlan, Recipe,
                      RecipeBook, RecipeBookEntry, RecipeImport, ShoppingList,
-                     Storage, Sync, SyncLog, get_model_name)
+                     Storage, Sync, SyncLog, Unit, get_model_name)
 from .views import api, data, delete, edit, import_export, lists, new, views, telegram
 
 router = routers.DefaultRouter()
@@ -176,7 +176,7 @@ for m in generic_models:
             )
         )
 
-vue_models = [Keyword, Food]
+vue_models = [Food, Keyword, Unit]
 for m in vue_models:
     py_name = get_model_name(m)
     url_name = py_name.replace('_', '-')

cookbook/views/api.py

@@ -119,7 +119,6 @@ class FuzzyFilterMixin(ViewSetMixin):
                     .filter(name__icontains=query).order_by('-exact')
                 )
 
-
         updated_at = self.request.query_params.get('updated_at', None)
         if updated_at is not None:
             try:
@@ -165,7 +164,12 @@ class MergeMixin(ViewSetMixin):  # TODO update Units to use merge API
                 if target in source.get_descendants_and_self():
                     content = {'error': True, 'msg': _('Cannot merge with child object!')}
                     return Response(content, status=status.HTTP_403_FORBIDDEN)
+                isTree = True
+            except AttributeError:
+                # AttributeError probably means its not a tree, so can safely ignore
+                isTree = False
 
+            try:
                 for link in [field for field in source._meta.get_fields() if issubclass(type(field), ForeignObjectRel)]:
                     linkManager = getattr(source, link.get_accessor_name())
                     related = linkManager.all()
@@ -182,9 +186,10 @@ class MergeMixin(ViewSetMixin):  # TODO update Units to use merge API
                     else:
                         # a new scenario exists and needs to be handled
                         raise NotImplementedError
-                children = source.get_children().exclude(id=target.id)
+                if isTree:
-                for c in children:
+                    children = source.get_children().exclude(id=target.id)
-                    c.move(target, 'sorted-child')
+                    for c in children:
+                        c.move(target, 'sorted-child')
                 content = {'msg': _(f'{source.name} was merged successfully with {target.name}')}
                 source.delete()
                 return Response(content, status=status.HTTP_200_OK)
@@ -363,14 +368,12 @@ class KeywordViewSet(viewsets.ModelViewSet, TreeMixin):
     pagination_class = DefaultPagination
 
 
-class UnitViewSet(viewsets.ModelViewSet, FuzzyFilterMixin):
+class UnitViewSet(viewsets.ModelViewSet, MergeMixin, FuzzyFilterMixin):
     queryset = Unit.objects
+    model = Unit
     serializer_class = UnitSerializer
     permission_classes = [CustomIsUser]
-
+    pagination_class = DefaultPagination
-    def get_queryset(self):
-        self.queryset = self.queryset.filter(space=self.request.space)
-        return super().get_queryset()
 
 
 class FoodViewSet(viewsets.ModelViewSet, TreeMixin):

cookbook/views/lists.py

@@ -133,3 +133,20 @@ def food(request):
             }
         }
     )
+
+
+@group_required('user')
+def unit(request):
+    # recipe-param is the name of the parameters used when filtering recipes by this attribute
+    # model-name is the models.js name of the model, probably ALL-CAPS
+    return render(
+        request,
+        'generic/model_template.html',
+        {
+            "title": _("Units"),
+            "config": {
+                'model': "UNIT",         # *REQUIRED* name of the model in models.js
+                # 'recipe_param': 'units'  # *OPTIONAL* name of the listRecipes parameter if filtering on this attribute
+            }
+        }
+    )

vue/src/components/GenericSplitLists.vue

@@ -176,7 +176,7 @@ export default {
       this.left_page = 0
       this.right += 1
       this.left += 1
-      this.$emit('reset')
+      // this.$emit('reset')  doublecheck if this is necessary
     },
     infiniteHandler: function($state, col) { 
         let params = {

vue/src/locales/en.json

@@ -122,5 +122,6 @@
   "Description": "Description",
   "Recipe": "Recipe",
   "tree_root": "Root of Tree",
-  "Icon": "Icon"
+  "Icon": "Icon",
+  "Unit": "Unit"
 }

vue/src/utils/models.js

@@ -136,7 +136,31 @@ export class Models {
             }
         },
     }
-    static UNIT = {}
+    static UNIT = {
+        'name': i18n.t('Unit'),              // *OPTIONAL: parameters will be built model -> model_type -> default
+        'apiName': 'Unit', 
+        'create': {
+            // if not defined partialUpdate will use the same parameters, prepending 'id'
+            'params': [['name', 'description']],
+            'form': {
+                'name': {
+                    'form_field': true,
+                    'type': 'text',
+                    'field': 'name',
+                    'label': i18n.t('Name'),
+                    'placeholder': ''
+                },
+                'description': {
+                    'form_field': true,
+                    'type': 'text',
+                    'field': 'description',
+                    'label': i18n.t('Description'),
+                    'placeholder': ''
+                }
+            }
+        },
+        'move': false
+    }
     static RECIPE = {}
     static SHOPPING_LIST = {}
     static RECIPE_BOOK = {