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

Models: Domain statistics and model change history tracking fixes #475

Merged
merged 3 commits into from
Feb 19, 2024

Conversation

lnielsen
Copy link
Member

@lnielsen lnielsen commented Feb 12, 2024

  • Fixes an issue where a newly uncommitted user would not yet have an id assigned, and hence would cause None value to be added to a list of user ids that needed to be reindexed, causing the indexing to fail due to transaction rollback.
  • Add change tracking of domains to reindex on updates.
  • Add task to compute domain statistics on a regular basis.
  • 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:

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

* Fixes an issue where a newly uncommitted user would not yet have an
  id assigned, and hence would cause None value to be added to a list
  of user ids that needed to be reindexed, causing the indexing to
  fail due to transaction rollback.
* Adds a task to efficiently updates the domain statistics on a
  regular basis.
@@ -19,14 +23,46 @@
class SessionAwareSQLAlchemyUserDatastore(SQLAlchemyUserDatastore):
"""Datastore which deletes active session when a user is deactivated."""

def verify_user(self, user):
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved here from Invenio-users-resources to treat these action consistently.

user.blocked_at = None
if user.confirmed_at is None:
user.confirmed_at = datetime.utcnow()
user_confirmed.send(current_app._get_current_object(), user=user)
Copy link
Member Author

Choose a reason for hiding this comment

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

Needs to send signal to run the code which takes care of registering the domain.

@@ -37,29 +73,39 @@ def commit(self):
datastore_post_commit.send(session=self.db.session)
current_db_change_history.clear_dirty_sets(self.db.session)

def put(self, model):
Copy link
Member Author

Choose a reason for hiding this comment

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

Put was being used for both roles and users, but the mark_changed() call only handled users, and in addition the model object only have an id if it was committed which is not always the case.

invenio_accounts/tasks.py Outdated Show resolved Hide resolved
@lnielsen lnielsen merged commit df1237f into inveniosoftware:master Feb 19, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

User and domain administration interface improvements.
3 participants