Add User Status Log #372
Add User Status Log #372
Conversation
AkshayPall
force-pushed
the
audit_log
branch
from
56e4deb to
fe5896b
Compare
akprasad
left a comment
akprasad
left a comment
There was a problem hiding this comment.
I see what this is going for, but I think there's a better design here. I'd like to see this model tied to a specific use case -- maybe a banning feature, say -- so that we can get a better feel for how and where this model would be used. For example, perhaps one simple schema could be:
class ModeratorLog:
id
moderator_id
user_id(nullable=true)
description
timestamp
which would be generic to all moderator actions, including automatic actions like verifying a user.
Also, let's clean up the role setters (set_is_banned) as discussed earlier
| change_description = Column(String, nullable=False) | ||
|
|
||
| #: When should this status change expire/revert, defaults to never. | ||
| expiry = Column(DateTime, default=None, nullable=True) |
There was a problem hiding this comment.
Since the semantics here are all in change_description which is unstructured text, there's nothing actionable we can do even if we know expiry. This seems like something we could fold into change_description itself
| timestamp = Column(DateTime, default=datetime.utcnow, nullable=False) | ||
|
|
||
| #: Describes the status change that occurred. | ||
| change_description = Column(String, nullable=False) |
There was a problem hiding this comment.
description since the change_ is implicit by dint of this being a log
| #: When should this status change expire/revert, defaults to never. | ||
| expiry = Column(DateTime, default=None, nullable=True) | ||
|
|
||
| @property |
There was a problem hiding this comment.
Let's wait on these until we know how to use them
| return f"<Role({self.id}, {self.name!r})>" | ||
|
|
||
|
|
||
| class UserStatusLog(AmbudaUserMixin, Base): |
There was a problem hiding this comment.
remove UserMixin here as this is not a user
| #: Primary key. | ||
| id = pk() | ||
|
|
||
| #: The user whose status was changed. |
There was a problem hiding this comment.
state changes should also indicate the user who made the change (or perhaps ambuda-bot if there isn't one)
2 participants
Diff 2/3 for #224