diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 62adcda0..f5d208d6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,11 +10,11 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout the repository - uses: actions/checkout@v2 + uses: actions/checkout@v4 with: path: ./src - name: Setup Python 3.11 - uses: actions/setup-python@v2 + uses: actions/setup-python@v5 with: python-version: '3.11' - name: Install all dependencies @@ -22,7 +22,7 @@ jobs: - name: Run all linting run: make lint - name: Upload src dir as artefact - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v4 with: name: src path: ./src @@ -38,12 +38,12 @@ jobs: oscar-version: ["3.2"] steps: - name: Download src dir - uses: actions/download-artifact@v2 + uses: actions/download-artifact@v4 with: name: src path: ./src - name: Setup Python 3.x - uses: actions/setup-python@v2 + uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} - name: Install all dependencies diff --git a/oscarapi/tests/unit/testproduct.py b/oscarapi/tests/unit/testproduct.py index 23c3902a..caa3fb29 100644 --- a/oscarapi/tests/unit/testproduct.py +++ b/oscarapi/tests/unit/testproduct.py @@ -2416,3 +2416,591 @@ def test_create_category_tree(self): }, ], ) + + def test_crazy_stuff(self): + Category.objects.all().delete() + data = [ + { + "data": {"code": "henk", "name": "Henk"}, + "children": [ + {"data": {"code": "klaas", "name": "Klaas"}}, + {"data": {"code": "klaas2", "name": "Klaas 2"}}, + {"data": {"code": "klaas3", "name": "Klaas 3"}}, + {"data": {"code": "klaas4", "name": "Klaas 4"}}, + {"data": {"code": "klaas5", "name": "Klaas 5"}}, + {"data": {"code": "klaas6", "name": "Klaas 6"}}, + ], + }, + { + "data": { + "code": "harrie", + "name": "Harrie", + "description": "Dit is de description", + }, + "children": [ + {"data": {"code": "koe", "name": "Koe"}}, + {"data": {"code": "koe2", "name": "Koe 2"}}, + {"data": {"code": "koe3", "name": "Koe 3"}}, + {"data": {"code": "koe4", "name": "Koe 4"}}, + {"data": {"code": "koe5", "name": "Koe 5"}}, + {"data": {"code": "koe6", "name": "Koe 6"}}, + ], + }, + ] + + self.response = self.post("admin-category-bulk", manual_data=data) + + self.assertEqual(Category.objects.count(), 14) + + self.assertEqual( + Category.dump_bulk(), + [ + { + "data": { + "name": "Henk", + "code": "henk", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "henk", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 1, + "children": [ + { + "data": { + "name": "Klaas", + "code": "klaas", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "klaas", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 2, + }, + { + "data": { + "name": "Klaas 2", + "code": "klaas2", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "klaas-2", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 3, + }, + { + "data": { + "name": "Klaas 3", + "code": "klaas3", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "klaas-3", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 4, + }, + { + "data": { + "name": "Klaas 4", + "code": "klaas4", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "klaas-4", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 5, + }, + { + "data": { + "name": "Klaas 5", + "code": "klaas5", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "klaas-5", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 6, + }, + { + "data": { + "name": "Klaas 6", + "code": "klaas6", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "klaas-6", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 7, + }, + ], + }, + { + "data": { + "name": "Harrie", + "code": "harrie", + "description": "Dit is de description", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "harrie", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 8, + "children": [ + { + "data": { + "name": "Koe", + "code": "koe", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "koe", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 9, + }, + { + "data": { + "name": "Koe 2", + "code": "koe2", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "koe-2", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 10, + }, + { + "data": { + "name": "Koe 3", + "code": "koe3", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "koe-3", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 11, + }, + { + "data": { + "name": "Koe 4", + "code": "koe4", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "koe-4", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 12, + }, + { + "data": { + "name": "Koe 5", + "code": "koe5", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "koe-5", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 13, + }, + { + "data": { + "name": "Koe 6", + "code": "koe6", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "koe-6", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 14, + }, + ], + }, + ], + ) + + data = [ + { + "data": {"code": "nieuwe1", "name": "Nieuwe 1"}, + "children": [ + {"data": {"code": "klaas", "name": "Klaas"}}, + {"data": {"code": "klaas2", "name": "Klaas 2"}}, + ], + }, + { + "data": {"code": "nieuwe2", "name": "Nieuwe 2"}, + "children": [ + {"data": {"code": "klaas3", "name": "Klaas 3"}}, + {"data": {"code": "klaas4", "name": "Klaas 4"}}, + ], + }, + { + "data": {"code": "nieuwe3", "name": "Nieuwe 3"}, + "children": [ + {"data": {"code": "klaas5", "name": "Klaas 5"}}, + {"data": {"code": "klaas6", "name": "Klaas 6"}}, + ], + }, + { + "data": {"code": "gekke1", "name": "Gekke 1"}, + "children": [ + {"data": {"code": "koe", "name": "Koe"}}, + {"data": {"code": "koe2", "name": "Koe 2"}}, + ], + }, + { + "data": {"code": "gekke2", "name": "Gekke 2"}, + "children": [ + {"data": {"code": "koe3", "name": "Koe 3"}}, + {"data": {"code": "koe4", "name": "Koe 4"}}, + ], + }, + { + "data": {"code": "gekke3", "name": "Gekke 3"}, + "children": [ + {"data": {"code": "koe5", "name": "Koe 5"}}, + {"data": {"code": "koe6", "name": "Koe 6"}}, + ], + }, + ] + + self.response = self.post("admin-category-bulk", manual_data=data) + + self.assertEqual(Category.objects.count(), 20) + + self.assertEqual( + Category.dump_bulk(), + [ + { + "data": { + "name": "Henk", + "code": "henk", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "henk", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 1, + }, + { + "data": { + "name": "Harrie", + "code": "harrie", + "description": "Dit is de description", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "harrie", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 8, + }, + { + "data": { + "name": "Nieuwe 1", + "code": "nieuwe1", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "nieuwe-1", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 15, + "children": [ + { + "data": { + "name": "Klaas", + "code": "klaas", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "klaas", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 2, + }, + { + "data": { + "name": "Klaas 2", + "code": "klaas2", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "klaas-2", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 3, + }, + ], + }, + { + "data": { + "name": "Nieuwe 2", + "code": "nieuwe2", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "nieuwe-2", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 16, + "children": [ + { + "data": { + "name": "Klaas 3", + "code": "klaas3", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "klaas-3", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 4, + }, + { + "data": { + "name": "Klaas 4", + "code": "klaas4", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "klaas-4", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 5, + }, + ], + }, + { + "data": { + "name": "Nieuwe 3", + "code": "nieuwe3", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "nieuwe-3", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 17, + "children": [ + { + "data": { + "name": "Klaas 5", + "code": "klaas5", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "klaas-5", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 6, + }, + { + "data": { + "name": "Klaas 6", + "code": "klaas6", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "klaas-6", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 7, + }, + ], + }, + { + "data": { + "name": "Gekke 1", + "code": "gekke1", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "gekke-1", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 18, + "children": [ + { + "data": { + "name": "Koe", + "code": "koe", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "koe", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 9, + }, + { + "data": { + "name": "Koe 2", + "code": "koe2", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "koe-2", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 10, + }, + ], + }, + { + "data": { + "name": "Gekke 2", + "code": "gekke2", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "gekke-2", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 19, + "children": [ + { + "data": { + "name": "Koe 3", + "code": "koe3", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "koe-3", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 11, + }, + { + "data": { + "name": "Koe 4", + "code": "koe4", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "koe-4", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 12, + }, + ], + }, + { + "data": { + "name": "Gekke 3", + "code": "gekke3", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "gekke-3", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 20, + "children": [ + { + "data": { + "name": "Koe 5", + "code": "koe5", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "koe-5", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 13, + }, + { + "data": { + "name": "Koe 6", + "code": "koe6", + "description": "", + "meta_title": None, + "meta_description": None, + "image": "", + "slug": "koe-6", + "is_public": True, + "ancestors_are_public": True, + }, + "id": 14, + }, + ], + }, + ], + ) diff --git a/oscarapi/utils/categories.py b/oscarapi/utils/categories.py index 1485b7a6..cca77e57 100644 --- a/oscarapi/utils/categories.py +++ b/oscarapi/utils/categories.py @@ -1,4 +1,5 @@ from django.utils.translation import gettext as _ +from django.db import transaction from rest_framework.exceptions import NotFound @@ -74,19 +75,22 @@ def find_from_full_slug(breadcrumb_str, separator="/"): def upsert_categories(data): - categories_to_update, fields_to_update = _upsert_categories(data) + with transaction.atomic(): + categories_to_update, fields_to_update = _upsert_categories(data) - if categories_to_update and fields_to_update: - Category.objects.bulk_update(categories_to_update, fields_to_update) + if categories_to_update and fields_to_update: + Category.objects.bulk_update(categories_to_update, fields_to_update) + + Category.fix_tree() def _upsert_categories(data, parent_category=None): if parent_category is None: - # Starting from root, we want the first category in the root - sibling = Category.get_first_root_node() + # Get the last category in the root + sibling = Category.get_last_root_node() else: - # We are further down the category tree, we want to get the first child from the parent - sibling = parent_category.get_first_child() + # Set sibling to None if there is a parent category, we want the first category in the data to be added as a first-child of the parent + sibling = None categories_to_update = [] category_fields_to_update = set() @@ -118,9 +122,12 @@ def _upsert_categories(data, parent_category=None): if (get_parent is None and parent_category is not None) or ( get_parent.pk != parent_category.pk ): - # Move the category as the first child under the parent category since we have not sibling + # Move the category as the first child under the parent category since we have no sibling category.move(parent_category, pos="first-child") + # Update the new path from the database after moving the category to it's new home + category.refresh_from_db(fields=["path"]) + # The category is now the sibling, new categories will be moved to the right of this category sibling = category