-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix(notifications): actor replacement #15623
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
base: main
Are you sure you want to change the base?
Conversation
cedric-anne
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.
Same proposal was made in #14796, but it has side-effects on status computation (see comments).
|
After irl discussion:
|
|
If this PR targets In this case, how to fix it on I have 2 customers who have reported this problem on 10.0 : !29660 + !30331 |
|
I still think that notifying the deleted actor as if he were still an active actor in the ticket is a bug and should be treated as I would point out that with the 2nd commit, the status doesn't change, it's not temporarily in "in coming". |
3d1b4fc to
a724df0
Compare
034f132 to
967dafc
Compare
| 'pendingreason_add' => __('Pending reason added'), | ||
| 'pendingreason_del' => __('Pending reason removed'), | ||
| 'pendingreason_close' => __('Pending reason auto close'), | ||
| 'del_assign_user' => __('Deletion of a user in assignees'), |
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.
IMHO, we should add a remove_xxx event for all possible actor type (e.g. requester user, requester group, observer user, observer group, assigned user, assigned group and assigned suplier).
If we add this only for assigned users, I am pretty sure that someone will ask for the same feature for another actor type as soon as the feature will be released.
| && ($item->countGroups(self::ASSIGN) === 0) | ||
| && ((int) $item->fields['status'] !== CommonITILObject::CLOSED) | ||
| && ((int) $item->fields['status'] !== CommonITILObject::SOLVED) | ||
| && !isset($this->input['_do_not_compute_status']) |
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.
I am not sure this is related to this PR.
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.
This prevents the ticket status from changing temporarily to new during the short period when there are no technicians, between deletion and addition.
On a ticket, when an assigned group was replaced by another, the notification was sent to the deleted group, and in the message content the old group was displayed as still assigned.
EDIT : same for users
This was because the notification was sent after the new group had been added to the database, but before the old group had been deleted.