-
Notifications
You must be signed in to change notification settings - Fork 588
Feature/reorder ingredients countries 565 #571
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f81cf4d
dc511ba
a201113
c9fe925
067a4f2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| # Generated by Django 5.2.7 on 2025-10-27 08:11 | ||
|
|
||
| from django.db import migrations, models | ||
|
|
||
|
|
||
| class Migration(migrations.Migration): | ||
|
|
||
| dependencies = [ | ||
| ("breads", "0009_alter_breadpage_body"), | ||
| ] | ||
|
|
||
| operations = [ | ||
| migrations.AlterModelOptions( | ||
| name="breadingredient", | ||
| options={ | ||
| "ordering": ["sort_order", "name"], | ||
| "verbose_name": "bread ingredient", | ||
| "verbose_name_plural": "bread ingredients", | ||
| }, | ||
| ), | ||
| migrations.AlterModelOptions( | ||
| name="country", | ||
| options={ | ||
| "ordering": ["sort_order", "title"], | ||
| "verbose_name": "country of origin", | ||
| "verbose_name_plural": "countries of origin", | ||
| }, | ||
| ), | ||
| migrations.AddField( | ||
| model_name="breadingredient", | ||
| name="sort_order", | ||
| field=models.IntegerField(blank=True, editable=False, null=True), | ||
| ), | ||
| migrations.AddField( | ||
| model_name="country", | ||
| name="sort_order", | ||
| field=models.IntegerField(blank=True, db_index=True, null=True), | ||
| ), | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| # Generated by Django 5.2.7 on 2025-10-27 08:12 | ||
|
|
||
| from django.db import migrations | ||
|
|
||
|
|
||
| def backfill_country_sort_order(apps, schema_editor): | ||
| """ | ||
| Backfill sort_order for existing Country instances. | ||
| Assigns sort_order based on alphabetical order of title. | ||
| """ | ||
| Country = apps.get_model("breads", "Country") | ||
| for index, country in enumerate(Country.objects.order_by("title"), start=1): | ||
| country.sort_order = index * 10 # Use increments of 10 for flexibility | ||
| country.save(update_fields=["sort_order"]) | ||
|
|
||
|
|
||
| def backfill_ingredient_sort_order(apps, schema_editor): | ||
| """ | ||
| Backfill sort_order for existing BreadIngredient instances. | ||
| Assigns sort_order based on alphabetical order of name. | ||
| """ | ||
| BreadIngredient = apps.get_model("breads", "BreadIngredient") | ||
| for index, ingredient in enumerate( | ||
| BreadIngredient.objects.order_by("name"), start=1 | ||
| ): | ||
| ingredient.sort_order = index * 10 # Use increments of 10 for flexibility | ||
| ingredient.save(update_fields=["sort_order"]) | ||
|
|
||
|
|
||
| def reverse_backfill(apps, schema_editor): | ||
| """ | ||
| Reverse migration: set all sort_order values back to None. | ||
| """ | ||
| Country = apps.get_model("breads", "Country") | ||
| Country.objects.update(sort_order=None) | ||
|
|
||
| BreadIngredient = apps.get_model("breads", "BreadIngredient") | ||
| BreadIngredient.objects.update(sort_order=None) | ||
|
|
||
|
|
||
| class Migration(migrations.Migration): | ||
|
|
||
| dependencies = [ | ||
| ("breads", "0010_alter_breadingredient_options_alter_country_options_and_more"), | ||
| ] | ||
|
|
||
| operations = [ | ||
| migrations.RunPython(backfill_country_sort_order, reverse_backfill), | ||
| migrations.RunPython(backfill_ingredient_sort_order, migrations.RunPython.noop), | ||
| ] |
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn’t seem to do much to me. It feels like we’re testing basic Django behavior, not any custom logic. The only "custom" code, |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| from django.test import TestCase | ||
|
|
||
| from bakerydemo.breads.models import BreadIngredient, Country | ||
|
|
||
|
|
||
| class CountryModelTest(TestCase): | ||
| """Test Country model ordering and sort_order field.""" | ||
|
|
||
| def test_country_has_sort_order_field(self): | ||
| """Test that Country model has sort_order field.""" | ||
| country = Country(title="Test Country", sort_order=10) | ||
| country.save() | ||
| self.assertEqual(country.sort_order, 10) | ||
|
|
||
| def test_country_ordering_respects_sort_order(self): | ||
| """Test that Country model ordering includes sort_order.""" | ||
| Country.objects.create(title="Country A", sort_order=20) | ||
| Country.objects.create(title="Country B", sort_order=10) | ||
| Country.objects.create(title="Country C", sort_order=30) | ||
|
|
||
| countries = list(Country.objects.all()) | ||
| self.assertEqual(countries[0].title, "Country B") | ||
| self.assertEqual(countries[1].title, "Country A") | ||
| self.assertEqual(countries[2].title, "Country C") | ||
|
|
||
| def test_country_ordering_with_null_sort_order(self): | ||
| """Test that Country with null sort_order is ordered by title.""" | ||
| Country.objects.create(title="AAA", sort_order=None) | ||
| Country.objects.create(title="BBB", sort_order=10) | ||
| Country.objects.create(title="CCC", sort_order=None) | ||
|
|
||
| countries = list(Country.objects.all()) | ||
| # NULL sort_order values come first in SQL, then ordered by title | ||
| # Then items with sort_order | ||
| self.assertEqual(countries[0].title, "AAA") | ||
| self.assertEqual(countries[1].title, "CCC") | ||
| self.assertEqual(countries[2].title, "BBB") | ||
|
|
||
|
|
||
| class BreadIngredientModelTest(TestCase): | ||
| """Test BreadIngredient model ordering and sort_order field.""" | ||
|
|
||
| def test_ingredient_has_sort_order_field(self): | ||
| """Test that BreadIngredient model has sort_order field.""" | ||
| ingredient = BreadIngredient(name="Test Ingredient", sort_order=10) | ||
| ingredient.save() | ||
| self.assertEqual(ingredient.sort_order, 10) | ||
|
|
||
| def test_ingredient_ordering_respects_sort_order(self): | ||
| """Test that BreadIngredient model ordering includes sort_order.""" | ||
| BreadIngredient.objects.create(name="Ingredient A", sort_order=20) | ||
| BreadIngredient.objects.create(name="Ingredient B", sort_order=10) | ||
| BreadIngredient.objects.create(name="Ingredient C", sort_order=30) | ||
|
|
||
| ingredients = list(BreadIngredient.objects.all()) | ||
| self.assertEqual(ingredients[0].name, "Ingredient B") | ||
| self.assertEqual(ingredients[1].name, "Ingredient A") | ||
| self.assertEqual(ingredients[2].name, "Ingredient C") | ||
|
|
||
| def test_ingredient_ordering_with_null_sort_order(self): | ||
| """Test that BreadIngredient with null sort_order is ordered by name.""" | ||
| BreadIngredient.objects.create(name="AAA", sort_order=None) | ||
| BreadIngredient.objects.create(name="BBB", sort_order=10) | ||
| BreadIngredient.objects.create(name="CCC", sort_order=None) | ||
|
|
||
| ingredients = list(BreadIngredient.objects.all()) | ||
| # NULL sort_order values come first in SQL, then ordered by name | ||
| # Then items with sort_order | ||
| self.assertEqual(ingredients[0].name, "AAA") | ||
| self.assertEqual(ingredients[1].name, "CCC") | ||
| self.assertEqual(ingredients[2].name, "BBB") |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,10 +18,11 @@ class Meta: | |
|
|
||
| class BreadIngredientSnippetViewSet(SnippetViewSet): | ||
| model = BreadIngredient | ||
| ordering = ("name",) | ||
| ordering = ("sort_order", "name") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I’m not sure this is a good example. There’s no reason to order by both here, and the manual sort ordering feels like it should be optional within the CMS. |
||
| search_fields = ("name",) | ||
| filterset_class = BreadIngredientFilterSet | ||
| inspect_view_enabled = True | ||
| sort_order_field = "sort_order" | ||
|
|
||
|
|
||
| class BreadTypeFilterSet(RevisionFilterSetMixin, WagtailFilterSet): | ||
|
|
@@ -39,10 +40,11 @@ class BreadTypeSnippetViewSet(SnippetViewSet): | |
|
|
||
| class CountryModelViewSet(ModelViewSet): | ||
| model = Country | ||
| ordering = ("title",) | ||
| ordering = ("sort_order", "title") | ||
| search_fields = ("title",) | ||
| icon = "globe" | ||
| inspect_view_enabled = True | ||
| sort_order_field = "sort_order" | ||
|
|
||
| panels = [ | ||
| FieldPanel("title"), | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There’s no need for this