moderation: added request type and service #335

alejandromumo · 2023-07-19T14:12:46Z

closes #334

NEEDS:

users: added moderation actions invenio-users-resources#91

ntarocco · 2023-07-20T11:57:03Z

invenio_requests/services/user_moderation/service.py

+from invenio_requests.services.user_moderation.user_moderation import UserModeration
+
+
+class UserModerationRequestService:


Question: why creating a specific service? Is it really needed?
The requests module has been designed to be generic, and each service method accepts a request_type that should allow you to create/search/do operations on a concrete request type.

~~I had to go a bit further and implement this service as a Service, mainly due to permissions and search.~~

User moderation had custom business logic (e.g. configurable receiver) and I felt it was better to have a tiny service layer.

invenio_requests/services/user_moderation/user_moderation.py

ntarocco · 2023-07-20T12:03:27Z

invenio_requests/services/user_moderation/user_moderation.py

+    creator_can_be_none = False
+    topic_can_be_none = False
+    allowed_creator_ref_types = ["user"]
+    allowed_receiver_ref_types = ["user"]


The default permissions are allowing the creator/receiver to perform actions, see permissions. You will have to override this behaviour and define who can do that, including the read/search of these requests.
You might want to create a new ad-hoc action, similar to administration, so that we can be flexible and delegate the user moderation to a specific set of people. This allows in the future to, for example, have a second line support managing this part.

Yes, I need to review permissions in the next iteration. I will implement the actions (accept, decline) and configure the permissions.

About the invenio-administration generator, I tried to use it in user's service but it was not working. Namely, if the generators of an action (e.g. search) have more than the administration (Administration()) then it would disappear and raise permission errors. As far as I debugged, this was happening here.

In the end, I am using SystemProcess(). But I do agree we need to review this and understand if that's good enough for the user moderation actions.

When you get there, maybe we can discuss it quickly, having a look to some code. In any case, Administration role must have access to the request, otherwise these users cannot perform searches and see it in the admin panel.

ntarocco · 2023-07-20T12:04:40Z

tests/services/user_moderation/test_user_moderation_service.py

+from invenio_requests.services.user_moderation.errors import InvalidCreator
+
+
+def test_request_moderation(app, users, identity_simple):


Nice! When you will have the resources, make sure that you add extensive tests there (we aligned on having more tests at the resource level than service level, when possible), and that you also test permission.

I added new tests, both for the resource and service.

alejandromumo · 2023-07-25T08:49:25Z

Tests are passing locally. The are failing on CI because it needs another PR (users-resources).

invenio_requests/customizations/user_moderation/user_moderation.py

invenio_requests/resources/user_moderation/resource.py

invenio_requests/services/user_moderation/service.py

invenio_requests/customizations/user_moderation/user_moderation.py

invenio_requests/resources/user_moderation/config.py

lnielsen · 2023-07-25T13:47:28Z

invenio_requests/services/user_moderation/service.py

+        if creator != system_user_id:
+            raise InvalidCreator(_("Moderation request creator can only be system."))
+
+        data = data or {}


Question: Is data being validated somewhere - e.g in the request_service?

Indeed, data is validated by the requests_service

invenio_requests/services/user_moderation/service.py

lnielsen · 2023-08-01T10:23:30Z

invenio_requests/services/user_moderation/service.py

+        data = data or {}
+
+        # For user moderation, topic is the user to be moderated
+        topic = {"user": str(topic)}


Question: Should the service method hardcode that we want a user and check that the user exists? Might be this is the end-result but not very clear from here, so if the case, then add a comment.

I am not sure I understood the question.

But the requests service resolves the entity if:

it's a dict (like above) and matches the entity dict signature

it's an instance of the entity (e.g. User() or Role()).

lnielsen · 2023-08-01T10:23:52Z

invenio_requests/services/user_moderation/service.py

+
+        # Receiver can be configured, by default send the request to users with moderation role
+        receiver = {"group": role.name}  # TODO to be changed to role id
+        creator = {"group": role.name}  # TODO to be changed to role id


Question: Should this be the system user instead?

I considered this case, however right now a "system user" is considered to be a "User" entity (only on entity resolve though)-

The user entity resolver is currently resolving both "normal" users and "system" user (system_user_id), which means that, if the creator entity is "user", then a normal user would also be allowed to be the creator and do things on these type of requests (e.g. read, update).

