Skip to content
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

fix(notifications): actor replacement #15623

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Rom1-B
Copy link
Contributor

@Rom1-B Rom1-B commented Sep 22, 2023

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.

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets !29660 #14704

Copy link
Member

@cedric-anne cedric-anne left a 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).

cedric-anne
cedric-anne previously approved these changes Oct 4, 2023
@cedric-anne cedric-anne requested review from trasher and orthagh and removed request for trasher and orthagh October 20, 2023 09:10
@orthagh
Copy link
Contributor

orthagh commented Oct 24, 2023

After irl discussion:

  • shoult target main branch
  • I guess 6 month after release, someone will want the inverse behavior, so:
    provide a new notification for deletion of actors (and copy the same recipients as the former)

@cedric-anne cedric-anne added this to the 10.1.0 milestone Oct 24, 2023
@cedric-anne cedric-anne changed the base branch from 10.0/bugfixes to main October 24, 2023 13:07
@cedric-anne cedric-anne linked an issue Oct 24, 2023 that may be closed by this pull request
2 tasks
@cedric-anne cedric-anne marked this pull request as draft October 30, 2023 10:30
@cedric-anne cedric-anne dismissed their stale review October 30, 2023 10:31

another solution was requested

@Rom1-B
Copy link
Contributor Author

Rom1-B commented Nov 9, 2023

If this PR targets main, i.e. 10.1.x, it doesn't correct the case on the current version.

In this case, how to fix it on 10.0/bugfixes?

I have 2 customers who have reported this problem on 10.0 : !29660 + !30331

@Rom1-B
Copy link
Contributor Author

Rom1-B commented Dec 29, 2023

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 bugfixes.

I would point out that with the 2nd commit, the status doesn't change, it's not temporarily in "in coming".

@Rom1-B Rom1-B marked this pull request as ready for review June 7, 2024 08:47
@cedric-anne cedric-anne self-requested a review November 21, 2024 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10.0.6 Bug notification with new group assigned
3 participants