fix(load): skip committee-approval emails and set approval_label - #618
zubeydecivelek wants to merge 1 commit into
Conversation
| @staticmethod | ||
| def _drop_notification_ops(uow, from_index): | ||
| """Remove notification ops registered at/after ``from_index``. | ||
|
|
||
| ``CommitteeApprovalRequest``'s create action is a create-and-submit that | ||
| emails the referee group. Migrated (already-approved) requests must not | ||
| notify anyone. | ||
| """ | ||
| uow._operations[from_index:] = [ | ||
| op | ||
| for op in uow._operations[from_index:] | ||
| if not isinstance(op, NotificationOp) | ||
| ] | ||
|
|
There was a problem hiding this comment.
instead of this we can change in cds-rdm. We can skip the submit notification when identity is system. Real UI submissions (community managers) still send email.
@zzacharo
There was a problem hiding this comment.
can you disable notifications in general from the migrator kit pod instead of adding the code?
There was a problem hiding this comment.
I would say that, unless we have some notification that make sense during migration, but I dont think so :)
There was a problem hiding this comment.
we don't have any, I specifically disabled the notifications on the migration pod because we don't want to send any emails
There was a problem hiding this comment.
I checked how it's done for Community submission. It uses a plain CreateAction on create, so migration can create the request and then set submitted/accepted without running submit/accept (where emails are sent). Committee approval is implemented differently so same service.create() validates, grants referees, and queues the notify email. So migration needs an extra step...
It can separated in cds-rdm also. Or do you prefer having a config variable for migrator to disable it?
There was a problem hiding this comment.
Cant' we just hook on this and write configure the email backend to not send email notifications from the migration pod?
There was a problem hiding this comment.
I might be wrong but as I understand NOTIFICATIONS_BACKENDS is applied where the Celery task runs, not where it is queued and we use the same mq so it doesnt matter it comes from migration pod
zzacharo
left a comment
zzacharo
left a comment
There was a problem hiding this comment.
the fix for the approval label seems fine!
6a7eb36 to
e140544
Compare
3 participants
closes CERNDocumentServer/cds-rdm#998