From 971218eabee65a2e412b7f5ed8466ff4f33a4eb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roger=20Boixader=20G=C3=BCell?= Date: Sat, 2 Dec 2023 16:07:14 +0100 Subject: [PATCH 1/2] fix: update groups with standard api --- guillotina/contrib/dbusers/subscribers.py | 25 +++-- .../tests/dbusers/test_manage_groups.py | 104 +++++++++++++++++- 2 files changed, 119 insertions(+), 10 deletions(-) diff --git a/guillotina/contrib/dbusers/subscribers.py b/guillotina/contrib/dbusers/subscribers.py index 55273d232..63d5be60b 100644 --- a/guillotina/contrib/dbusers/subscribers.py +++ b/guillotina/contrib/dbusers/subscribers.py @@ -64,17 +64,26 @@ async def on_group_removed(group: Group, event: ObjectAddedEvent) -> None: @configure.subscriber(for_=(IGroup, IBeforeObjectModifiedEvent)) async def on_group_modified(group: Group, event: BeforeObjectModifiedEvent) -> None: # keep group.users updated with changes from users - old_users = group.users or [] - users_added = set() - set(old_users) - users_removed = set(old_users) - set() + users_added = [] + users_removed = [] changes = event.payload.get("users", None) if changes is None: return - for user, is_new in changes.items(): - if is_new: - users_added.add(user) - else: - users_removed.add(user) + + if isinstance(changes, list): + for user in changes: + if user not in group.users: + users_added.append(user) + + for user in group.users: + if user not in changes: + users_removed.append(user) + else: + for user, is_new in changes.items(): + if is_new and user not in group.users: + users_added.append(user) + elif not is_new: + users_removed.append(user) await _update_users(group.id, users_added, users_removed) diff --git a/guillotina/tests/dbusers/test_manage_groups.py b/guillotina/tests/dbusers/test_manage_groups.py index 29b514454..6c72ca264 100644 --- a/guillotina/tests/dbusers/test_manage_groups.py +++ b/guillotina/tests/dbusers/test_manage_groups.py @@ -17,8 +17,13 @@ async def user_data(): return settings.user_data.copy() +@pytest.fixture() +async def second_user_data(): + return settings.second_user_data.copy() + + @pytest.mark.app_settings(settings.DEFAULT_SETTINGS) -async def test_ensure_crud_groups(dbusers_requester, user_data): +async def test_ensure_crud_groups(dbusers_requester, user_data, second_user_data): async with dbusers_requester as requester: resp, status_code = await requester("POST", "/db/guillotina/groups", data=json.dumps(_group)) assert status_code == 201 @@ -43,6 +48,7 @@ async def test_ensure_crud_groups(dbusers_requester, user_data): # create the user resp, status_code = await requester("GET", "/db/guillotina/users") resp, status_code = await requester("POST", "/db/guillotina/users", data=json.dumps(user_data)) + resp, status_code = await requester("POST", "/db/guillotina/users", data=json.dumps(second_user_data)) data = {"users": {"foobar": True}} resp, status = await requester("PATCH", "/db/guillotina/@groups/foo", data=json.dumps(data)) @@ -60,13 +66,33 @@ async def test_ensure_crud_groups(dbusers_requester, user_data): resp, status = await requester("GET", "/db/guillotina/users/foobar") assert resp["user_groups"] == ["foo"] - data = {"users": {"foobar": False}} + + data = {"users": {"foobar": True}} + resp, status = await requester("PATCH", "/db/guillotina/@groups/foo", data=json.dumps(data)) + assert status == 204 + + resp, status = await requester("GET", "/db/guillotina/users/foobar") + assert resp["user_groups"] == ["foo"] + + data = {"users": {"foobar": False, "foobar_2": True}} + resp, status = await requester("PATCH", "/db/guillotina/@groups/foo", data=json.dumps(data)) + assert status == 204 + resp, status = await requester("GET", "/db/guillotina/@groups/foo") + assert len(resp["users"]["items"]) == 1 + resp, status = await requester("GET", "/db/guillotina/users/foobar") + assert resp["user_groups"] == [] + resp, status = await requester("GET", "/db/guillotina/users/foobar_2") + assert resp["user_groups"] == ["foo"] + + data = {"users": {"foobar": False, "foobar_2": False}} resp, status = await requester("PATCH", "/db/guillotina/@groups/foo", data=json.dumps(data)) assert status == 204 resp, status = await requester("GET", "/db/guillotina/@groups/foo") assert len(resp["users"]["items"]) == 0 resp, status = await requester("GET", "/db/guillotina/users/foobar") assert resp["user_groups"] == [] + resp, status = await requester("GET", "/db/guillotina/users/foobar_2") + assert resp["user_groups"] == [] # ensure we cannot patch invalid users data = {"users": {"foobarx": True}} @@ -74,6 +100,80 @@ async def test_ensure_crud_groups(dbusers_requester, user_data): assert status == 412 +@pytest.mark.app_settings(settings.DEFAULT_SETTINGS) +async def test_ensure_crud_groups_using_standard_api(dbusers_requester, user_data, second_user_data): + async with dbusers_requester as requester: + resp, status_code = await requester("POST", "/db/guillotina/groups", data=json.dumps(_group)) + assert status_code == 201 + + data = {"user_roles": ["guillotina.Manager", "guillotina.Tester"]} + + resp, status = await requester("PATCH", "/db/guillotina/groups/foo", data=json.dumps(data)) + assert status == 204 + resp, status = await requester("GET", "/db/guillotina/groups/foo") + assert set(resp["user_roles"]) == set(["guillotina.Manager", "guillotina.Tester"]) + + data = {"user_roles": ["guillotina.Tester"]} + resp, status = await requester("PATCH", "/db/guillotina/groups/foo", data=json.dumps(data)) + assert status == 204 + resp, status = await requester("GET", "/db/guillotina/groups/foo") + assert set(resp["user_roles"]) == set(["guillotina.Tester"]) + + # create the user + resp, status_code = await requester("GET", "/db/guillotina/users") + resp, status_code = await requester("POST", "/db/guillotina/users", data=json.dumps(user_data)) + resp, status_code = await requester("POST", "/db/guillotina/users", data=json.dumps(second_user_data)) + + data = {"users": ["foobar"]} + resp, status = await requester("PATCH", "/db/guillotina/groups/foo", data=json.dumps(data)) + assert status == 204 + resp, status = await requester("GET", "/db/guillotina/groups/foo") + assert resp["users"] == ["foobar"] + + # fix bug https://github.com/plone/guillotina/issues/1069 + resp, status = await requester( + "PATCH", "/db/guillotina/groups/foo", data=json.dumps({"user_roles": ["guillotina.Reader"]}) + ) + assert status == 204 + resp, status = await requester("GET", "/db/guillotina/groups/foo") + assert resp["users"] == ["foobar"] + + resp, status = await requester("GET", "/db/guillotina/users/foobar") + assert resp["user_groups"] == ["foo"] + + data = {"users": ["foobar"]} + resp, status = await requester("PATCH", "/db/guillotina/groups/foo", data=json.dumps(data)) + assert status == 204 + + resp, status = await requester("GET", "/db/guillotina/users/foobar") + assert resp["user_groups"] == ["foo"] + + data = {"users": ["foobar_2"]} + resp, status = await requester("PATCH", "/db/guillotina/groups/foo", data=json.dumps(data)) + assert status == 204 + resp, status = await requester("GET", "/db/guillotina/groups/foo") + assert len(resp["users"]) == 1 + resp, status = await requester("GET", "/db/guillotina/users/foobar") + assert resp["user_groups"] == [] + resp, status = await requester("GET", "/db/guillotina/users/foobar_2") + assert resp["user_groups"] == ["foo"] + + data = {"users": []} + resp, status = await requester("PATCH", "/db/guillotina/groups/foo", data=json.dumps(data)) + assert status == 204 + resp, status = await requester("GET", "/db/guillotina/groups/foo") + assert len(resp["users"]) == 0 + resp, status = await requester("GET", "/db/guillotina/users/foobar") + assert resp["user_groups"] == [] + resp, status = await requester("GET", "/db/guillotina/users/foobar_2") + assert resp["user_groups"] == [] + + # ensure we cannot patch invalid users + data = {"users": {"foobarx": True}} + resp, status = await requester("PATCH", "/db/guillotina/groups/foo", data=json.dumps(data)) + assert status == 412 + + settings_with_catalog = copy.deepcopy(settings.DEFAULT_SETTINGS) settings_with_catalog["applications"].append("guillotina.contrib.catalog.pg") settings_with_catalog.setdefault("load_utilities", {}) # type: ignore From 602083afd1e82d03a13b85f217f97b74ae6f9a5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roger=20Boixader=20G=C3=BCell?= Date: Sat, 2 Dec 2023 16:07:21 +0100 Subject: [PATCH 2/2] Fix: Delete indexed_name property in indexes in User and Group interfaces --- CHANGELOG.rst | 6 ++ guillotina/contrib/dbusers/content/groups.py | 6 +- guillotina/contrib/dbusers/content/users.py | 4 +- guillotina/contrib/dbusers/services/groups.py | 6 +- guillotina/contrib/dbusers/services/users.py | 4 +- guillotina/contrib/dbusers/users.py | 2 +- guillotina/tests/dbusers/settings.py | 9 ++ guillotina/tests/dbusers/test_search.py | 83 +++++++++++++++++++ 8 files changed, 109 insertions(+), 11 deletions(-) create mode 100644 guillotina/tests/dbusers/test_search.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 40ab10668..d8459a35b 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -10,6 +10,12 @@ CHANGELOG - Chore: Update multidict dependency [rboixaderg] +- Fix: Update group in standard API + [rboixaderg] + +- Fix: Delete indexed_name property in indexes in User and Group interfaces + [rboixaderg] + 6.4.4 (2023-11-20) ------------------ diff --git a/guillotina/contrib/dbusers/content/groups.py b/guillotina/contrib/dbusers/content/groups.py index 741eb9041..29e2dde60 100644 --- a/guillotina/contrib/dbusers/content/groups.py +++ b/guillotina/contrib/dbusers/content/groups.py @@ -13,15 +13,15 @@ class IGroupManager(IFolder): class IGroup(IFolder, IPrincipal): - index_field("name", index_name="group_name", type="textkeyword") + index_field("name", type="searchabletext") name = schema.TextLine(title=_("Group name"), required=False) description = schema.TextLine(title=_("Group Description"), required=False) - index_field("user_roles", index_name="group_user_roles", type="textkeyword") + index_field("user_roles", type="textkeyword") user_roles = schema.List(title=_("Roles"), value_type=schema.TextLine(), required=False) - index_field("users", index_name="group_users", type="textkeyword") + index_field("users", type="textkeyword") users = schema.List(title=_("Users"), value_type=schema.TextLine(), required=False, default=[]) diff --git a/guillotina/contrib/dbusers/content/users.py b/guillotina/contrib/dbusers/content/users.py index 89ed104e9..7be21ee3e 100644 --- a/guillotina/contrib/dbusers/content/users.py +++ b/guillotina/contrib/dbusers/content/users.py @@ -21,10 +21,10 @@ class IUser(IFolder, IPrincipal): username = schema.TextLine(title=_("Username"), required=False) - index_field("email", index_name="user_email", type="keyword") + index_field("email", type="keyword") email = schema.TextLine(title=_("Email"), required=False) - index_field("name", index_name="user_name", type="textkeyword") + index_field("name", type="searchabletext") name = schema.TextLine(title=_("Name"), required=False) read_permission(password="guillotina.Nobody") diff --git a/guillotina/contrib/dbusers/services/groups.py b/guillotina/contrib/dbusers/services/groups.py index 44dfe965d..abb71e76f 100644 --- a/guillotina/contrib/dbusers/services/groups.py +++ b/guillotina/contrib/dbusers/services/groups.py @@ -42,9 +42,9 @@ async def process_catalog_obj(self, obj) -> dict: return { "@name": obj.get("@name"), "id": obj.get("id"), - "title": obj.get("group_name"), - "users": obj.get("group_users") or [], - "roles": obj.get("group_user_roles") or [], + "title": obj.get("name"), + "users": obj.get("users") or [], + "roles": obj.get("user_roles") or [], } diff --git a/guillotina/contrib/dbusers/services/users.py b/guillotina/contrib/dbusers/services/users.py index 2215a5fca..821272ae6 100644 --- a/guillotina/contrib/dbusers/services/users.py +++ b/guillotina/contrib/dbusers/services/users.py @@ -131,7 +131,7 @@ async def process_catalog_obj(self, obj) -> dict: return { "@name": obj.get("@name"), "id": obj.get("id"), - "fullname": obj.get("user_name"), - "email": obj.get("user_email"), + "fullname": obj.get("name"), + "email": obj.get("email"), "roles": obj.get("user_roles") or [], } diff --git a/guillotina/contrib/dbusers/users.py b/guillotina/contrib/dbusers/users.py index 5de8c8678..a04530d13 100644 --- a/guillotina/contrib/dbusers/users.py +++ b/guillotina/contrib/dbusers/users.py @@ -77,7 +77,7 @@ async def get_user(self, token: typing.Dict) -> typing.Optional[IPrincipal]: json->>'type_name' = 'User' AND parent_id != 'DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD' AND json->>'container_id' = $1::varchar - AND lower(json->>'user_email') = lower($2::varchar) + AND lower(json->>'email') = lower($2::varchar) """ async with txn.lock: row = await conn.fetchrow(sql, container.id, token.get("id")) diff --git a/guillotina/tests/dbusers/settings.py b/guillotina/tests/dbusers/settings.py index 15a15842f..cdc6e45de 100644 --- a/guillotina/tests/dbusers/settings.py +++ b/guillotina/tests/dbusers/settings.py @@ -27,6 +27,15 @@ "password": "password", } +second_user_data = { + "@type": "User", + "name": "Second user name", + "id": "foobar_2", + "username": "foobar_2", + "email": "foo2@bar.com", + "password": "password", +} + user_data_id_email = { "@type": "User", "name": "Foobar", diff --git a/guillotina/tests/dbusers/test_search.py b/guillotina/tests/dbusers/test_search.py new file mode 100644 index 000000000..e0d2bb611 --- /dev/null +++ b/guillotina/tests/dbusers/test_search.py @@ -0,0 +1,83 @@ +from . import settings + +import copy +import json +import os +import pytest + + +pytestmark = pytest.mark.asyncio + +NOT_POSTGRES = os.environ.get("DATABASE", "DUMMY") in ("cockroachdb", "DUMMY") +PG_CATALOG_SETTINGS = copy.deepcopy(settings.DEFAULT_SETTINGS) +PG_CATALOG_SETTINGS["applications"].append("guillotina.contrib.catalog.pg") +PG_CATALOG_SETTINGS.setdefault("load_utilities", {}) # type: ignore +PG_CATALOG_SETTINGS["load_utilities"]["catalog"] = { # type: ignore + "provides": "guillotina.interfaces.ICatalogUtility", + "factory": "guillotina.contrib.catalog.pg.utility.PGSearchUtility", +} + + +@pytest.mark.app_settings(PG_CATALOG_SETTINGS) +@pytest.mark.skipif(NOT_POSTGRES, reason="Only PG") +async def test_search_user(dbusers_requester): + async with dbusers_requester as requester: + # Create a user + _, status_code = await requester("POST", "/db/guillotina/users", data=json.dumps(settings.user_data)) + assert status_code == 201 + + _, status_code = await requester( + "POST", "/db/guillotina/users", data=json.dumps(settings.second_user_data) + ) + assert status_code == 201 + + resp, status_code = await requester("GET", "/db/guillotina/@search?type_name=User") + assert status_code == 200 + assert resp["items_total"] == 2 + assert "name" in resp["items"][0] + assert "email" in resp["items"][0] + + resp, status_code = await requester("GET", "/db/guillotina/@search?type_name=User&name=user") + assert status_code == 200 + assert resp["items_total"] == 1 + + resp, status_code = await requester("GET", "/db/guillotina/@search?type_name=User&name=foobar") + assert status_code == 200 + assert resp["items_total"] == 1 + + resp, status_code = await requester("GET", "/db/guillotina/@search?type_name=User&name=unknownname") + assert status_code == 200 + assert resp["items_total"] == 0 + + resp, status_code = await requester("GET", "/db/guillotina/@search?type_name=User&email=foo@bar.com") + assert status_code == 200 + assert resp["items_total"] == 1 + + resp, status_code = await requester( + "GET", "/db/guillotina/@search?type_name=User&email=no-email@bar.com" + ) + assert status_code == 200 + assert resp["items_total"] == 0 + + +@pytest.mark.app_settings(PG_CATALOG_SETTINGS) +@pytest.mark.skipif(NOT_POSTGRES, reason="Only PG") +async def test_search_groups(dbusers_requester): + async with dbusers_requester as requester: + _, status_code = await requester( + "POST", "/db/guillotina/groups", data=json.dumps(settings.group_data) + ) + assert status_code == 201 + + resp, status_code = await requester("GET", "/db/guillotina/@search?type_name=Group") + assert status_code == 200 + assert resp["items_total"] == 1 + assert "name" in resp["items"][0] + + resp, status_code = await requester("GET", "/db/guillotina/@search?type_name=Group&name=foobar") + assert status_code == 200 + assert resp["items_total"] == 1 + + resp, status_code = await requester("GET", "/db/guillotina/@search?type_name=Group&name=unknownname") + assert status_code == 200 + assert resp["items_total"] == 0