-
Notifications
You must be signed in to change notification settings - Fork 71
[RORDEV-1574] New logged_user audit field #285
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
[RORDEV-1574] New logged_user audit field #285
Conversation
WalkthroughDocumentation updates in Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
details/audit.md(2 hunks)
🔇 Additional comments (1)
details/audit.md (1)
33-34: Verify consistency with predefined serializers documentation.The example audit event now includes the new
logged_userandpresented_identityfields. However, line 412 in the predefined serializers section still documents the olduserfield. If this field is being removed or replaced, the predefined serializers documentation should be updated accordingly to avoid confusion.Please verify one of the following:
- Should line 412 be updated to reference
logged_userinstead ofuser?- Should the predefined serializers section document both the old
userfield and the newlogged_userandpresented_identityfields for backward compatibility?- Is this an intentional change where the two new fields coexist with the old
userfield?
coutoPL
left a comment
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.
LGTM
Summary by CodeRabbit