fix: Tools Shouldn't Be Unique Across Groups (#2505)

* fixed type/abc errors in tests

* fixed false positive multitentant tests

* fix tools not allowing unique slugs across groups

* fixed alembic refs

---------

Co-authored-by: Hayden <64056131+hay-kot@users.noreply.github.com>

alembic/versions/2023-08-15-16.25.07_bcfdad6b7355_remove_tool_name_and_slug_unique_.py

@@ -0,0 +1,32 @@
+"""remove tool name and slug unique contraints
+
+Revision ID: bcfdad6b7355
+Revises: 1825b5225403
+Create Date: 2023-08-15 16:25:07.058929
+
+"""
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = "bcfdad6b7355"
+down_revision = "1825b5225403"
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.drop_index("ix_tools_name", table_name="tools")
+    op.create_index(op.f("ix_tools_name"), "tools", ["name"], unique=False)
+    op.drop_index("ix_tools_slug", table_name="tools")
+    op.create_index(op.f("ix_tools_slug"), "tools", ["slug"], unique=False)
+    # ### end Alembic commands ###
+
+
+def downgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.drop_index(op.f("ix_tools_slug"), table_name="tools")
+    op.create_index("ix_tools_slug", "tools", ["slug"], unique=True)
+    op.drop_index(op.f("ix_tools_name"), table_name="tools")
+    op.create_index("ix_tools_name", "tools", ["name"], unique=True)
+    # ### end Alembic commands ###

mealie/db/models/recipe/tool.py

@@ -36,8 +36,8 @@ class Tool(SqlAlchemyBase, BaseMixins):
     group_id: Mapped[GUID] = mapped_column(GUID, ForeignKey("groups.id"), nullable=False, index=True)
     group: Mapped["Group"] = orm.relationship("Group", back_populates="tools", foreign_keys=[group_id])
 
