Skip to content

Add project tracking via group_events attribute - #581

Merged
Mark-Powers merged 3 commits into
masterfrom
track_membership
Mar 9, 2026
Merged

Add project tracking via group_events attribute#581
Mark-Powers merged 3 commits into
masterfrom
track_membership

Conversation

@Mark-Powers

Copy link
Copy Markdown
Contributor

Tracks project changes on each user via a new keycloak attribute. Also includes a management command to initialize this attribute with current project membership, indicating that the events were backdated.

Tracks project changes on each user via a new keycloak attribute. Also includes
a management command to initialize this attribute with current project membership,
indicating that the events were backdated.
@Mark-Powers
Mark-Powers requested review from msherman64 and pdmars March 3, 2026 19:29

@pdmars pdmars left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall looks good to me. Do we have admin/operator docs on when, and how, to use this command?

@Mark-Powers

Copy link
Copy Markdown
Contributor Author

Overall looks good to me. Do we have admin/operator docs on when, and how, to use this command?

I can add some docs in the file. This will just be run once it is deployed for the initial data. It shouldn't be run again. I included it here so that we can reference the implementation if we need to in the future.

@msherman64 msherman64 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The data we store seems reasonable

My main question is about the details of storing this in a keycloak attribute.

it seems like this is getting appended to a single field?
How long a value can keycloak store?
if we want to query this later, how is this done?

I guess the alternative would be storing this in a portal db table, which also means we need to keep it in sync with keycloak.

Comment thread util/keycloak_client.py Outdated
citizenship=None,
phone=None,
lifecycle_allocation_joined: "datetime" = None,
group_events: dict = None,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is group_events a dict or a list?



class Command(BaseCommand):
help = "Normalize user-supplied institution strings into Institution/UserInstitution models"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

wrong help text?

Comment thread projects/membership.py
Args:
kc_user (dict): The Keycloak user dict.
charge_code (str): The charge code of the project.
event_type (str): The type of event ("add" or "remove"). For backdated events, there is also "add_backdate".

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it could be worth considering storing two timestamps:

audit_time, and event_time, this would distinguish backdated events naturally

@Mark-Powers

Copy link
Copy Markdown
Contributor Author

it seems like this is getting appended to a single field? How long a value can keycloak store? if we want to query this later, how is this done?

The keycloak user API returns this info. We use it already to store if the user has agreed to terms and what institution they are from. It persists forever.

The field is a list, but it is serialized as a string if there is only one value. This is how keycloak handles attributes.

I guess the alternative would be storing this in a portal db table, which also means we need to keep it in sync with keycloak.

Yes, it would be easier to query in portal directly, but it seems more consistent to keep this in portal. We probably should sync this to a reporting DB like we do for events. Right now it is easy to verify this field is accurate, as we can look at any individual user and compare the attributes to their project membership.

I do like the idea of audit time vs event time, but I am trying to minimize the complexity to process the data.

@Mark-Powers
Mark-Powers merged commit d523437 into master Mar 9, 2026
10 of 11 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.

3 participants