[WIP] Moderation emails debouncing #9553
Conversation
| @@ -10,3 +10,8 @@ def __init__(self, request, annotation_id, action: AnnotationAction): | |||
| self.request = request | |||
| self.annotation_id = annotation_id | |||
| self.action = action | |||
|
|
|||
|
|
|||
| class ModerationEvent: | |||
|
|
||
|
|
||
| def upgrade() -> None: | ||
| op.add_column( |
There was a problem hiding this comment.
New column on ModerationLog pointing to the relevant notification.
|
|
||
|
|
||
| def upgrade() -> None: | ||
| op.drop_constraint( |
There was a problem hiding this comment.
Check the existing no more than one notification per annotation to "no more than one notification per annotations, for replies and mentions".
| @@ -3,8 +3,9 @@ | |||
|
|
|||
|
|
|||
| class AnnotationModerationService: | |||
| def __init__(self, session): | |||
| def __init__(self, session): # , annotation_write: AnnotationWriteService): | |||
There was a problem hiding this comment.
There's a half finished refactor going on. I reckon we need AnnotationSlim service to untagle thing here.
| @@ -165,20 +165,6 @@ def update_annotation( | |||
|
|
|||
| return annotation | |||
|
|
|||
| def hide(self, annotation: Annotation, user: User): | |||
There was a problem hiding this comment.
No need for this wrapper around moderation. We have view -> annotation write -> moderation service.
Changing things to go from view to moderation services.
| @@ -17,10 +19,12 @@ | |||
| permission=Permission.Annotation.MODERATE, | |||
| ) | |||
| def hide(context, request): | |||
There was a problem hiding this comment.
Use the moderation services directly here. We could even refactor the client to use the new set_status endpoint for now just make these as similar as possible.
| select(ModerationLog) | ||
| .where( | ||
| ModerationLog.annotation_id == annotation_id, | ||
| ModerationLog.created >= moderation_datetime, |
There was a problem hiding this comment.
This might better using the "moderation_log.id" instead of passing the date.
| .with_for_update(skip_locked=True) | ||
| ).all() | ||
|
|
||
| if not moderation_log: |
There was a problem hiding this comment.
This case is when no moderation status changes have been found. Not sure in which scenario.
| ) | ||
| return | ||
|
|
||
| if moderation_log[-1].created > moderation_datetime: |
There was a problem hiding this comment.
Another status change has happened while we were waiting the three minutes.
| print(moderation_log[-1].id) | ||
| print(old_status, new_status) | ||
|
|
||
| if old_status == new_status: |
There was a problem hiding this comment.
No change between the first and last moderation logs.
There was a problem hiding this comment.
While we don't want to send a notification for these we do want to exclude them from processing next time:
Let's say in quick succession:
APPROVED -> DENIED -> APPROVED
Then 10 minutes later
DENIED
Although the result might be the same we only want take into account the last "APPROVED" in the logs, not start from the first one for the initial state.
I'm not 100% sure of this
| ) | ||
| return | ||
|
|
||
| email_sending_status_changes = { |
There was a problem hiding this comment.
Only moderation changes that do send an email
| (ModerationStatus.SPAM, ModerationStatus.APPROVED), | ||
| } | ||
|
|
||
| if (old_status, new_status) not in email_sending_status_changes: |
There was a problem hiding this comment.
We have the same issue described here: #9553 (comment)
No description provided.