-
Notifications
You must be signed in to change notification settings - Fork 138
Add user activity logs to messaging endpoint #2691
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
base: main
Are you sure you want to change the base?
Conversation
034742a to
5f62acb
Compare
5f62acb to
6ecbd2e
Compare
| cache.set(f"{USER_PROFILE_PREFIX}{user_name}", profile) | ||
|
|
||
| # send notification on creating new user account | ||
| user = User.objects.get(username__iexact=user_name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we fetch user via the id instead?
user = User.objects.get(id=profile.get("id")
| send_message( | ||
| instance_id=user_name, | ||
| target_id=user.id, | ||
| target_type=USER, | ||
| user=user, | ||
| message_verb=USER_CREATED, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we move this directly into the UserProfileSerializer class instead? I think the above additional query for the user will be unnecessary in such a case.
| cache.set(f"{USER_PROFILE_PREFIX}{username}", response.data) | ||
|
|
||
| # send notification on updating user profile | ||
| send_message( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can also move this to the serializer. Since we are sending the messaging for both create and update, we can override the serializer's save method and call send_message there. Example
We can differentiate create from update by checking self.instance. When doing an update self.instance is not None
| profile.save() | ||
|
|
||
| # send notification on updating user profile | ||
| send_message( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
partial_update will call the serializer's save method. If we refactor to send the message for update within the serializer, then we can get rid of sending the message here
Changes / Features implemented
Adds message notifications for the following actions:
Steps taken to verify this change does what is intended
Before submitting this PR for review, please make sure you have:
Closes #