feat(backend): ✨ refactor/fix group management for admins (#838)

* fix(frontend): 🐛 update dialog implementation to simplify state management

* test(backend): ✅ refactor test fixtures + admin group tests

* chore(backend): 🔨 add launcher.json for python debugging (tests)

* fix typing

* feat(backend): ✨ refactor/fix group management for admins

* feat(frontend): ✨ add/fix admin group management

* add LDAP checker

Co-authored-by: hay-kot <hay-kot@pm.me>

tests/conftest.py

@@ -1,19 +1,13 @@
 from tests.pre_test import settings  # isort:skip
 
-import json
-
-import requests
 from fastapi.testclient import TestClient
 from pytest import fixture
 
 from mealie.app import app
 from mealie.db.db_setup import SessionLocal, generate_session
 from mealie.db.init_db import main
-from tests.app_routes import AppRoutes
+from tests.fixtures import *  # noqa: F403 F401
 from tests.test_config import TEST_DATA
-from tests.utils.factories import random_email, random_string, user_registration_factory
-from tests.utils.fixture_schemas import TestUser
-from tests.utils.recipe_data import get_raw_no_image, get_raw_recipe, get_recipe_test_cases
 
 main()
 
@@ -39,11 +33,6 @@ def api_client():
         pass
 
 
-@fixture(scope="session")
-def api_routes():
-    return AppRoutes()
-
-
 @fixture(scope="session")
 def test_image_jpg():
     return TEST_DATA.joinpath("images", "test_image.jpg")
@@ -52,132 +41,3 @@ def test_image_jpg():
 @fixture(scope="session")
 def test_image_png():
     return TEST_DATA.joinpath("images", "test_image.png")
-
-
-def login(form_data, api_client: requests, api_routes: AppRoutes):
-    response = api_client.post(api_routes.auth_token, form_data)
-    assert response.status_code == 200
-    token = json.loads(response.text).get("access_token")
-    return {"Authorization": f"Bearer {token}"}
-
-
-@fixture(scope="session")
-def admin_token(api_client: requests, api_routes: AppRoutes):
-    form_data = {"username": "changeme@email.com", "password": settings.DEFAULT_PASSWORD}
-    return login(form_data, api_client, api_routes)
-
-
-@fixture(scope="session")
-def g2_user(admin_token, api_client: requests, api_routes: AppRoutes):
-    # Create the user
-    create_data = {
-        "fullName": random_string(),
-        "username": random_string(),
-        "email": random_email(),
-        "password": "useruser",
-        "group": "New Group",
-        "admin": False,
-        "tokens": [],
-    }
-
-    response = api_client.post(api_routes.groups, json={"name": "New Group"}, headers=admin_token)
-    response = api_client.post(api_routes.users, json=create_data, headers=admin_token)
-
-    assert response.status_code == 201
-
-    # Log in as this user
-    form_data = {"username": create_data["email"], "password": "useruser"}
-
-    token = login(form_data, api_client, api_routes)
-
-    self_response = api_client.get(api_routes.users_self, headers=token)
-
-    assert self_response.status_code == 200
-
-    user_id = json.loads(self_response.text).get("id")
-    group_id = json.loads(self_response.text).get("groupId")
-
-    return TestUser(user_id=user_id, group_id=group_id, token=token, email=create_data["email"])
-
-
-@fixture(scope="session")
-def user_token(admin_token, api_client: requests, api_routes: AppRoutes):
-    # Create the user
-    create_data = {
-        "fullName": random_string(),
-        "username": random_string(),
-        "email": random_email(),
-        "password": "useruser",
-        "group": "Home",
-        "admin": False,
-        "tokens": [],
-    }
-
-    response = api_client.post(api_routes.users, json=create_data, headers=admin_token)
-
-    assert response.status_code == 201
-
-    # Log in as this user
-    form_data = {"username": create_data["email"], "password": "useruser"}
-    return login(form_data, api_client, api_routes)
-
-
-@fixture(scope="session")
-def raw_recipe():
-    return get_raw_recipe()
-
-
-@fixture(scope="session")
-def raw_recipe_no_image():
-    return get_raw_no_image()
-
-
-@fixture(scope="session")
-def recipe_store():
-    return get_recipe_test_cases()
-
-
-@fixture(scope="module")
-def unique_user(api_client: TestClient, api_routes: AppRoutes):
-    registration = user_registration_factory()
-
-    response = api_client.post("/api/users/register", json=registration.dict(by_alias=True))
-    assert response.status_code == 201
-
-    form_data = {"username": registration.username, "password": registration.password}
-
-    token = login(form_data, api_client, api_routes)
-
-    user_data = api_client.get(api_routes.users_self, headers=token).json()
-    assert token is not None
-
-    try:
-        yield TestUser(
-            group_id=user_data.get("groupId"), user_id=user_data.get("id"), email=user_data.get("email"), token=token
-        )
-    finally:
-        # TODO: Delete User after test
-        pass
-
-
-@fixture(scope="session")
-def admin_user(api_client: TestClient, api_routes: AppRoutes):
-
-    form_data = {"username": "changeme@email.com", "password": settings.DEFAULT_PASSWORD}
-
-    token = login(form_data, api_client, api_routes)
-
-    user_data = api_client.get(api_routes.users_self, headers=token).json()
-    assert token is not None
-
-    assert user_data.get("admin") is True
-    assert user_data.get("groupId") is not None
-    assert user_data.get("id") is not None
-
-    try:
-        yield TestUser(
-            group_id=user_data.get("groupId"), user_id=user_data.get("id"), email=user_data.get("email"), token=token
-        )
-    finally:
-        # TODO: Delete User after test
-        pass

tests/fixtures/__init__.py

@@ -0,0 +1,4 @@
+from .fixture_admin import *
+from .fixture_recipe import *
+from .fixture_routes import *
+from .fixture_users import *

tests/fixtures/fixture_admin.py

@@ -0,0 +1,41 @@
+import requests
+from pytest import fixture
+from starlette.testclient import TestClient
+
+from mealie.core.config import get_app_settings
+from tests import utils
+
+
+@fixture(scope="session")
+def admin_token(api_client: requests, api_routes: utils.AppRoutes):
+    settings = get_app_settings()
+
+    form_data = {"username": "changeme@email.com", "password": settings.DEFAULT_PASSWORD}
+    return utils.login(form_data, api_client, api_routes)
+
+
+@fixture(scope="session")
+def admin_user(api_client: TestClient, api_routes: utils.AppRoutes):
+    settings = get_app_settings()
+
+    form_data = {"username": "changeme@email.com", "password": settings.DEFAULT_PASSWORD}
+
+    token = utils.login(form_data, api_client, api_routes)
+
+    user_data = api_client.get(api_routes.users_self, headers=token).json()
+    assert token is not None
+
+    assert user_data.get("admin") is True
+    assert user_data.get("groupId") is not None
+    assert user_data.get("id") is not None
+
+    try:
+        yield utils.TestUser(
+            group_id=user_data.get("groupId"),
+            user_id=user_data.get("id"),
+            email=user_data.get("email"),
+            token=token,
+        )
+    finally:
+        # TODO: Delete User after test
+        pass

tests/fixtures/fixture_recipe.py

@@ -0,0 +1,18 @@
+from pytest import fixture
+
+from tests.utils.recipe_data import get_raw_no_image, get_raw_recipe, get_recipe_test_cases
+
+
+@fixture(scope="session")
+def raw_recipe():
+    return get_raw_recipe()
+
+
+@fixture(scope="session")
+def raw_recipe_no_image():
+    return get_raw_no_image()
+
+
+@fixture(scope="session")
+def recipe_store():
+    return get_recipe_test_cases()

tests/fixtures/fixture_routes.py

@@ -0,0 +1,8 @@
+from pytest import fixture
+
+from tests import utils
+
+
+@fixture(scope="session")
+def api_routes():
+    return utils.AppRoutes()

tests/fixtures/fixture_users.py

@@ -0,0 +1,91 @@
+import json
+
+import requests
+from pytest import fixture
+from starlette.testclient import TestClient
+
+from tests import utils
+
+
+@fixture(scope="module")
+def g2_user(admin_token, api_client: requests, api_routes: utils.AppRoutes):
+    # Create the user
+    create_data = {
+        "fullName": utils.random_string(),
+        "username": utils.random_string(),
+        "email": utils.random_email(),
+        "password": "useruser",
+        "group": "New Group",
+        "admin": False,
+        "tokens": [],
+    }
+
+    response = api_client.post(api_routes.groups, json={"name": "New Group"}, headers=admin_token)
+    response = api_client.post(api_routes.users, json=create_data, headers=admin_token)
+
+    assert response.status_code == 201
+
+    # Log in as this user
+    form_data = {"username": create_data["email"], "password": "useruser"}
+
+    token = utils.login(form_data, api_client, api_routes)
+
+    self_response = api_client.get(api_routes.users_self, headers=token)
+
+    assert self_response.status_code == 200
+
+    user_id = json.loads(self_response.text).get("id")
+    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"])
+    finally:
+        # TODO: Delete User after test
+        pass
+
+
+@fixture(scope="module")
+def unique_user(api_client: TestClient, api_routes: utils.AppRoutes):
+    registration = utils.user_registration_factory()
+    response = api_client.post("/api/users/register", json=registration.dict(by_alias=True))
+    assert response.status_code == 201
+
+    form_data = {"username": registration.username, "password": registration.password}
+
+    token = utils.login(form_data, api_client, api_routes)
+
+    user_data = api_client.get(api_routes.users_self, headers=token).json()
+    assert token is not None
+
+    try:
+        yield utils.TestUser(
+            group_id=user_data.get("groupId"),
+            user_id=user_data.get("id"),
+            email=user_data.get("email"),
+            token=token,
+        )
+    finally:
+        # TODO: Delete User after test
+        pass
+
+
+@fixture(scope="session")
+def user_token(admin_token, api_client: requests, api_routes: utils.AppRoutes):
+    # Create the user
+    create_data = {
+        "fullName": utils.random_string(),
+        "username": utils.random_string(),
+        "email": utils.random_email(),
+        "password": "useruser",
+        "group": "Home",
+        "admin": False,
+        "tokens": [],
+    }
+
+    response = api_client.post(api_routes.users, json=create_data, headers=admin_token)
+
+    assert response.status_code == 201
+
+    # Log in as this user
+    form_data = {"username": create_data["email"], "password": "useruser"}
+    return utils.login(form_data, api_client, api_routes)

tests/integration_tests/admin_tests/test_admin_user_actions.py

@@ -2,7 +2,7 @@ import json
 
 from fastapi.testclient import TestClient
 
-from tests.app_routes import AppRoutes
+from tests.utils.app_routes import AppRoutes
 from tests.utils.fixture_schemas import TestUser
 
 

tests/integration_tests/admin_tests/test_group_admin_actions.py

@@ -1,8 +1,7 @@
-import json
-
 from fastapi.testclient import TestClient
 
-from tests.utils.factories import random_string
+from tests.utils.assertion_helpers import assert_ignore_keys
+from tests.utils.factories import random_bool, random_string
 from tests.utils.fixture_schemas import TestUser
 
 
@@ -12,35 +11,69 @@ class Routes:
     def item(id: str) -> str:
         return f"{Routes.base}/{id}"
 
-
-def test_create_group(api_client: TestClient, admin_token):
-    response = api_client.post(Routes.base, json={"name": random_string()}, headers=admin_token)
-    assert response.status_code == 201
+    def user(id: str) -> str:
+        return f"api/admin/users/{id}"
 
 
-def test_user_cant_create_group(api_client: TestClient, unique_user: TestUser):
-    response = api_client.post(Routes.base, json={"name": random_string()}, headers=unique_user.token)
-    assert response.status_code == 403
-
-
-def test_home_group_not_deletable(api_client: TestClient, admin_token):
-    response = api_client.delete(Routes.item(1), headers=admin_token)
-
+def test_home_group_not_deletable(api_client: TestClient, admin_user: TestUser):
+    response = api_client.delete(Routes.item(1), headers=admin_user.token)
     assert response.status_code == 400
 
 
-def test_delete_group(api_client: TestClient, admin_token):
-    response = api_client.post(Routes.base, json={"name": random_string()}, headers=admin_token)
+def test_admin_group_routes_are_restricted(api_client: TestClient, unique_user: TestUser):
+    response = api_client.get(Routes.base, headers=unique_user.token)
+    assert response.status_code == 403
+
+    response = api_client.post(Routes.base, json={}, headers=unique_user.token)
+    assert response.status_code == 403
+
+    response = api_client.get(Routes.item(1), headers=unique_user.token)
+    assert response.status_code == 403
+
+    response = api_client.get(Routes.user(1), headers=unique_user.token)
+    assert response.status_code == 403
+
+
+def test_admin_create_group(api_client: TestClient, admin_user: TestUser):
+    response = api_client.post(Routes.base, json={"name": random_string()}, headers=admin_user.token)
     assert response.status_code == 201
 
-    group_id = json.loads(response.text)["id"]
 
-    response = api_client.delete(Routes.item(group_id), headers=admin_token)
+def test_admin_update_group(api_client: TestClient, admin_user: TestUser, unique_user: TestUser):
+    update_payload = {
+        "id": unique_user.group_id,
+        "name": "New Name",
+        "preferences": {
+            "privateGroup": random_bool(),
+            "firstDayOfWeek": 2,
+            "recipePublic": random_bool(),
+            "recipeShowNutrition": random_bool(),
+            "recipeShowAssets": random_bool(),
+            "recipeLandscapeView": random_bool(),
+            "recipeDisableComments": random_bool(),
+            "recipeDisableAmount": random_bool(),
+        },
+    }
+
+    response = api_client.put(Routes.item(unique_user.group_id), json=update_payload, headers=admin_user.token)
 
     assert response.status_code == 200
 
-    # Ensure Group is Deleted
-    response = api_client.get(Routes.base, headers=admin_token)
+    as_json = response.json()
 
-    for g in response.json():
-        assert g["id"] != group_id
+    assert as_json["name"] == update_payload["name"]
+    assert_ignore_keys(as_json["preferences"], update_payload["preferences"])
+
+
+def test_admin_delete_group(api_client: TestClient, admin_user: TestUser, unique_user: TestUser):
+    # Delete User
+    response = api_client.delete(Routes.user(unique_user.user_id), headers=admin_user.token)
+    assert response.status_code == 200
+
+    # Delete Group
+    response = api_client.delete(Routes.item(unique_user.group_id), headers=admin_user.token)
+    assert response.status_code == 200
+
+    # Ensure Group is Deleted
+    response = api_client.get(Routes.item(unique_user.user_id), headers=admin_user.token)
+    assert response.status_code == 404

tests/integration_tests/test_import_routes.py

@@ -3,7 +3,7 @@ import json
 import pytest
 from fastapi.testclient import TestClient
 
-from tests.app_routes import AppRoutes
+from tests.utils.app_routes import AppRoutes
 
 
 @pytest.fixture

tests/integration_tests/test_migration_routes.py

@@ -8,8 +8,8 @@ from fastapi.testclient import TestClient
 from mealie.core.config import get_app_dirs
 
 app_dirs = get_app_dirs()
-from tests.app_routes import AppRoutes
 from tests.test_config import TEST_CHOWDOWN_DIR, TEST_NEXTCLOUD_DIR
+from tests.utils.app_routes import AppRoutes
 
 
 @pytest.fixture(scope="session")

tests/integration_tests/user_recipe_tests/test_recipe_crud.py

@@ -4,7 +4,7 @@ import pytest
 from fastapi.testclient import TestClient
 from slugify import slugify
 
-from tests.app_routes import AppRoutes
+from tests.utils.app_routes import AppRoutes
 from tests.utils.fixture_schemas import TestUser
 from tests.utils.recipe_data import RecipeSiteTestCase, get_recipe_test_cases
 

tests/integration_tests/user_tests/test_user_api_token.py

@@ -3,7 +3,7 @@ import json
 from fastapi.testclient import TestClient
 from pytest import fixture
 
-from tests.app_routes import AppRoutes
+from tests.utils.app_routes import AppRoutes
 
 
 @fixture

tests/integration_tests/user_tests/test_user_images.py

@@ -5,7 +5,7 @@ from fastapi.testclient import TestClient
 from mealie.core.config import get_app_dirs
 
 app_dirs = get_app_dirs()
-from tests.app_routes import AppRoutes
+from tests.utils.app_routes import AppRoutes
 
 
 def test_update_user_image(

tests/integration_tests/user_tests/test_user_login.py

@@ -2,7 +2,7 @@ import json
 
 from fastapi.testclient import TestClient
 
-from tests.app_routes import AppRoutes
+from tests.utils.app_routes import AppRoutes
 
 
 def test_failed_login(api_client: TestClient, api_routes: AppRoutes):

tests/utils/__init__.py

@@ -0,0 +1,4 @@
+from .app_routes import *
+from .factories import *
+from .fixture_schemas import *
+from .user_login import *

tests/utils/assertion_helpers.py

@@ -1,14 +1,14 @@
-def assert_ignore_keys(dict1: dict, dict2: dict, ignore_keys: list) -> None:
+def assert_ignore_keys(dict1: dict, dict2: dict, ignore_keys: list = None) -> None:
     """
     Itterates through a list of keys and checks if they are in the the provided ignore_keys list,
     if they are not in the ignore_keys list, it checks the value of the key in the provided against
     the value provided in dict2. If the value of the key in dict1 is not equal to the value of the
     key in dict2, The assertion fails. Useful for testing id / group_id agnostic data
 
-    Note: ignore_keys defaults to ['id', 'group_id']
+    Note: ignore_keys defaults to ['id', 'group_id', 'groupId']
     """
     if ignore_keys is None:
-        ignore_keys = ["id", "group_id"]
+        ignore_keys = ["id", "group_id", "groupId"]
 
     for key, value in dict1.items():
         if key in ignore_keys:

tests/utils/user_login.py

@@ -0,0 +1,12 @@
+import json
+
+import requests
+
+from .app_routes import AppRoutes
+
+
+def login(form_data, api_client: requests, api_routes: AppRoutes):
+    response = api_client.post(api_routes.auth_token, form_data)
+    assert response.status_code == 200
+    token = json.loads(response.text).get("access_token")
+    return {"Authorization": f"Bearer {token}"}