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

groups: updates mapping to contain description #75

Merged
merged 3 commits into from
Jun 15, 2023
Merged
Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion invenio_users_resources/ext.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,4 @@ def _after_rollback(session):
# rollbacked. In a case (e.g. nested transactions) when an
# exception is caught and then the session is commited,
# the sets might not reflect all the users that were changed.
current_db_change_history._clear_dirty_sets(session)
current_db_change_history.clear_dirty_sets(session)
10 changes: 7 additions & 3 deletions invenio_users_resources/records/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,10 @@ def parse_user_data(user):
def parse_role_data(role):
"""Parse the role's information into a dictionary."""
data = {
"id": role.name, # due to flask security exposing user id
"name": role.description,
"is_managed": True, # TODO
"id": role.id,
"name": role.name,
"description": role.description,
"is_managed": role.is_managed,
}
return data

Expand Down Expand Up @@ -189,6 +190,9 @@ class GroupAggregate(Record):
name = DictField("name")
"""The group's name."""

description = DictField("description")
"""The group's description."""

is_managed = DictField("is_managed")
"""If the group is managed manually."""

Expand Down
44 changes: 21 additions & 23 deletions invenio_users_resources/records/hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ def pre_commit(sender, session):
# it seems that the {dirty,new,deleted} sets aren't populated
# in after_commit anymore, that's why we need to collect the
# information here.
# session is a scoped_session and does not have _model_changes
# so we have rely only on .dirty
# session is a scoped_session and does not have _model_changes,
# so we have to rely only on .dirty
updated = session.dirty.union(session.new)
deleted = session.deleted
sid = id(session)
Expand All @@ -31,19 +31,17 @@ def pre_commit(sender, session):

# users need to be reindexed if their user model was updated, or
# their profile was changed (or even possibly deleted)
current_db_change_history.updated_users[sid].extend(
[u.id for u in updated if isinstance(u, User)]
)
current_db_change_history.updated_roles[sid].extend(
[r.id for r in updated if isinstance(r, Role)]
)
for item in updated:
if isinstance(item, User):
current_db_change_history.add_updated_user(sid, item.id)
if isinstance(item, Role):
current_db_change_history.add_updated_role(sid, item.id)

current_db_change_history.deleted_users[sid].extend(
[u.id for u in deleted if isinstance(u, User)]
)
current_db_change_history.deleted_roles[sid].extend(
[r.id for r in deleted if isinstance(r, Role)]
)
for item in deleted:
if isinstance(item, User):
current_db_change_history.add_deleted_user(sid, item.id)
if isinstance(item, Role):
current_db_change_history.add_deleted_role(sid, item.id)


def post_commit(sender, session):
Expand All @@ -52,15 +50,15 @@ def post_commit(sender, session):
# DB operations are allowed here, not even lazy-loading of
# properties!
sid = id(session)
for user_id in current_db_change_history.updated_users[sid]:
reindex_user.delay(user_id)
if current_db_change_history.sessions.get(sid):
ntarocco marked this conversation as resolved.
Show resolved Hide resolved
for user_id in current_db_change_history.sessions[sid].updated_users:
reindex_user.delay(user_id)

for role_id in current_db_change_history.updated_roles[sid]:
reindex_group.delay(role_id)
for role_id in current_db_change_history.sessions[sid].updated_roles:
reindex_group.delay(role_id)

for user_id in current_db_change_history.deleted_users[sid]:
unindex_user.delay(user_id)
for role_id in current_db_change_history.deleted_roles[sid]:
unindex_group.delay(role_id)
for user_id in current_db_change_history.sessions[sid].deleted_users:
unindex_user.delay(user_id)

current_db_change_history._clear_dirty_sets(session)
for role_id in current_db_change_history.sessions[sid].deleted_roles:
unindex_group.delay(role_id)
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@
}
}
},
"description": {
Copy link
Member

Choose a reason for hiding this comment

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

topic to raise: same old question, this forces people to delete indexes and reindex. Creating a v2 would allow just querying the alias and getting them all (without having to delete/reindex, i.e. avoiding users functionality downtime). This is also possible since description is not a mandatory field.

"type": "text"
},
"provider": {
"type": "text"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@
}
}
},
"description": {
"type": "text"
},
"provider": {
"type": "text"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@
}
}
},
"description": {
jrcastro2 marked this conversation as resolved.
Show resolved Hide resolved
"type": "text"
},
"provider": {
"type": "text"
},
Expand Down
19 changes: 18 additions & 1 deletion run-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,23 @@ set -o errexit
# Quit on unbound symbols
set -o nounset

keep_services=0
pytest_args=()
for arg in $@; do
# from the CLI args, filter out some known values and forward the rest to "pytest"
# note: we don't use "getopts" here b/c of some limitations (e.g. long options),
# which means that we can't combine short options (e.g. "./run-tests -Kk pattern")
case ${arg} in
-K|--keep-services)
keep_services=1
;;
*)
pytest_args+=( ${arg} )
;;
esac
done


# Always bring down docker services
function cleanup() {
eval "$(docker-services-cli down --env)"
Expand All @@ -29,6 +46,6 @@ python -m check_manifest
python -m setup extract_messages --output-file /dev/null
python -m sphinx.cmd.build -qnNW docs docs/_build/html
eval "$(docker-services-cli up --db ${DB:-postgresql} --search ${SEARCH:-opensearch} --cache ${CACHE:-redis} --mq ${MQ:-rabbitmq} --env)"
python -m pytest
python -m pytest ${pytest_args[@]+"${pytest_args[@]}"}
tests_exit_code=$?
exit "$tests_exit_code"
6 changes: 3 additions & 3 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ packages = find:
python_requires = >=3.7
zip_safe = False
install_requires =
invenio-accounts>=2.2.0,<3.0.0
invenio-accounts>=3.0.0,<4.0.0
invenio-i18n>=2.0.0
invenio-notifications>=0.1.0,<1.0.0
invenio-oauthclient>=2.2.0,<3.0.0
invenio-records-resources>=4.0.0,<5.0.0
invenio-oauthclient>=3.0.0,<4.0.0
invenio-records-resources>=4.3.0,<5.0.0

[options.extras_require]
tests =
Expand Down
34 changes: 27 additions & 7 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,11 @@ def users(UserFixture, app, database, users_data):
return users


def _create_group(name, description, database):
def _create_group(id, name, description, is_managed, database):
"""Creates a Role/Group."""
r = current_datastore.create_role(name=name, description=description)
r = current_datastore.create_role(
id=id, name=name, description=description, is_managed=is_managed
)
current_datastore.commit()

return r
Expand All @@ -224,20 +226,38 @@ def _create_group(name, description, database):
@pytest.fixture(scope="module")
def group(database):
"""A single group."""
r = _create_group(name="it-dep", description="IT Department", database=database)
r = _create_group(
id="it-dep",
name="it-dep",
description="IT Department",
is_managed=True,
database=database,
)

GroupAggregate.index.refresh()
return r


@pytest.fixture(scope="module")
def groups(database, group):
def group2(database):
"""A single group."""
roles = [group] # it-dep
roles.append(
_create_group(name="hr-dep", description="HR Department", database=database)
r = _create_group(
id="hr-dep",
name="hr-dep",
description="HR Department",
is_managed=True,
database=database,
)

GroupAggregate.index.refresh()
return r


@pytest.fixture(scope="module")
def groups(database, group, group2):
"""A single group."""
roles = [group, group2]

GroupAggregate.index.refresh()
return roles

Expand Down