-    name: Mapped[str] = mapped_column(String, index=True, unique=True, nullable=False)
+    name: Mapped[str] = mapped_column(String, index=True, nullable=False)
-    slug: Mapped[str] = mapped_column(String, index=True, unique=True, nullable=False)
+    slug: Mapped[str] = mapped_column(String, index=True, nullable=False)
     on_hand: Mapped[bool | None] = mapped_column(Boolean, default=False)
     recipes: Mapped[list["RecipeModel"]] = orm.relationship(
         "RecipeModel", secondary=recipes_to_tools, back_populates="tools"

tests/multitenant_tests/case_abc.py

@@ -10,17 +10,18 @@ class ABCMultiTenantTestCase(ABC):
     def __init__(self, database: AllRepositories, client: TestClient) -> None:
         self.database = database
         self.client = client
-        self.items = []
+        self.items: list = []
 
     @abstractmethod
-    def seed_action(repos: AllRepositories, group_id: str) -> set[int] | set[str]:
+    def seed_action(self, group_id: str) -> set[int] | set[str]:
         ...
 
-    def seed_multi(self, group1_id: str, group2_id: str) -> tuple[set[int], set[int]]:
+    @abstractmethod
-        pass
+    def seed_multi(self, group1_id: str, group2_id: str) -> tuple[set[str], set[str]]:
+        ...
 
     @abstractmethod
-    def get_all(token: str) -> Response:
+    def get_all(self, token: str) -> Response:
         ...
 
     @abstractmethod

tests/multitenant_tests/case_categories.py

@@ -29,9 +29,9 @@ class CategoryTestCase(ABCMultiTenantTestCase):
         g1_item_ids: set[str] = set()
         g2_item_ids: set[str] = set()
 
-        for group_id, item_ids in [(group1_id, g1_item_ids), (group2_id, g2_item_ids)]:
         for _ in range(10):
             name = utils.random_string(10)
+            for group_id, item_ids in [(group1_id, g1_item_ids), (group2_id, g2_item_ids)]:
                 category = self.database.categories.create(
                     CategorySave(
                         group_id=group_id,

tests/multitenant_tests/case_foods.py

@@ -28,9 +28,9 @@ class FoodsTestCase(ABCMultiTenantTestCase):
         g1_item_ids: set[str] = set()
         g2_item_ids: set[str] = set()
 
-        for group_id, item_ids in [(group1_id, g1_item_ids), (group2_id, g2_item_ids)]:
         for _ in range(10):
             name = utils.random_string(10)
+            for group_id, item_ids in [(group1_id, g1_item_ids), (group2_id, g2_item_ids)]:
                 food = self.database.ingredient_foods.create(
                     SaveIngredientFood(
                         group_id=group_id,

tests/multitenant_tests/case_tags.py

@@ -29,9 +29,9 @@ class TagsTestCase(ABCMultiTenantTestCase):
         g1_item_ids: set[str] = set()
         g2_item_ids: set[str] = set()
 
-        for group_id, item_ids in [(group1_id, g1_item_ids), (group2_id, g2_item_ids)]:
         for _ in range(10):
             name = utils.random_string(10)
+            for group_id, item_ids in [(group1_id, g1_item_ids), (group2_id, g2_item_ids)]:
                 category = self.database.tags.create(
                     TagSave(
                         group_id=group_id,

tests/multitenant_tests/case_tools.py

@@ -29,9 +29,9 @@ class ToolsTestCase(ABCMultiTenantTestCase):
         g1_item_ids: set[str] = set()
         g2_item_ids: set[str] = set()
 
-        for group_id, item_ids in [(group1_id, g1_item_ids), (group2_id, g2_item_ids)]:
         for _ in range(10):
             name = utils.random_string(10)
+            for group_id, item_ids in [(group1_id, g1_item_ids), (group2_id, g2_item_ids)]:
                 tool = self.database.tools.create(
                     RecipeToolSave(
                         group_id=group_id,

tests/multitenant_tests/case_units.py

@@ -28,9 +28,9 @@ class UnitsTestCase(ABCMultiTenantTestCase):
         g1_item_ids: set[str] = set()
         g2_item_ids: set[str] = set()
 
-        for group_id, item_ids in [(group1_id, g1_item_ids), (group2_id, g2_item_ids)]:
         for _ in range(10):
             name = utils.random_string(10)
+            for group_id, item_ids in [(group1_id, g1_item_ids), (group2_id, g2_item_ids)]:
                 food = self.database.ingredient_units.create(
                     SaveIngredientUnit(
                         group_id=group_id,

tests/multitenant_tests/test_multitenant_cases.py

@@ -19,12 +19,12 @@ all_cases = [
 ]
 
 
-@pytest.mark.parametrize("test_case", all_cases)
+@pytest.mark.parametrize("test_case_type", all_cases)
 def test_multitenant_cases_get_all(
     api_client: TestClient,
     multitenants: MultiTenant,
     database: AllRepositories,
-    test_case: type[ABCMultiTenantTestCase],
+    test_case_type: type[ABCMultiTenantTestCase],
 ):
     """
     This test will run all the multitenant test cases and validate that they return only the data for their group.
@@ -34,11 +34,11 @@ def test_multitenant_cases_get_all(
     user1 = multitenants.user_one
     user2 = multitenants.user_two
 
-    test_case = test_case(database, api_client)
+    test_case = test_case_type(database, api_client)
 
     with test_case:
         expected_ids = test_case.seed_action(user1.group_id)
-        expected_results = [
+        expected_results: list = [
             (user1.token, expected_ids),
             (user2.token, []),
         ]
@@ -56,12 +56,12 @@ def test_multitenant_cases_get_all(
                     assert item["id"] in item_ids
 
 
-@pytest.mark.parametrize("test_case", all_cases)
+@pytest.mark.parametrize("test_case_type", all_cases)
 def test_multitenant_cases_same_named_resources(
     api_client: TestClient,
     multitenants: MultiTenant,
     database: AllRepositories,
-    test_case: type[ABCMultiTenantTestCase],
+    test_case_type: type[ABCMultiTenantTestCase],
 ):
     """
     This test is used to ensure that the same resource can be created with the same values in different tenants.
@@ -71,7 +71,7 @@ def test_multitenant_cases_same_named_resources(
     user1 = multitenants.user_one
     user2 = multitenants.user_two
 
-    test_case = test_case(database, api_client)
+    test_case = test_case_type(database, api_client)
 
     with test_case:
         expected_ids, expected_ids2 = test_case.seed_multi(user1.group_id, user2.group_id)