global: improved domain/user indexing, data flow, search and REST API #124

lnielsen · 2024-02-12T09:22:03Z

Description

Depends on inveniosoftware/invenio-accounts#475

Add domains REST api and underlying service.

Refactors data flow and indexing so that the aggregate data model is in charge of all data parsing of the user and role model as well as indexing
Adds domain data and user identities and further attributes to the users index and makes them searchable for admins.
Fixes indexing/facets of email domain values.
Allows admins to search for restricted email addresses.
Add admin facets for domain, account status, domain status.
Add sort options to admin user search.
INCOMPATIBLE: There's a new users index, so when deployed all users will have to be reindexed, and not sure how the code will work with having users from both indexes at the same time. Probably requires downtime to deploy.

Note, right now the denormalised information in the index about the domain is not updated when the domain is updated. However, as soon as a user logins, update profile etc it is updated. Later we can add a task that one a domainn is updated we also reindex the users.

closes User and domain administration interface improvements. invenio-app-rdm#2575

Checklist

Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:

Third-party code

If you've added third-party code (copy/pasted or new dependencies), please reach out to an architect.

Reminder

By using GitHub, you have already agreed to the GitHub’s Terms of Service including that:

You license your contribution under the same terms as the current repository’s license.
You agree that you have the right to license your contribution under the current repository’s license.

lnielsen · 2024-02-12T09:35:07Z

Changes in user resources is used for improving the admin experience:

Users admin

Domains admin

lnielsen · 2024-02-12T09:39:41Z

invenio_users_resources/records/api.py

@@ -8,62 +8,111 @@
 # details.

 """API classes for user and group management in Invenio."""
-import random
+


Major changes in this file and models.py to make make the record classes behave more like normal records, and to clearly separating the responsibilities of parsing the user/role/domain objects from Invenio-accounts.

Now UserAggregate/GroupAggregate/DomainAggregate are records that is responsible for harmonising data access from search index and DB. the <Model>AggregateModel objects are responsible for metadata storage and for parsing the Invenio-accounts user/role/domain models.

lnielsen · 2024-02-12T09:40:19Z

invenio_users_resources/records/api.py

+"""Emulated PID"""
+
+
+class AggregatePID:


Provides an interface that is needed by Records-Resources service layer (otherwise we have to implement all the service methods).

lnielsen · 2024-02-12T09:41:11Z

invenio_users_resources/records/dumpers/email.py

@@ -28,6 +28,7 @@ def dump(self, record, data):
        email_visible = record.preferences["email_visibility"]
        if email_visible == "public":
            data[self.field] = email
+            data[self.hidden_field] = email


We now always index the email address in the hidden field as well so we can make it searchable for admins.

~~the mappings still have~~

"email_hidden": { "type": "keyword", "index": "false" },

~~which means the field won't be indexed, is it intentional ?~~

EDIT: just saw the new mapping, all good, ignore the above

nit: we could remove the else below and dump the hidden field at all times?

ATTENTION for maintainers: will need reindexing

lnielsen · 2024-02-12T09:41:57Z

invenio_users_resources/records/mappings/os-v2/users/user-v1.0.0.json

