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>
This commit is contained in:
Michael Genson 2023-08-20 13:38:46 -05:00 committed by GitHub
parent 99372aa2b6
commit 095edef95e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 166 additions and 18 deletions

View File

@ -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 ###

View File

@ -496,6 +496,22 @@ export default defineComponent({
}
const { copyText } = useCopy();
const groupSlug = ref<string>("");
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<any> } = {
@ -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}`);
},
};

View File

@ -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<Recipe>(routes.recipe(groupId, recipeSlug));
async recipe(groupSlug: string, recipeSlug: string) {
return await this.requests.get<Recipe>(routes.recipe(groupSlug, recipeSlug));
}
}

View File

@ -41,6 +41,7 @@ export interface GroupBase {
export interface GroupInDB {
name: string;
id: string;
slug: string;
categories?: CategoryBase[];
webhooks?: unknown[];
users?: UserOut[];

View File

@ -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);

View File

@ -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

View File

@ -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.tag import Tag
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)

View File

@ -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 @@ router = APIRouter(prefix="/explore", tags=["Explore: Recipes"])
@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")

View File

@ -181,6 +181,7 @@ class PrivateUser(UserOut):
class UpdateGroup(GroupBase):
id: UUID4
name: str
slug: str
categories: list[CategoryBase] | None = []
webhooks: list[Any] = []

View File

@ -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,
)
)

View File

@ -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

View File

@ -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):