To allow only "system", we might need a "system user" entity resolver that does not resolve regular users.

If this is something we want, I would create an issue and tackle it separately.

zzacharo · 2023-08-02T08:32:51Z

invenio_requests/customizations/user_moderation/user_moderation.py

+    available_actions = {
+        "delete": actions.DeleteAction,
+        "submit": actions.SubmitAction,
+        "create": actions.CreateAction,


nit: can we do something like

Suggested change

"create": actions.CreateAction,

**RequestType.available_actions,

...

?

invenio_requests/services/user_moderation/service.py

ntarocco reviewed Jul 20, 2023

View reviewed changes

alejandromumo force-pushed the add_user_moderation branch from 5e50125 to c22ab3d Compare July 20, 2023 13:44

alejandromumo self-assigned this Jul 21, 2023

alejandromumo force-pushed the add_user_moderation branch 3 times, most recently from 1d23eeb to a59b49b Compare July 25, 2023 07:27

alejandromumo commented Jul 25, 2023

View reviewed changes

invenio_requests/customizations/user_moderation/user_moderation.py Outdated Show resolved Hide resolved

alejandromumo commented Jul 25, 2023

View reviewed changes

invenio_requests/resources/user_moderation/resource.py Outdated Show resolved Hide resolved

alejandromumo commented Jul 25, 2023

View reviewed changes

invenio_requests/services/user_moderation/service.py Outdated Show resolved Hide resolved

alejandromumo force-pushed the add_user_moderation branch 4 times, most recently from 4a99ea7 to 06d2472 Compare July 27, 2023 15:20

alejandromumo mentioned this pull request Jul 27, 2023

Refactor roleneed roleid #338

Open

alejandromumo force-pushed the add_user_moderation branch from 06d2472 to ea87f64 Compare July 28, 2023 08:33

lnielsen reviewed Aug 1, 2023

View reviewed changes

alejandromumo force-pushed the add_user_moderation branch from ea87f64 to 12c0999 Compare August 1, 2023 14:06

zzacharo reviewed Aug 2, 2023

View reviewed changes

alejandromumo added 5 commits August 2, 2023 10:49

moderation: added request type and service

fef987b

moderation: added resource and permisisons

a976838

moderation: updated service structure and permissions

4ee2b5b

permissions: refactor entity resolver to Role

f64d7ae

moderation: removed resource & refactored service

f6920c0

alejandromumo force-pushed the add_user_moderation branch from 12c0999 to f6920c0 Compare August 2, 2023 09:43

zzacharo approved these changes Aug 2, 2023

View reviewed changes

zzacharo merged commit f4e7bf7 into inveniosoftware:master Aug 2, 2023
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

moderation: added request type and service #335

moderation: added request type and service #335

alejandromumo commented Jul 19, 2023 •

edited

Loading

ntarocco Jul 20, 2023

alejandromumo Jul 25, 2023 •

edited

Loading

ntarocco Jul 20, 2023

alejandromumo Jul 20, 2023

ntarocco Jul 20, 2023

ntarocco Jul 20, 2023

alejandromumo Jul 25, 2023

alejandromumo commented Jul 25, 2023

lnielsen Jul 25, 2023

alejandromumo Aug 1, 2023

lnielsen Aug 1, 2023

alejandromumo Aug 1, 2023 •

edited

Loading

lnielsen Aug 1, 2023

alejandromumo Aug 1, 2023 •

edited

Loading

zzacharo Aug 2, 2023

		from invenio_requests.services.user_moderation.user_moderation import UserModeration


		class UserModerationRequestService:

		from invenio_requests.services.user_moderation.errors import InvalidCreator


		def test_request_moderation(app, users, identity_simple):

	"create": actions.CreateAction,
	**RequestType.available_actions,
	...

moderation: added request type and service #335

moderation: added request type and service #335

Conversation

alejandromumo commented Jul 19, 2023 • edited Loading

Choose a reason for hiding this comment

alejandromumo Jul 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alejandromumo commented Jul 25, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alejandromumo Aug 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alejandromumo Aug 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alejandromumo commented Jul 19, 2023 •

edited

Loading

alejandromumo Jul 25, 2023 •

edited

Loading

alejandromumo Aug 1, 2023 •

edited

Loading

alejandromumo Aug 1, 2023 •

edited

Loading