From 7053a857c21f2adb2c61d566fade6936f6ed0c75 Mon Sep 17 00:00:00 2001 From: smilerz Date: Mon, 6 Sep 2021 10:20:43 -0500 Subject: [PATCH] reworked MergeMixin to handle non-trees --- cookbook/models.py | 3 +- cookbook/serializer.py | 28 +++- cookbook/templates/base.html | 29 +++- .../templates/forms/edit_internal_recipe.html | 2 +- cookbook/templates/shopping_list.html | 6 +- cookbook/tests/api/test_api_food.py | 3 +- cookbook/tests/api/test_api_unit.py | 129 ++++++++++++++++-- cookbook/urls.py | 4 +- cookbook/views/api.py | 21 +-- cookbook/views/lists.py | 17 +++ vue/src/components/GenericSplitLists.vue | 2 +- vue/src/locales/en.json | 3 +- vue/src/utils/models.js | 26 +++- 13 files changed, 230 insertions(+), 43 deletions(-) diff --git a/cookbook/models.py b/cookbook/models.py index e03c7461..0a239c87 100644 --- a/cookbook/models.py +++ b/cookbook/models.py @@ -104,8 +104,7 @@ class TreeModel(MP_Node): class Meta: abstract = True - - + class PermissionModelMixin: @staticmethod diff --git a/cookbook/serializer.py b/cookbook/serializer.py index 7e21e300..8bada6ca 100644 --- a/cookbook/serializer.py +++ b/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[ - 'request'].space.max_file_storage_mb != 0: + if ( + (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(), - space=self.context['request'].space) + validated_data['name'] = validated_data['name'].strip() + 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') - read_only_fields = ('id',) + fields = ('id', 'name', 'description', 'numrecipe', 'image') + read_only_fields = ('id', 'numrecipe', 'image') class SupermarketCategorySerializer(UniqueFieldsMixin, WritableNestedModelSerializer): diff --git a/cookbook/templates/base.html b/cookbook/templates/base.html index 45886ae7..97859839 100644 --- a/cookbook/templates/base.html +++ b/cookbook/templates/base.html @@ -97,6 +97,9 @@ {% trans 'Ingredients' %} + {% trans 'Units' %} + {% trans 'Supermarket' %} @@ -114,6 +117,28 @@ class="fas fa-edit fa-fw"> {% trans 'Batch Edit' %} + {% 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 %} +
  • + +
    + {% trans 'Ingredients' %} + + {% trans 'Units' %} + + {% trans 'Keyword' %} + {% comment %} does this funcionatliy need moved to the keyword list page? + the Recipe Search might be better? {% endcomment %} + {% trans 'Batch Edit' %} +
    +
  • {% trans 'Statistics' %} - {% trans 'Units & Ingredients' %} + {% comment %} {% trans 'Units & Ingredients' %} {% endcomment %} {% trans 'Import Recipe' %} diff --git a/cookbook/templates/forms/edit_internal_recipe.html b/cookbook/templates/forms/edit_internal_recipe.html index 4ba9235c..b14732fd 100644 --- a/cookbook/templates/forms/edit_internal_recipe.html +++ b/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) { diff --git a/cookbook/templates/shopping_list.html b/cookbook/templates/shopping_list.html index 0b8110f0..247b1440 100644 --- a/cookbook/templates/shopping_list.html +++ b/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') diff --git a/cookbook/tests/api/test_api_food.py b/cookbook/tests/api/test_api_food.py index eaaffb49..41b36d47 100644 --- a/cookbook/tests/api/test_api_food.py +++ b/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, diff --git a/cookbook/tests/api/test_api_unit.py b/cookbook/tests/api/test_api_unit.py index 2a97a39b..13d0aaec 100644 --- a/cookbook/tests/api/test_api_unit.py +++ b/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 len(json.loads(u1_s2.get(reverse(LIST_URL)).content)) == 0 + assert json.loads(u1_s1.get(reverse(LIST_URL)).content)['count'] == 2 + 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 len(json.loads(u1_s2.get(reverse(LIST_URL)).content)) == 1 + assert json.loads(u1_s1.get(reverse(LIST_URL)).content)['count'] == 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 - # unit model isn't sorted - this isn't a stable test - # assert response[0]['name'] == obj_1.name + assert response['count'] == 2 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[0]['name'] == obj_1.name + assert response['count'] == 1 + 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() == ([], [], [], [], []) diff --git a/cookbook/urls.py b/cookbook/urls.py index a0c1aab8..7ea95e99 100644 --- a/cookbook/urls.py +++ b/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('_', '-') diff --git a/cookbook/views/api.py b/cookbook/views/api.py index 59dd9daa..93ec08d2 100644 --- a/cookbook/views/api.py +++ b/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) - for c in children: - c.move(target, 'sorted-child') + if isTree: + children = source.get_children().exclude(id=target.id) + 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] - - def get_queryset(self): - self.queryset = self.queryset.filter(space=self.request.space) - return super().get_queryset() + pagination_class = DefaultPagination class FoodViewSet(viewsets.ModelViewSet, TreeMixin): diff --git a/cookbook/views/lists.py b/cookbook/views/lists.py index 1d751b69..8d1789f1 100644 --- a/cookbook/views/lists.py +++ b/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 + } + } + ) \ No newline at end of file diff --git a/vue/src/components/GenericSplitLists.vue b/vue/src/components/GenericSplitLists.vue index 4ed6dbe5..5648ff27 100644 --- a/vue/src/components/GenericSplitLists.vue +++ b/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 = { diff --git a/vue/src/locales/en.json b/vue/src/locales/en.json index bbf14fd8..40d795d9 100644 --- a/vue/src/locales/en.json +++ b/vue/src/locales/en.json @@ -122,5 +122,6 @@ "Description": "Description", "Recipe": "Recipe", "tree_root": "Root of Tree", - "Icon": "Icon" + "Icon": "Icon", + "Unit": "Unit" } diff --git a/vue/src/utils/models.js b/vue/src/utils/models.js index b014b4fd..a27c311c 100644 --- a/vue/src/utils/models.js +++ b/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 = {