From a2f8f271937780a198cf6340293ec7a49513e124 Mon Sep 17 00:00:00 2001 From: Hayden <64056131+hay-kot@users.noreply.github.com> Date: Sat, 11 Dec 2021 19:12:08 -0900 Subject: [PATCH] =?UTF-8?q?feat:=20=E2=9C=A8=20support=20for=20lockable=20?= =?UTF-8?q?recipes=20(#876)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: :sparkles: support for lockable recipes * feat(backend): :sparkles: check user can update before updating recipe * test(backend): :white_check_mark: add recipe lock tests * feat(frontend): :sparkles: disabled lock action when not owner * test(backend): :white_check_mark: test non-owner can't lock recipe * hide quantity on zero value * fix(backend): :bug: temp/partial fix for recipes with same name. WIP --- .../Domain/Recipe/RecipeActionMenu.vue | 23 ++++++- .../Domain/Recipe/RecipeSettingsMenu.vue | 6 ++ .../recipes/use-recipe-ingredients.ts | 6 +- frontend/pages/recipe/_slug/index.vue | 3 +- mealie/db/data_access_layer/_access_model.py | 47 ++++++++++++-- mealie/db/models/recipe/settings.py | 3 + mealie/schema/recipe/recipe_settings.py | 5 +- .../_base_http_service/base_http_service.py | 2 +- mealie/services/recipe/recipe_service.py | 24 ++++++-- tests/fixtures/fixture_users.py | 61 ++++++++++++++++++- .../user_recipe_tests/test_recipe_owner.py | 43 ++++++++++++- 11 files changed, 202 insertions(+), 21 deletions(-) diff --git a/frontend/components/Domain/Recipe/RecipeActionMenu.vue b/frontend/components/Domain/Recipe/RecipeActionMenu.vue index cd9e6123..f68c2785 100644 --- a/frontend/components/Domain/Recipe/RecipeActionMenu.vue +++ b/frontend/components/Domain/Recipe/RecipeActionMenu.vue @@ -22,10 +22,9 @@ <v-spacer></v-spacer> <div v-if="!value" class="custom-btn-group ma-1"> <RecipeFavoriteBadge v-if="loggedIn" class="mx-1" color="info" button-style :slug="slug" show-always /> - <v-tooltip bottom color="info"> + <v-tooltip v-if="!locked" bottom color="info"> <template #activator="{ on, attrs }"> <v-btn - v-if="loggedIn" fab small class="mx-1" @@ -39,6 +38,22 @@ </template> <span>{{ $t("general.edit") }}</span> </v-tooltip> + <v-tooltip v-else bottom color="info"> + <template #activator="{ on, attrs }"> + <v-btn + fab + small + class="mx-1" + color="info" + v-bind="attrs" + v-on="on" + > + <v-icon> {{ $globals.icons.lock }} </v-icon> + </v-btn> + </template> + <span> Locked by Owner </span> + </v-tooltip> + <RecipeContextMenu show-print :menu-top="false" @@ -109,6 +124,10 @@ export default { required: true, type: Number, }, + locked: { + type: Boolean, + default: false, + }, }, data() { return { diff --git a/frontend/components/Domain/Recipe/RecipeSettingsMenu.vue b/frontend/components/Domain/Recipe/RecipeSettingsMenu.vue index 82fe4ccb..9cf14b74 100644 --- a/frontend/components/Domain/Recipe/RecipeSettingsMenu.vue +++ b/frontend/components/Domain/Recipe/RecipeSettingsMenu.vue @@ -23,6 +23,7 @@ v-model="value[key]" xs dense + :disabled="key == 'locked' && !isOwner" class="my-1" :label="labels[key]" hide-details @@ -41,6 +42,10 @@ export default { type: Object, required: true, }, + isOwner: { + type: Boolean, + required: false, + }, }, computed: { @@ -52,6 +57,7 @@ export default { landscapeView: this.$t("recipe.landscape-view-coming-soon"), disableComments: this.$t("recipe.disable-comments"), disableAmount: this.$t("recipe.disable-amount"), + locked: "Locked", }; }, }, diff --git a/frontend/composables/recipes/use-recipe-ingredients.ts b/frontend/composables/recipes/use-recipe-ingredients.ts index f8225bf1..cc459fde 100644 --- a/frontend/composables/recipes/use-recipe-ingredients.ts +++ b/frontend/composables/recipes/use-recipe-ingredients.ts @@ -10,6 +10,8 @@ export function parseIngredientText(ingredient: RecipeIngredient, disableAmount: const { quantity, food, unit, note } = ingredient; + const validQuantity = quantity !== null && quantity !== undefined && quantity !== 0; + let returnQty = ""; if (unit?.fraction) { const fraction = frac(quantity * scale, 10, true); @@ -20,8 +22,10 @@ export function parseIngredientText(ingredient: RecipeIngredient, disableAmount: if (fraction[1] > 0) { returnQty += ` <sup>${fraction[1]}</sup>⁄<sub>${fraction[2]}</sub>`; } - } else { + } else if (validQuantity) { returnQty = (quantity * scale).toString(); + } else { + returnQty = ""; } return `${returnQty} ${unit?.name || " "} ${food?.name || " "} ${note}`.replace(/ {2,}/g, " "); diff --git a/frontend/pages/recipe/_slug/index.vue b/frontend/pages/recipe/_slug/index.vue index 2628f8b4..f2016046 100644 --- a/frontend/pages/recipe/_slug/index.vue +++ b/frontend/pages/recipe/_slug/index.vue @@ -51,6 +51,7 @@ <RecipeActionMenu v-model="form" :slug="recipe.slug" + :locked="$auth.user.id !== recipe.userId && recipe.settings.locked" :name="recipe.name" :logged-in="$auth.loggedIn" :open="form" @@ -77,7 +78,7 @@ > <div v-if="form" class="d-flex justify-start align-center"> <RecipeImageUploadBtn class="my-1" :slug="recipe.slug" @upload="uploadImage" @refresh="imageKey++" /> - <RecipeSettingsMenu class="my-1 mx-1" :value="recipe.settings" @upload="uploadImage" /> + <RecipeSettingsMenu class="my-1 mx-1" :value="recipe.settings" :is-owner="recipe.userId == $auth.user.id" @upload="uploadImage" /> </div> <!-- Recipe Title Section --> <template v-if="!form && enableLandscape"> diff --git a/mealie/db/data_access_layer/_access_model.py b/mealie/db/data_access_layer/_access_model.py index 83fba27b..f658d09c 100644 --- a/mealie/db/data_access_layer/_access_model.py +++ b/mealie/db/data_access_layer/_access_model.py @@ -1,6 +1,7 @@ from __future__ import annotations -from typing import Callable, Generic, TypeVar, Union +from typing import Any, Callable, Generic, TypeVar, Union +from uuid import UUID from sqlalchemy import func from sqlalchemy.orm import load_only @@ -29,9 +30,36 @@ class AccessModel(Generic[T, D]): self.schema = schema self.observers: list = [] + self.limit_by_group = False + self.user_id = None + + self.limit_by_user = False + self.group_id = None + def subscribe(self, func: Callable) -> None: self.observers.append(func) + def by_user(self, user_id: int) -> AccessModel: + self.limit_by_user = True + self.user_id = user_id + return self + + def by_group(self, group_id: UUID) -> AccessModel: + self.limit_by_group = True + self.group_id = group_id + return self + + def _filter_builder(self, **kwargs) -> dict[str, Any]: + dct = {} + + if self.limit_by_user: + dct["user_id"] = self.user_id + + if self.limit_by_group: + dct["group_id"] = self.group_id + + return {**dct, **kwargs} + # TODO: Run Observer in Async Background Task def update_observers(self) -> None: if self.observers: @@ -114,16 +142,21 @@ class AccessModel(Generic[T, D]): if match_key is None: match_key = self.primary_key - return self.session.query(self.sql_model).filter_by(**{match_key: match_value}).one() + filter = self._filter_builder(**{match_key: match_value}) + return self.session.query(self.sql_model).filter_by(**filter).one() def get_one(self, value: str | int, key: str = None, any_case=False, override_schema=None) -> T: key = key or self.primary_key + q = self.session.query(self.sql_model) + if any_case: search_attr = getattr(self.sql_model, key) - result = self.session.query(self.sql_model).filter(func.lower(search_attr) == key.lower()).one_or_none() + q = q.filter(func.lower(search_attr) == key.lower()).filter_by(**self._filter_builder()) else: - result = self.session.query(self.sql_model).filter_by(**{key: value}).one_or_none() + q = self.session.query(self.sql_model).filter_by(**self._filter_builder(**{key: value})) + + result = q.one_or_none() if not result: return @@ -255,7 +288,11 @@ class AccessModel(Generic[T, D]): return self.session.query(self.sql_model).filter_by(**{match_key: match_value}).count() def _count_attribute( - self, attribute_name: str, attr_match: str = None, count=True, override_schema=None + self, + attribute_name: str, + attr_match: str = None, + count=True, + override_schema=None, ) -> Union[int, T]: eff_schema = override_schema or self.schema # attr_filter = getattr(self.sql_model, attribute_name) diff --git a/mealie/db/models/recipe/settings.py b/mealie/db/models/recipe/settings.py index b7f4ecdc..a3954e06 100644 --- a/mealie/db/models/recipe/settings.py +++ b/mealie/db/models/recipe/settings.py @@ -13,6 +13,7 @@ class RecipeSettings(SqlAlchemyBase): landscape_view = sa.Column(sa.Boolean) disable_amount = sa.Column(sa.Boolean, default=False) disable_comments = sa.Column(sa.Boolean, default=False) + locked = sa.Column(sa.Boolean, default=False) def __init__( self, @@ -22,7 +23,9 @@ class RecipeSettings(SqlAlchemyBase): landscape_view=True, disable_amount=True, disable_comments=False, + locked=False, ) -> None: + self.locked = locked self.public = public self.show_nutrition = show_nutrition self.show_assets = show_assets diff --git a/mealie/schema/recipe/recipe_settings.py b/mealie/schema/recipe/recipe_settings.py index 77fe07c3..4c0e5790 100644 --- a/mealie/schema/recipe/recipe_settings.py +++ b/mealie/schema/recipe/recipe_settings.py @@ -1,9 +1,5 @@ from fastapi_camelcase import CamelModel -from mealie.core.config import get_app_settings - -settings = get_app_settings() - class RecipeSettings(CamelModel): public: bool = False @@ -12,6 +8,7 @@ class RecipeSettings(CamelModel): landscape_view: bool = False disable_comments: bool = True disable_amount: bool = True + locked: bool = False class Config: orm_mode = True diff --git a/mealie/services/_base_http_service/base_http_service.py b/mealie/services/_base_http_service/base_http_service.py index 6a7e7514..a5d68689 100644 --- a/mealie/services/_base_http_service/base_http_service.py +++ b/mealie/services/_base_http_service/base_http_service.py @@ -109,7 +109,7 @@ class BaseHttpService(Generic[T, D], ABC): def group_id(self): # TODO: Populate Group in Private User Call WARNING: May require significant refactoring if not self._group_id_cache: - group = self.db.groups.get(self.user.group, "name") + group = self.db.groups.get_one(self.user.group, "name") self._group_id_cache = group.id return self._group_id_cache diff --git a/mealie/services/recipe/recipe_service.py b/mealie/services/recipe/recipe_service.py index 03fe9fd8..8e43cda7 100644 --- a/mealie/services/recipe/recipe_service.py +++ b/mealie/services/recipe/recipe_service.py @@ -9,7 +9,7 @@ from zipfile import ZipFile from fastapi import Depends, HTTPException, UploadFile, status from sqlalchemy import exc -from mealie.core.dependencies.grouped import PublicDeps, UserDeps +from mealie.core.dependencies.grouped import UserDeps from mealie.core.root_logger import get_logger from mealie.db.data_access_layer.recipe_access_model import RecipeDataAccessModel from mealie.schema.recipe.recipe import CreateRecipe, Recipe, RecipeSummary @@ -41,14 +41,14 @@ class RecipeService(CrudHttpMixins[CreateRecipe, Recipe, Recipe], UserHttpServic @cached_property def dal(self) -> RecipeDataAccessModel: - return self.db.recipes + return self.db.recipes.by_group(self.group_id) @classmethod def write_existing(cls, slug: str, deps: UserDeps = Depends()): return super().write_existing(slug, deps) @classmethod - def read_existing(cls, slug: str, deps: PublicDeps = Depends()): + def read_existing(cls, slug: str, deps: UserDeps = Depends()): return super().write_existing(slug, deps) def assert_existing(self, slug: str): @@ -59,6 +59,12 @@ class RecipeService(CrudHttpMixins[CreateRecipe, Recipe, Recipe], UserHttpServic if not self.item.settings.public and not self.user: raise HTTPException(status.HTTP_403_FORBIDDEN) + def can_update(self) -> bool: + if self.user.id == self.item.user_id: + return True + + raise HTTPException(status.HTTP_403_FORBIDDEN) + def get_all(self, start=0, limit=None, load_foods=False) -> list[RecipeSummary]: items = self.db.recipes.summary(self.user.group_id, start=start, limit=limit, load_foods=load_foods) @@ -78,7 +84,7 @@ class RecipeService(CrudHttpMixins[CreateRecipe, Recipe, Recipe], UserHttpServic def create_one(self, create_data: Union[Recipe, CreateRecipe]) -> Recipe: group = self.db.groups.get(self.group_id, "id") - create_data = recipe_creation_factory( + create_data: Recipe = recipe_creation_factory( self.user, name=create_data.name, additional_attrs=create_data.dict(), @@ -129,18 +135,28 @@ class RecipeService(CrudHttpMixins[CreateRecipe, Recipe, Recipe], UserHttpServic return self.item def update_one(self, update_data: Recipe) -> Recipe: + self.can_update() + + if self.item.settings.locked != update_data.settings.locked and self.item.user_id != self.user.id: + raise HTTPException(status.HTTP_403_FORBIDDEN) + original_slug = self.item.slug self._update_one(update_data, original_slug) + self.check_assets(original_slug) return self.item def patch_one(self, patch_data: Recipe) -> Recipe: + self.can_update() + original_slug = self.item.slug self._patch_one(patch_data, original_slug) + self.check_assets(original_slug) return self.item def delete_one(self) -> Recipe: + self.can_update() self._delete_one(self.item.slug) self.delete_assets() self._create_event("Recipe Delete", f"'{self.item.name}' deleted by {self.user.full_name}") diff --git a/tests/fixtures/fixture_users.py b/tests/fixtures/fixture_users.py index 0ef7ed55..485dcd23 100644 --- a/tests/fixtures/fixture_users.py +++ b/tests/fixtures/fixture_users.py @@ -1,4 +1,5 @@ import json +from typing import Tuple import requests from pytest import fixture @@ -38,7 +39,12 @@ def g2_user(admin_token, api_client: requests, api_routes: utils.AppRoutes): group_id = json.loads(self_response.text).get("groupId") try: - yield utils.TestUser(user_id=user_id, _group_id=group_id, token=token, email=create_data["email"]) + yield utils.TestUser( + user_id=user_id, + _group_id=group_id, + token=token, + email=create_data["email"], + ) finally: # TODO: Delete User after test pass @@ -69,6 +75,59 @@ def unique_user(api_client: TestClient, api_routes: utils.AppRoutes): pass +@fixture(scope="module") +def user_tuple(admin_token, api_client: requests, api_routes: utils.AppRoutes) -> Tuple[utils.TestUser]: + group_name = utils.random_string() + # Create the user + create_data_1 = { + "fullName": utils.random_string(), + "username": utils.random_string(), + "email": utils.random_email(), + "password": "useruser", + "group": group_name, + "admin": False, + "tokens": [], + } + + create_data_2 = { + "fullName": utils.random_string(), + "username": utils.random_string(), + "email": utils.random_email(), + "password": "useruser", + "group": group_name, + "admin": False, + "tokens": [], + } + + users_out = [] + + for usr in [create_data_1, create_data_2]: + response = api_client.post(api_routes.groups, json={"name": "New Group"}, headers=admin_token) + response = api_client.post(api_routes.users, json=usr, headers=admin_token) + assert response.status_code == 201 + + # Log in as this user + form_data = {"username": usr["email"], "password": "useruser"} + token = utils.login(form_data, api_client, api_routes) + response = api_client.get(api_routes.users_self, headers=token) + assert response.status_code == 200 + user_data = json.loads(response.text) + + users_out.append( + utils.TestUser( + _group_id=user_data.get("groupId"), + user_id=user_data.get("id"), + email=user_data.get("email"), + token=token, + ) + ) + + try: + yield users_out + finally: + pass + + @fixture(scope="session") def user_token(admin_token, api_client: requests, api_routes: utils.AppRoutes): # Create the user diff --git a/tests/integration_tests/user_recipe_tests/test_recipe_owner.py b/tests/integration_tests/user_recipe_tests/test_recipe_owner.py index 6bbe7139..67af89a8 100644 --- a/tests/integration_tests/user_recipe_tests/test_recipe_owner.py +++ b/tests/integration_tests/user_recipe_tests/test_recipe_owner.py @@ -22,9 +22,7 @@ def test_ownership_on_new_with_admin(api_client: TestClient, admin_user: TestUse def test_ownership_on_new_with_user(api_client: TestClient, g2_user: TestUser): recipe_name = random_string() - response = api_client.post(Routes.base, json={"name": recipe_name}, headers=g2_user.token) - assert response.status_code == 201 response = api_client.get(Routes.base + f"/{recipe_name}", headers=g2_user.token) @@ -67,3 +65,44 @@ def test_unique_slug_by_group(api_client: TestClient, unique_user: TestUser, g2_ # Try to create a recipe again with the same name response = api_client.post(Routes.base, json=create_data, headers=g2_user.token) assert response.status_code == 400 + + +def test_user_locked_recipe(api_client: TestClient, user_tuple: list[TestUser]) -> None: + usr_1, usr_2 = user_tuple + + # Setup Recipe + recipe_name = random_string() + response = api_client.post(Routes.base, json={"name": recipe_name}, headers=usr_1.token) + assert response.status_code == 201 + + # Get Recipe + response = api_client.get(Routes.base + f"/{recipe_name}", headers=usr_1.token) + assert response.status_code == 200 + recipe = response.json() + + # Lock Recipe + recipe["settings"]["locked"] = True + response = api_client.put(Routes.base + f"/{recipe_name}", json=recipe, headers=usr_1.token) + + # Try To Update Recipe with User 2 + response = api_client.put(Routes.base + f"/{recipe_name}", json=recipe, headers=usr_2.token) + assert response.status_code == 403 + + +def test_other_user_cant_lock_recipe(api_client: TestClient, user_tuple: list[TestUser]) -> None: + usr_1, usr_2 = user_tuple + + # Setup Recipe + recipe_name = random_string() + response = api_client.post(Routes.base, json={"name": recipe_name}, headers=usr_1.token) + assert response.status_code == 201 + + # Get Recipe + response = api_client.get(Routes.base + f"/{recipe_name}", headers=usr_2.token) + assert response.status_code == 200 + recipe = response.json() + + # Lock Recipe + recipe["settings"]["locked"] = True + response = api_client.put(Routes.base + f"/{recipe_name}", json=recipe, headers=usr_2.token) + assert response.status_code == 403