Activity log filter by user #2924
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gagandeepb
added
enhancement
nelsonkopliku
requested changes
Sep 9, 2024
There was a problem hiding this comment.
Hey @gagandeepb nice job!
Praise for the usage of channels/websockets instead of a new rest endpoint! I believe it puts us in a good spot.
Now, besides what already commented, I noticed a behaviour with deleted users that we need to take into account
- a deleted user's username is stored as
<initialUsername>__<dateOfDeletion>. ie thefoobaruser, when deleted, becomesfoobar__2024-09-09 08:37:57.605629Z - usernames can be reused. ie there can be a new user
foobarafter the initialfoobaruser was deleted
That said, we need to decide what to do.
Options I see are:
- we detect deleted user's usernames and extract only the relevant part (the one before the double underscore
__), and remove duplicates in case there is another user with the same username. This leads to possibly returning, when searching the activity log by such user, entries for two different users: the one that was deleted plus the one currently in the system - we keep track and differentiate between deleted/current users with same usernames. This would require enriching the activity log entries with the identifier of the user (or a join table)
Let's check with @abravosuse as well.
- Deletes unused code - Pulls out state access into the ActivityLog page context
- Deletes unnecessary code from the composed filter - Adds a new function the users context - Adds needed context calls to the ActivityLog channel module
Fix for identityFunctionCheck warning in console.
- channel test for activity log - activity log state test
Adds a hook that passes in requisite state to the ActivityLogPage component
Specific users in list are not necessary
Previously a system user was not returned; now it is possible to filter the system activity log entries.
Renaming
Collapses usernames from deleted users into a username without timestamp suffix
According to PR conversation, the update push interval from the channel is now a compile time Application env variable.
The users context function modified here now returns username tupled with the deleted_at timestamp. This simplifies somewhat the implementation of the collapse_usernames function in the ActivityLogChannel module. Specifically, this function no longer needs to do any DateTime parsing to infer soft deleted users/associated usernames.
This test makes use of non empty/non default state to assert on number of user filter options rendered.
Some changes introduced post rebase were breaking a test. This commit fixes the failing test.
gagandeepb
force-pushed
the
activity-log-filter-by-user
branch
from
ccd8ac3 to
bd7ca7c
Compare
- import re-ordering/spacing, etc.
Removes the trailing _ts
In order to minimize diffs/conflicts
nelsonkopliku
approved these changes
Sep 13, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Activity log filter by user.
Did you add the right label?
Remember to add the right labels to this PR.
How was this tested?
Describe the tests that have been added/changed for this new behavior.
Unit tests
Did you update the documentation?
Remember to ask yourself if your PR requires changes to the following documentation:
Add a documentation PR or write that no changes are required for the documentation.
No doc changes required.