From 095edef95e81f0f51fa5b9a0b4b3ced769878a82 Mon Sep 17 00:00:00 2001 From: Michael Genson <71845777+michael-genson@users.noreply.github.com> Date: Sun, 20 Aug 2023 13:38:46 -0500 Subject: [PATCH] feat: Improve Public URL Readability (#2482) * added support for group slugs * modified frontend to use links with group slug * fixed test refs * unused import --------- Co-authored-by: Hayden <64056131+hay-kot@users.noreply.github.com> --- ...-21.00.34_04ac51cbe9a4_added_group_slug.py | 56 +++++++++++++++++++ .../Domain/Recipe/RecipeContextMenu.vue | 25 ++++++++- frontend/lib/api/public/explore.ts | 6 +- frontend/lib/api/types/user.ts | 1 + .../{_groupId => _groupSlug}/_slug.vue | 4 +- mealie/db/models/group/group.py | 1 + mealie/repos/repository_group.py | 44 ++++++++++++++- .../explore/controller_public_recipes.py | 9 ++- mealie/schema/user/user.py | 1 + .../public_recipe_tests.py | 7 ++- .../repository_tests/test_group_repository.py | 24 ++++++++ tests/utils/api_routes/__init__.py | 6 +- 12 files changed, 166 insertions(+), 18 deletions(-) create mode 100644 alembic/versions/2023-08-06-21.00.34_04ac51cbe9a4_added_group_slug.py rename frontend/pages/explore/recipes/{_groupId => _groupSlug}/_slug.vue (90%) create mode 100644 tests/unit_tests/repository_tests/test_group_repository.py diff --git a/alembic/versions/2023-08-06-21.00.34_04ac51cbe9a4_added_group_slug.py b/alembic/versions/2023-08-06-21.00.34_04ac51cbe9a4_added_group_slug.py new file mode 100644 index 0000000000..9d846adf4d --- /dev/null +++ b/alembic/versions/2023-08-06-21.00.34_04ac51cbe9a4_added_group_slug.py @@ -0,0 +1,56 @@ +"""added group slug + +Revision ID: 04ac51cbe9a4 +Revises: b3dbb554ba53 +Create Date: 2023-08-06 21:00:34.582905 + +""" +import sqlalchemy as sa +from slugify import slugify +from sqlalchemy.orm import Session + +from alembic import op +from mealie.db.models.group.group import Group + +# revision identifiers, used by Alembic. +revision = "04ac51cbe9a4" +down_revision = "b3dbb554ba53" +branch_labels = None +depends_on = None + + +def populate_group_slugs(session: Session): + groups: list[Group] = session.query(Group).all() + seen_slugs: set[str] = set() + for group in groups: + original_name = group.name + attempts = 0 + while True: + slug = slugify(group.name) + if slug not in seen_slugs: + break + + attempts += 1 + group.name = f"{original_name} ({attempts})" + + seen_slugs.add(slug) + group.slug = slug + + session.commit() + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column("groups", sa.Column("slug", sa.String(), nullable=True)) + op.create_index(op.f("ix_groups_slug"), "groups", ["slug"], unique=True) + # ### end Alembic commands ### + + session = Session(bind=op.get_bind()) + populate_group_slugs(session) + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_index(op.f("ix_groups_slug"), table_name="groups") + op.drop_column("groups", "slug") + # ### end Alembic commands ### diff --git a/frontend/components/Domain/Recipe/RecipeContextMenu.vue b/frontend/components/Domain/Recipe/RecipeContextMenu.vue index 942e4600fa..a5016bd753 100644 --- a/frontend/components/Domain/Recipe/RecipeContextMenu.vue +++ b/frontend/components/Domain/Recipe/RecipeContextMenu.vue @@ -496,6 +496,22 @@ export default defineComponent({ } const { copyText } = useCopy(); + const groupSlug = ref(""); + + async function setGroupSlug() { + if (!props.groupId) { + groupSlug.value = props.groupId; + return; + } + + const {data} = await api.groups.getOne(props.groupId); + if (!data) { + groupSlug.value = props.groupId; + return; + } + + groupSlug.value = data.slug; + } // Note: Print is handled as an event in the parent component const eventHandlers: { [key: string]: () => void | Promise } = { @@ -525,13 +541,18 @@ export default defineComponent({ share: () => { state.shareDialog = true; }, - publicUrl: () => { + publicUrl: async () => { if (!props.groupId) { alert.error("Unknown group ID"); console.error("prop `groupId` is required when requesting a public URL"); return; } - copyText(`${window.location.origin}/explore/recipes/${props.groupId}/${props.slug}`); + + if (!groupSlug.value) { + await setGroupSlug(); + } + + copyText(`${window.location.origin}/explore/recipes/${groupSlug.value}/${props.slug}`); }, }; diff --git a/frontend/lib/api/public/explore.ts b/frontend/lib/api/public/explore.ts index e33276a591..a750e42a16 100644 --- a/frontend/lib/api/public/explore.ts +++ b/frontend/lib/api/public/explore.ts @@ -4,11 +4,11 @@ import { Recipe } from "~/lib/api/types/recipe"; const prefix = "/api"; const routes = { - recipe: (groupId: string, recipeSlug: string) => `${prefix}/explore/recipes/${groupId}/${recipeSlug}`, + recipe: (groupSlug: string, recipeSlug: string) => `${prefix}/explore/recipes/${groupSlug}/${recipeSlug}`, }; export class ExploreApi extends BaseAPI { - async recipe(groupId: string, recipeSlug: string) { - return await this.requests.get(routes.recipe(groupId, recipeSlug)); + async recipe(groupSlug: string, recipeSlug: string) { + return await this.requests.get(routes.recipe(groupSlug, recipeSlug)); } } diff --git a/frontend/lib/api/types/user.ts b/frontend/lib/api/types/user.ts index 26e607beb5..d7e6d5dfcb 100644 --- a/frontend/lib/api/types/user.ts +++ b/frontend/lib/api/types/user.ts @@ -41,6 +41,7 @@ export interface GroupBase { export interface GroupInDB { name: string; id: string; + slug: string; categories?: CategoryBase[]; webhooks?: unknown[]; users?: UserOut[]; diff --git a/frontend/pages/explore/recipes/_groupId/_slug.vue b/frontend/pages/explore/recipes/_groupSlug/_slug.vue similarity index 90% rename from frontend/pages/explore/recipes/_groupId/_slug.vue rename to frontend/pages/explore/recipes/_groupSlug/_slug.vue index 10e7f1d441..70f197a9e0 100644 --- a/frontend/pages/explore/recipes/_groupId/_slug.vue +++ b/frontend/pages/explore/recipes/_groupSlug/_slug.vue @@ -18,7 +18,7 @@ export default defineComponent({ setup() { const route = useRoute(); const router = useRouter(); - const groupId = route.value.params.groupId; + const groupSlug = route.value.params.groupSlug; const slug = route.value.params.slug; const api = usePublicApi(); @@ -26,7 +26,7 @@ export default defineComponent({ const { recipeMeta } = useRecipeMeta(); const recipe = useAsync(async () => { - const { data, error } = await api.explore.recipe(groupId, slug); + const { data, error } = await api.explore.recipe(groupSlug, slug); if (error) { console.error("error loading recipe -> ", error); diff --git a/mealie/db/models/group/group.py b/mealie/db/models/group/group.py index fd6ea81445..a8bf0e5b75 100644 --- a/mealie/db/models/group/group.py +++ b/mealie/db/models/group/group.py @@ -32,6 +32,7 @@ class Group(SqlAlchemyBase, BaseMixins): __tablename__ = "groups" id: Mapped[GUID] = mapped_column(GUID, primary_key=True, default=GUID.generate) name: Mapped[str] = mapped_column(sa.String, index=True, nullable=False, unique=True) + slug: Mapped[str | None] = mapped_column(sa.String, index=True, unique=True) users: Mapped[list["User"]] = orm.relationship("User", back_populates="group") categories: Mapped[Category] = orm.relationship( Category, secondary=group_to_categories, single_parent=True, uselist=True diff --git a/mealie/repos/repository_group.py b/mealie/repos/repository_group.py index e46c132e93..d153f67491 100644 --- a/mealie/repos/repository_group.py +++ b/mealie/repos/repository_group.py @@ -1,5 +1,11 @@ +from collections.abc import Iterable +from typing import cast +from uuid import UUID + from pydantic import UUID4 +from slugify import slugify from sqlalchemy import func, select +from sqlalchemy.exc import IntegrityError from mealie.db.models.group import Group from mealie.db.models.recipe.category import Category @@ -8,19 +14,55 @@ from mealie.db.models.recipe.tool import Tool from mealie.db.models.users.users import User from mealie.schema.group.group_statistics import GroupStatistics -from mealie.schema.user.user import GroupInDB +from mealie.schema.user.user import GroupBase, GroupInDB from ..db.models._model_base import SqlAlchemyBase from .repository_generic import RepositoryGeneric class RepositoryGroup(RepositoryGeneric[GroupInDB, Group]): + def create(self, data: GroupBase | dict) -> GroupInDB: + if isinstance(data, GroupBase): + data = data.dict() + + max_attempts = 10 + original_name = cast(str, data["name"]) + + attempts = 0 + while True: + try: + data["slug"] = slugify(data["name"]) + return super().create(data) + except IntegrityError: + self.session.rollback() + attempts += 1 + if attempts >= max_attempts: + raise + + data["name"] = f"{original_name} ({attempts})" + + def create_many(self, data: Iterable[GroupInDB | dict]) -> list[GroupInDB]: + # since create uses special logic for resolving slugs, we don't want to use the standard create_many method + return [self.create(new_group) for new_group in data] + def get_by_name(self, name: str) -> GroupInDB | None: dbgroup = self.session.execute(select(self.model).filter_by(name=name)).scalars().one_or_none() if dbgroup is None: return None return self.schema.from_orm(dbgroup) + def get_by_slug_or_id(self, slug_or_id: str | UUID) -> GroupInDB | None: + if isinstance(slug_or_id, str): + try: + slug_or_id = UUID(slug_or_id) + except ValueError: + pass + + if isinstance(slug_or_id, UUID): + return self.get_one(slug_or_id) + else: + return self.get_one(slug_or_id, key="slug") + def statistics(self, group_id: UUID4) -> GroupStatistics: def model_count(model: type[SqlAlchemyBase]) -> int: stmt = select(func.count(model.id)).filter_by(group_id=group_id) diff --git a/mealie/routes/explore/controller_public_recipes.py b/mealie/routes/explore/controller_public_recipes.py index 509a778c2f..66c495408f 100644 --- a/mealie/routes/explore/controller_public_recipes.py +++ b/mealie/routes/explore/controller_public_recipes.py @@ -1,5 +1,4 @@ from fastapi import APIRouter, HTTPException -from pydantic import UUID4 from mealie.routes._base import controller from mealie.routes._base.base_controllers import BasePublicController @@ -10,14 +9,14 @@ @controller(router) class PublicRecipesController(BasePublicController): - @router.get("/recipes/{group_id}/{recipe_slug}", response_model=Recipe) - def get_recipe(self, group_id: UUID4, recipe_slug: str) -> Recipe: - group = self.repos.groups.get_one(group_id) + @router.get("/recipes/{group_slug}/{recipe_slug}", response_model=Recipe) + def get_recipe(self, group_slug: str, recipe_slug: str) -> Recipe: + group = self.repos.groups.get_by_slug_or_id(group_slug) if not group or group.preferences.private_group: raise HTTPException(404, "group not found") - recipe = self.repos.recipes.by_group(group_id).get_one(recipe_slug) + recipe = self.repos.recipes.by_group(group.id).get_one(recipe_slug) if not recipe or not recipe.settings.public: raise HTTPException(404, "recipe not found") diff --git a/mealie/schema/user/user.py b/mealie/schema/user/user.py index 2240aeb730..7a3ed557a8 100644 --- a/mealie/schema/user/user.py +++ b/mealie/schema/user/user.py @@ -181,6 +181,7 @@ def loader_options(cls) -> list[LoaderOption]: class UpdateGroup(GroupBase): id: UUID4 name: str + slug: str categories: list[CategoryBase] | None = [] webhooks: list[Any] = [] diff --git a/tests/integration_tests/public_explorer_tests/public_recipe_tests.py b/tests/integration_tests/public_explorer_tests/public_recipe_tests.py index f07da1f557..551224f169 100644 --- a/tests/integration_tests/public_explorer_tests/public_recipe_tests.py +++ b/tests/integration_tests/public_explorer_tests/public_recipe_tests.py @@ -34,6 +34,8 @@ def test_public_recipe_success( test_case: PublicRecipeTestCase, ): group = database.groups.get_one(unique_user.group_id) + assert group + group.preferences.private_group = test_case.private_group database.group_preferences.update(group.id, group.preferences) @@ -42,9 +44,10 @@ def test_public_recipe_success( database.recipes.update(random_recipe.slug, random_recipe) # Try to access recipe + recipe_group = database.groups.get_by_slug_or_id(random_recipe.group_id) response = api_client.get( - api_routes.explore_recipes_group_id_recipe_slug( - random_recipe.group_id, + api_routes.explore_recipes_group_slug_recipe_slug( + recipe_group.slug, random_recipe.slug, ) ) diff --git a/tests/unit_tests/repository_tests/test_group_repository.py b/tests/unit_tests/repository_tests/test_group_repository.py new file mode 100644 index 0000000000..2f6c9ffc43 --- /dev/null +++ b/tests/unit_tests/repository_tests/test_group_repository.py @@ -0,0 +1,24 @@ +from mealie.repos.repository_factory import AllRepositories +from tests.utils.factories import random_int, random_string + + +def test_group_resolve_similar_names(database: AllRepositories): + base_group_name = random_string() + groups = database.groups.create_many({"name": base_group_name} for _ in range(random_int(3, 10))) + + seen_names = set() + seen_slugs = set() + for group in groups: + assert group.name not in seen_names + assert group.slug not in seen_slugs + seen_names.add(group.name) + seen_slugs.add(group.slug) + + assert base_group_name in group.name + + +def test_group_get_by_slug_or_id(database: AllRepositories): + groups = [database.groups.create({"name": random_string()}) for _ in range(random_int(3, 10))] + for group in groups: + assert database.groups.get_by_slug_or_id(group.id) == group + assert database.groups.get_by_slug_or_id(group.slug) == group diff --git a/tests/utils/api_routes/__init__.py b/tests/utils/api_routes/__init__.py index b0d787737e..34b6328f63 100644 --- a/tests/utils/api_routes/__init__.py +++ b/tests/utils/api_routes/__init__.py @@ -225,9 +225,9 @@ def comments_item_id(item_id): return f"{prefix}/comments/{item_id}" -def explore_recipes_group_id_recipe_slug(group_id, recipe_slug): - """`/api/explore/recipes/{group_id}/{recipe_slug}`""" - return f"{prefix}/explore/recipes/{group_id}/{recipe_slug}" +def explore_recipes_group_slug_recipe_slug(group_slug, recipe_slug): + """`/api/explore/recipes/{group_slug}/{recipe_slug}`""" + return f"{prefix}/explore/recipes/{group_slug}/{recipe_slug}" def foods_item_id(item_id):