From b153ddf858347e96c468eafc8bdb961af2667540 Mon Sep 17 00:00:00 2001
From: Michael Genson <71845777+michael-genson@users.noreply.github.com>
Date: Sat, 7 Oct 2023 16:06:00 -0500
Subject: [PATCH] feat: more shopping list enhancements (#2587)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* fix new position calculataion

* ensure consistent list item ordering

* fix recipe ref overflow on small screens

* added recipe ref elevation

* tweaked line height (for long notes)

* removed unused user dependency

* remove old shopping list items when there's >100

* 🤷

* cleaned up function generator

* fixed test

* fix potential type error

* made max position calc more efficient
---
 .../Recipe/RecipeIngredientListItem.vue       |  2 +-
 .../components/Domain/Recipe/RecipeList.vue   |  8 +-
 frontend/pages/shopping-lists/_id.vue         | 15 +++-
 mealie/app.py                                 |  1 +
 .../groups/controller_shopping_lists.py       |  4 +-
 .../services/group_services/shopping_lists.py |  5 +-
 mealie/services/scheduler/tasks/__init__.py   |  2 +
 .../delete_old_checked_shopping_list_items.py | 70 +++++++++++++++
 ..._delete_old_checked_shopping_list_items.py | 90 +++++++++++++++++++
 9 files changed, 189 insertions(+), 8 deletions(-)
 create mode 100644 mealie/services/scheduler/tasks/delete_old_checked_shopping_list_items.py
 create mode 100644 tests/unit_tests/services_tests/scheduler/tasks/test_delete_old_checked_shopping_list_items.py

diff --git a/frontend/components/Domain/Recipe/RecipeIngredientListItem.vue b/frontend/components/Domain/Recipe/RecipeIngredientListItem.vue
index bde6e75ba5a3..0e16277fbb3d 100644
--- a/frontend/components/Domain/Recipe/RecipeIngredientListItem.vue
+++ b/frontend/components/Domain/Recipe/RecipeIngredientListItem.vue
@@ -54,7 +54,7 @@ export default defineComponent({
 }
 
 .note {
-  line-height: 0.8em;
+  line-height: 1.25em;
   font-size: 0.8em;
   opacity: 0.7;
 }
diff --git a/frontend/components/Domain/Recipe/RecipeList.vue b/frontend/components/Domain/Recipe/RecipeList.vue
index 78f3c5e6b3b2..f4aac3012382 100644
--- a/frontend/components/Domain/Recipe/RecipeList.vue
+++ b/frontend/components/Domain/Recipe/RecipeList.vue
@@ -1,6 +1,12 @@
 <template>
   <v-list :class="tile ? 'd-flex flex-wrap background' : 'background'">
-    <v-sheet v-for="recipe, index in recipes" :key="recipe.id" :class="attrs.class.sheet" :style="tile ? 'width: fit-content;' : 'width: 100%;'">
+    <v-sheet
+      v-for="recipe, index in recipes"
+      :key="recipe.id"
+      :elevation="2"
+      :class="attrs.class.sheet"
+      :style="tile ? 'max-width: 100%; width: fit-content;' : 'width: 100%;'"
+    >
       <v-list-item :to="'/recipe/' + recipe.slug" :class="attrs.class.listItem">
         <v-list-item-avatar :class="attrs.class.avatar">
           <v-icon :class="attrs.class.icon" dark :small="small"> {{ $globals.icons.primary }} </v-icon>
diff --git a/frontend/pages/shopping-lists/_id.vue b/frontend/pages/shopping-lists/_id.vue
index 7452ecc8fe32..4b94f7a68484 100644
--- a/frontend/pages/shopping-lists/_id.vue
+++ b/frontend/pages/shopping-lists/_id.vue
@@ -268,6 +268,7 @@ export default defineComponent({
       // only update the list with the new value if we're not loading, to prevent UI jitter
       if (!loadingCounter.value) {
         shoppingList.value = newListValue;
+        sortListItems();
         updateItemsByLabel();
       }
     }
@@ -473,6 +474,15 @@ export default defineComponent({
 
     const itemsByLabel = ref<{ [key: string]: ShoppingListItemOut[] }>({});
 
+    function sortListItems() {
+      if (!shoppingList.value?.listItems?.length) {
+        return;
+      }
+
+      // sort by position ascending, then createdAt descending
+      shoppingList.value.listItems.sort((a, b) => (a.position > b.position || a.createdAt < b.createdAt ? 1 : -1))
+    }
+
     function updateItemsByLabel() {
       const items: { [prop: string]: ShoppingListItemOut[] } = {};
       const noLabelText = i18n.tc("shopping-list.no-label");
@@ -603,6 +613,7 @@ export default defineComponent({
         });
       }
 
+      sortListItems();
       updateItemsByLabel();
 
       loadingCounter.value += 1;
@@ -656,7 +667,9 @@ export default defineComponent({
       loadingCounter.value += 1;
 
       // make sure it's inserted into the end of the list, which may have been updated
-      createListItemData.value.position = shoppingList.value?.listItems?.length || 1;
+      createListItemData.value.position = shoppingList.value?.listItems?.length
+        ? (shoppingList.value.listItems.reduce((a, b) => (a.position || 0) > (b.position || 0) ? a : b).position || 0) + 1
+        : 0;
       const { data } = await userApi.shopping.items.createOne(createListItemData.value);
       loadingCounter.value -= 1;
 
diff --git a/mealie/app.py b/mealie/app.py
index e1874330c47b..850121a4da18 100644
--- a/mealie/app.py
+++ b/mealie/app.py
@@ -57,6 +57,7 @@ async def start_scheduler():
         tasks.purge_password_reset_tokens,
         tasks.purge_group_data_exports,
         tasks.create_mealplan_timeline_events,
+        tasks.delete_old_checked_list_items,
     )
 
     SchedulerRegistry.register_minutely(
diff --git a/mealie/routes/groups/controller_shopping_lists.py b/mealie/routes/groups/controller_shopping_lists.py
index bfe66680a40f..290c71d9c9a4 100644
--- a/mealie/routes/groups/controller_shopping_lists.py
+++ b/mealie/routes/groups/controller_shopping_lists.py
@@ -89,7 +89,7 @@ def publish_list_item_events(publisher: Callable, items_collection: ShoppingList
 class ShoppingListItemController(BaseCrudController):
     @cached_property
     def service(self):
-        return ShoppingListService(self.repos, self.user, self.group)
+        return ShoppingListService(self.repos, self.group)
 
     @cached_property
     def repo(self):
@@ -154,7 +154,7 @@ router = APIRouter(prefix="/groups/shopping/lists", tags=["Group: Shopping Lists
 class ShoppingListController(BaseCrudController):
     @cached_property
     def service(self):
-        return ShoppingListService(self.repos, self.user, self.group)
+        return ShoppingListService(self.repos, self.group)
 
     @cached_property
     def repo(self):
diff --git a/mealie/services/group_services/shopping_lists.py b/mealie/services/group_services/shopping_lists.py
index 1315e796ef6d..b477d9bb5669 100644
--- a/mealie/services/group_services/shopping_lists.py
+++ b/mealie/services/group_services/shopping_lists.py
@@ -19,13 +19,12 @@ from mealie.schema.group.group_shopping_list import (
 )
 from mealie.schema.recipe.recipe_ingredient import IngredientFood, IngredientUnit, RecipeIngredient
 from mealie.schema.response.pagination import OrderDirection, PaginationQuery
-from mealie.schema.user.user import GroupInDB, PrivateUser
+from mealie.schema.user.user import GroupInDB
 
 
 class ShoppingListService:
-    def __init__(self, repos: AllRepositories, user: PrivateUser, group: GroupInDB):
+    def __init__(self, repos: AllRepositories, group: GroupInDB):
         self.repos = repos
-        self.user = user
         self.group = group
         self.shopping_lists = repos.group_shopping_lists
         self.list_items = repos.group_shopping_list_item
diff --git a/mealie/services/scheduler/tasks/__init__.py b/mealie/services/scheduler/tasks/__init__.py
index 393b46f56fc4..2c7c850f7b29 100644
--- a/mealie/services/scheduler/tasks/__init__.py
+++ b/mealie/services/scheduler/tasks/__init__.py
@@ -1,4 +1,5 @@
 from .create_timeline_events import create_mealplan_timeline_events
+from .delete_old_checked_shopping_list_items import delete_old_checked_list_items
 from .post_webhooks import post_group_webhooks
 from .purge_group_exports import purge_group_data_exports
 from .purge_password_reset import purge_password_reset_tokens
@@ -7,6 +8,7 @@ from .reset_locked_users import locked_user_reset
 
 __all__ = [
     "create_mealplan_timeline_events",
+    "delete_old_checked_list_items",
     "post_group_webhooks",
     "purge_password_reset_tokens",
     "purge_group_data_exports",
diff --git a/mealie/services/scheduler/tasks/delete_old_checked_shopping_list_items.py b/mealie/services/scheduler/tasks/delete_old_checked_shopping_list_items.py
new file mode 100644
index 000000000000..cc532c8caa2a
--- /dev/null
+++ b/mealie/services/scheduler/tasks/delete_old_checked_shopping_list_items.py
@@ -0,0 +1,70 @@
+from collections.abc import Callable
+
+from pydantic import UUID4
+
+from mealie.db.db_setup import session_context
+from mealie.repos.all_repositories import get_repositories
+from mealie.routes.groups.controller_shopping_lists import publish_list_item_events
+from mealie.schema.response.pagination import OrderDirection, PaginationQuery
+from mealie.schema.user.user import DEFAULT_INTEGRATION_ID
+from mealie.services.event_bus_service.event_bus_service import EventBusService
+from mealie.services.event_bus_service.event_types import EventDocumentDataBase, EventTypes
+from mealie.services.group_services.shopping_lists import ShoppingListService
+
+MAX_CHECKED_ITEMS = 100
+
+
+def _create_publish_event(event_bus_service: EventBusService, group_id: UUID4):
+    def publish_event(event_type: EventTypes, document_data: EventDocumentDataBase, message: str = ""):
+        event_bus_service.dispatch(
+            integration_id=DEFAULT_INTEGRATION_ID,
+            group_id=group_id,
+            event_type=event_type,
+            document_data=document_data,
+            message=message,
+        )
+
+    return publish_event
+
+
+def _trim_list_items(shopping_list_service: ShoppingListService, shopping_list_id: UUID4, event_publisher: Callable):
+    pagination = PaginationQuery(
+        page=1,
+        per_page=-1,
+        query_filter=f'shopping_list_id="{shopping_list_id}" AND checked=true',
+        order_by="update_at",
+        order_direction=OrderDirection.desc,
+    )
+    query = shopping_list_service.list_items.page_all(pagination)
+    if len(query.items) <= MAX_CHECKED_ITEMS:
+        return
+
+    items_to_delete = query.items[MAX_CHECKED_ITEMS:]
+    items_response = shopping_list_service.bulk_delete_items([item.id for item in items_to_delete])
+    publish_list_item_events(event_publisher, items_response)
+
+
+def delete_old_checked_list_items(group_id: UUID4 | None = None):
+    with session_context() as session:
+        repos = get_repositories(session)
+        if group_id is None:
+            # if not specified, we check all groups
+            groups = repos.groups.page_all(PaginationQuery(page=1, per_page=-1)).items
+
+        else:
+            group = repos.groups.get_one(group_id)
+            if not group:
+                raise Exception(f'Group not found: "{group_id}"')
+
+            groups = [group]
+
+        for group in groups:
+            event_bus_service = EventBusService(session=session, group_id=group.id)
+            shopping_list_service = ShoppingListService(repos, group)
+            shopping_list_data = repos.group_shopping_lists.by_group(group.id).page_all(
+                PaginationQuery(page=1, per_page=-1)
+            )
+            for shopping_list in shopping_list_data.items:
+                _trim_list_items(
+                    shopping_list_service, shopping_list.id, _create_publish_event(event_bus_service, group.id)
+                )
diff --git a/tests/unit_tests/services_tests/scheduler/tasks/test_delete_old_checked_shopping_list_items.py b/tests/unit_tests/services_tests/scheduler/tasks/test_delete_old_checked_shopping_list_items.py
new file mode 100644
index 000000000000..b462745daba0
--- /dev/null
+++ b/tests/unit_tests/services_tests/scheduler/tasks/test_delete_old_checked_shopping_list_items.py
@@ -0,0 +1,90 @@
+from datetime import datetime
+
+from mealie.repos.repository_factory import AllRepositories
+from mealie.schema.group.group_shopping_list import ShoppingListItemCreate, ShoppingListItemOut, ShoppingListSave
+from mealie.services.scheduler.tasks.delete_old_checked_shopping_list_items import (
+    MAX_CHECKED_ITEMS,
+    delete_old_checked_list_items,
+)
+from tests.utils.factories import random_int, random_string
+from tests.utils.fixture_schemas import TestUser
+
+
+def test_cleanup(database: AllRepositories, unique_user: TestUser):
+    list_repo = database.group_shopping_lists.by_group(unique_user.group_id)
+    list_item_repo = database.group_shopping_list_item
+
+    shopping_list = list_repo.create(ShoppingListSave(name=random_string(), group_id=unique_user.group_id))
+    unchecked_items = list_item_repo.create_many(
+        [
+            ShoppingListItemCreate(note=random_string(), shopping_list_id=shopping_list.id)
+            for _ in range(random_int(MAX_CHECKED_ITEMS + 10, MAX_CHECKED_ITEMS + 20))
+        ]
+    )
+
+    # create them one at a time so the update timestamps are different
+    checked_items: list[ShoppingListItemOut] = []
+    for _ in range(random_int(MAX_CHECKED_ITEMS + 10, MAX_CHECKED_ITEMS + 20)):
+        new_item = list_item_repo.create(
+            ShoppingListItemCreate(note=random_string(), shopping_list_id=shopping_list.id)
+        )
+        new_item.checked = True
+        checked_items.append(list_item_repo.update(new_item.id, new_item))
+
+    # make sure we see all items
+    shopping_list = list_repo.get_one(shopping_list.id)  # type: ignore
+    assert shopping_list
+    assert len(shopping_list.list_items) == len(unchecked_items) + len(checked_items)
+    for item in unchecked_items + checked_items:
+        assert item in shopping_list.list_items
+
+    checked_items.sort(key=lambda x: x.update_at or datetime.now(), reverse=True)
+    expected_kept_items = unchecked_items + checked_items[:MAX_CHECKED_ITEMS]
+    expected_deleted_items = checked_items[MAX_CHECKED_ITEMS:]
+
+    # make sure we only see the expected items
+    delete_old_checked_list_items()
+    shopping_list = list_repo.get_one(shopping_list.id)  # type: ignore
+    assert shopping_list
+    assert len(shopping_list.list_items) == len(expected_kept_items)
+    for item in expected_kept_items:
+        assert item in shopping_list.list_items
+    for item in expected_deleted_items:
+        assert item not in shopping_list.list_items
+
+
+def test_no_cleanup(database: AllRepositories, unique_user: TestUser):
+    list_repo = database.group_shopping_lists.by_group(unique_user.group_id)
+    list_item_repo = database.group_shopping_list_item
+
+    shopping_list = list_repo.create(ShoppingListSave(name=random_string(), group_id=unique_user.group_id))
+    unchecked_items = list_item_repo.create_many(
+        [
+            ShoppingListItemCreate(note=random_string(), shopping_list_id=shopping_list.id)
+            for _ in range(MAX_CHECKED_ITEMS)
+        ]
+    )
+
+    # create them one at a time so the update timestamps are different
+    checked_items: list[ShoppingListItemOut] = []
+    for _ in range(MAX_CHECKED_ITEMS):
+        new_item = list_item_repo.create(
+            ShoppingListItemCreate(note=random_string(), shopping_list_id=shopping_list.id)
+        )
+        new_item.checked = True
+        checked_items.append(list_item_repo.update(new_item.id, new_item))
+
+    # make sure we see all items
+    shopping_list = list_repo.get_one(shopping_list.id)  # type: ignore
+    assert shopping_list
+    assert len(shopping_list.list_items) == len(unchecked_items) + len(checked_items)
+    for item in unchecked_items + checked_items:
+        assert item in shopping_list.list_items
+
+    # make sure we still see all items
+    delete_old_checked_list_items()
+    shopping_list = list_repo.get_one(shopping_list.id)  # type: ignore
+    assert shopping_list
+    assert len(shopping_list.list_items) == len(unchecked_items) + len(checked_items)
+    for item in unchecked_items + checked_items:
+        assert item in shopping_list.list_items