openproject icon indicating copy to clipboard operation
openproject copied to clipboard

[69448] Send assignee notification email only once after PIR completion

Open cbliard opened this issue 3 months ago • 2 comments

Ticket

https://community.openproject.org/wp/69448

What are you trying to accomplish?

Sometimes, two emails are sent out to the person being mentioned in the comment of the work package created after project initiation request completion. There can be only one.

Screenshots

What approach did you choose and why?

In WorkPackages::CreateService, the work package was actually saved twice: first time at creation, second time when rescheduling related work packages, which includes self.

Each save created a WorkflowJob. Normally this is ok, but if there are mentions in the description or journal_notes, and if the two jobs are processed in parallel (in different GoodJob threads), this leads to having the mention notification email sent twice. Indeed, each one will try to create the notifications and send them. The mail_alert_sent flag on the notification is not set to true until one mail has been sent.

To fix it, change WorkPackages::CreateService: the work packages that have not been changed after rescheduling related work packages are filtered out. This guarantees there is only one single save in the nominal cases where there are no related work packages that would force a second save of the work package. Single save -> single mention notification email.

This innocent change has pulled some issues in other areas:

  • two journals created one after the other without touching the updated_at of the journable fails
    • it worked before because as it was saving twice, the updated_at_previously_changed? in app/services/journals/create_service.rb controlling this behavior was returning false and the timestamp was regenerated.
  • the project copy was not working as before when some work packages had some validation errors: the work package would not be copied again
    • it worked before because of side-effects of UpdateAncestorsService which does save and also due to changes introduced in https://github.com/opf/openproject/commit/07002998bf77d3be69bec06721939cb344ea5257 which refactored the part that detect save returned false, leading to believe that save always returned true.
    • it was fixed by ignoring all contract validations on project copy
      • this broke tests as the substitution of unassignable members with nil was not working for a long time, but this was not detected. This was fixed in wp https://community.openproject.org/wp/70290.

Merge checklist

  • [x] Added/updated tests
  • [ ] Added/updated documentation in Lookbook (patterns, previews, etc)
  • [ ] Tested major browsers (Chrome, Firefox, Edge, ...)

cbliard avatar Dec 03 '25 13:12 cbliard

This introduces regressions that are being worked on in https://github.com/opf/openproject/pull/21332

cbliard avatar Dec 04 '25 08:12 cbliard