@@ -1,31 +1,4 @@
 {
-  "settings": {


This is all moved to the python side now

we shouldn't modify the mapping since we add the v2.0.0, otherwise we lose the backwards compatibility, no? Unless I have misunderstood the intention of this change

Correct, this is a mistake 👍 The mapping should not be modified

lnielsen · 2024-02-12T09:43:03Z

invenio_users_resources/resources/domains/resource.py

@@ -0,0 +1,33 @@
+# -*- coding: utf-8 -*-
+#
+# Copyright (C) 2022 TU Wien.


TODO: Fix header

lnielsen · 2024-02-12T09:43:57Z

invenio_users_resources/resources/users/config.py

@@ -35,12 +39,13 @@ class UsersResourceConfig(RecordResourceConfig):
    url_prefix = "/users"
    routes = {
        "list": "",
-        "moderation_search": "/moderation",
+        "search_all": "/all",


Comment: Changing name here, because the API is more than just moderation, it's provides all the admin interface with all the user information

lnielsen · 2024-02-12T09:46:05Z

invenio_users_resources/services/schemas.py

+
+    @validates_schema
+    def validate_props(self, data, **kwargs):
+        """Apply instance specific validation on props."""


We allow each instance to define their own schema for extra properties on a domain (by default just stores the country).

lnielsen · 2024-02-12T09:47:27Z

invenio_users_resources/services/users/config.py

+    return current_users_service.check_permission(ctx["identity"], "manage")
+
+
+def word_domain_status(node):


allows searching uses from their domain status: domain.status:verified

lnielsen · 2024-02-12T09:48:03Z

invenio_users_resources/services/users/config.py

@@ -63,41 +91,69 @@ class UserSearchOptions(SearchOptions, SearchOptionsMixin):
        SortParam,
        FixedPagination,
        FacetsParam,
-        ModerationFilterParam.factory(param="is_blocked", field="blocked_at"),


Removing this, because these shouldnt' be available on he API used for searching members to invite.

lnielsen · 2024-02-12T09:48:18Z

invenio_users_resources/services/users/config.py

    ]

-    facets = {
-        "email_domain": facets.email_domain,


We should also not have faces on the API where users can search for members.

lnielsen · 2024-02-12T09:49:14Z

invenio_users_resources/services/users/service.py

@@ -145,16 +138,13 @@ def block(self, identity, id_, uow=None):

        self.require_permission(identity, "manage", record=user)

+        if user.blocked:


TODO: We should not allow an admin to block themselves as well.

lnielsen · 2024-02-12T09:51:00Z

tests/services/users/test_user_moderation.py


 from invenio_users_resources.proxies import current_actions_registry
 from invenio_users_resources.services.users.lock import ModerationMutex


+@pytest.fixture()


Previously the service did not check if a user was already blocked, now that it checks the test code doesnt' work anymore because the same users is being blocked multiple times, so this was a quick way of fixing the test to work.

invenio_users_resources/records/api.py

invenio_users_resources/records/models.py

invenio_users_resources/records/systemfields/__init__.py

invenio_users_resources/resources/domains/resource.py

invenio_users_resources/services/domains/__init__.py

invenio_users_resources/services/domains/config.py

invenio_users_resources/services/domains/service.py

invenio_users_resources/services/domains/tasks.py

invenio_users_resources/services/schemas.py

invenio_users_resources/services/users/config.py

Samk13 · 2024-02-13T01:29:08Z

I recognize my approaches may seem detail-oriented, but it's in service of our collective effort to enhance the project. I welcome any discussions on these.
Thanks for this amazing feature!

kpsherva · 2024-02-14T13:12:19Z

invenio_users_resources/services/users/config.py

    QueryParser,
    SearchFieldTransformer,
 )
+from luqum.tree import Word


minor: direct import from implicit dependency (installed by another module, records-resources)

I think this is ok, because Records-Resources is in charge of setting the right dependency of luqum - otherwise we get issues later on, when we have to upgrade luqum in all places where it is used.

kpsherva · 2024-02-14T13:12:50Z

invenio_users_resources/services/users/config.py

 class UserSearchOptions(SearchOptions, SearchOptionsMixin):
    """Search options."""

    pagination_options = {
        "default_results_per_page": 10,
        "default_max_results": 10,
    }
+    # ATTENTION: Risk of leaking account information!!!


lnielsen · 2024-02-19T11:44:44Z

I recognize my approaches may seem detail-oriented, but it's in service of our collective effort to enhance the project. I welcome any discussions on these. Thanks for this amazing feature!

Thanks for the review! I've integrated the changes now.

* Refactors data flow and indexing so that the aggregate data model is in charge of all data parsing of the user and role model as well as indexing * Adds domain data and user identities and further attributes to the users index and makes them searchable for admins. * Fixes indexing/facets of email domain values. * Allows admins to search for restricted email addresses. * Add admin facets for domain, account status, domain status. * Add sort options to admin user search.

Co-authored-by: Karolina Przerwa <[email protected]> Co-authored-by: Sam Arbid <[email protected]>

lnielsen commented Feb 12, 2024

View reviewed changes

lnielsen mentioned this pull request Feb 12, 2024

Users admin inveniosoftware/invenio-app-rdm#2574

Merged

Samk13 reviewed Feb 12, 2024

View reviewed changes

invenio_users_resources/records/api.py Show resolved Hide resolved

Samk13 reviewed Feb 12, 2024

View reviewed changes

invenio_users_resources/records/api.py Outdated Show resolved Hide resolved

Samk13 reviewed Feb 12, 2024

View reviewed changes

invenio_users_resources/records/models.py Show resolved Hide resolved

Samk13 reviewed Feb 12, 2024

View reviewed changes

invenio_users_resources/records/models.py Show resolved Hide resolved