Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

user_moderation: user search for fetching user communities #1204

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 29 additions & 45 deletions invenio_communities/requests/user_moderation/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,60 +7,39 @@
# it under the terms of the MIT License; see LICENSE file for more details.
"""Communities user moderation actions."""

from collections import defaultdict

from invenio_access.permissions import Identity, system_identity
from invenio_db import db
from invenio_i18n import lazy_gettext as _
from invenio_pidstore.errors import PIDDoesNotExistError
from invenio_records_resources.services.uow import TaskOp
from invenio_search.api import RecordsSearchV2
from invenio_search.engine import dsl
from invenio_vocabularies.proxies import current_service as vocab_service

from invenio_communities.communities.records.systemfields.deletion_status import (
CommunityDeletionStatusEnum,
)
from invenio_communities.proxies import current_communities

from .tasks import delete_community, restore_community


def _get_communities_for_user(user_id):
"""Helper function for getting all communities with the user as the sole owner.

Note: This function performs DB queries yielding all communities with the given
user as the sole owner (which is not hard-limited in the system) and performs
service calls on each of them. Thus, this function has the potential of being a very
heavy operation, and should not be called as part of the handling of an
HTTP request!
"""
comm_cls = current_communities.service.record_cls
comm_model_cls = comm_cls.model_cls
def get_user_communities(user_id, from_db=False):
"""Helper function for getting all communities with the user as the sole owner."""
mem_cls = current_communities.service.members.record_cls
mem_model_cls = mem_cls.model_cls

# collect the owners for each community
comm_owners = defaultdict(list)
for comm_owner in [
mem_cls(m.data, model=m)
for m in mem_model_cls.query.filter(mem_model_cls.role == "owner").all()
]:
comm_owners[comm_owner.community_id].append(comm_owner)

# filter for communities that are owned solely by the user in question
relevant_comm_ids = [
comm_id
for comm_id, owners in comm_owners.items()
if len(owners) == 1 and str(owners[0].user_id) == user_id
]

# resolve the communities in question
communities = [
comm_cls(m.data, model=m)
for m in comm_model_cls.query.filter(
comm_model_cls.id.in_(relevant_comm_ids)
).all()
]

return communities
if from_db:
query = db.session.query(mem_model_cls.community_id).filter(
mem_model_cls.user_id == user_id,
mem_model_cls.role == "owner",
)
return (row[0] for row in query.yield_per(1000))
else:
search = (
RecordsSearchV2(index=mem_cls.index._name)
.filter("term", user_id=user_id)
.filter("term", role="owner")
.source(["community_id"])
)
return (hit["community_id"] for hit in search.scan())
Comment on lines +29 to +42
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Major: Hmm I think we are missing the filtering for the communities that are owned solely by the user, unless I am missing something?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, this doesn't check anymore if the user is the only owner. Part of the change was to improve on the overall performance (before there was some in-memory overfetching). I think adding the check back is not that big of a performance hit, so I'll have a go at it.



def on_block(user_id, uow=None, **kwargs):
Expand All @@ -83,9 +62,14 @@ def on_block(user_id, uow=None, **kwargs):
except PIDDoesNotExistError:
pass

# soft-delete all the communities of that user (only if they are the only owner)
for comm in _get_communities_for_user(user_id):
delete_community.delay(comm.pid.pid_value, tombstone_data)
for comm in get_user_communities(user_id):
uow.register(
TaskOp(
delete_community,
pid=comm.pid.pid_value,
tombstone_data=tombstone_data,
)
)


def on_restore(user_id, uow=None, **kwargs):
Expand All @@ -98,8 +82,8 @@ def on_restore(user_id, uow=None, **kwargs):
user_id = str(user_id)

# restore all the deleted records of that user
for comm in _get_communities_for_user(user_id):
restore_community.delay(comm.pid.pid_value)
for comm in get_user_communities(user_id):
uow.register(TaskOp(restore_community, pid=comm.pid.pid_value))


def on_approve(user_id, uow=None, **kwargs):
Expand Down
Loading