[69448] Send assignee notification email only once after PIR completion
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?inapp/services/journals/create_service.rbcontrolling this behavior was returning false and the timestamp was regenerated.
- it worked before because as it was saving twice, the
- 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
UpdateAncestorsServicewhich does save and also due to changes introduced in https://github.com/opf/openproject/commit/07002998bf77d3be69bec06721939cb344ea5257 which refactored the part that detectsavereturnedfalse, leading to believe thatsavealways returnedtrue. - it was fixed by ignoring all contract validations on project copy
- this broke tests as the substitution of unassignable members with
nilwas not working for a long time, but this was not detected. This was fixed in wp https://community.openproject.org/wp/70290.
- this broke tests as the substitution of unassignable members with
- it worked before because of side-effects of
Merge checklist
- [x] Added/updated tests
- [ ] Added/updated documentation in Lookbook (patterns, previews, etc)
- [ ] Tested major browsers (Chrome, Firefox, Edge, ...)
This introduces regressions that are being worked on in https://github.com/opf/openproject/pull/21332