#1199 closed defect (fixed)
metadata/DefaultStatusActions.java:processList() doesnt send the e-mail to all users
Reported by: | landry | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | v2.8.0 |
Component: | Catalog server | Version: | v2.8.0RC2 |
Keywords: | Cc: | simon.pigot@… |
Description
When changing a metadata status from unknown to submitted, some mails should be sent by the catalog to all reviewers. What i'm seeing instead is :
- the first reviewer in the list doesnt receive an e-mail
- all the others receive a mail
- the last reviewer receives a dupe of the mail
with the given sql request made by am.getContentReviewers() in informContentReviewers()
SELECT m.id as metadataid, u.id as userid, u.name as name, u.surname as surname, u.email as email from Metadata m JOIN UserGroups ug on m.groupOwner = ug.groupId JOIN Users u on u.id = ug.userId WHERE m.id=282 and u.profile='Reviewer' order by u.id;
say i get a list with a@…, b@… and c@… as email field.
a@… will receive nothing, b@… will receive a mail, and c@… will receive two mails.
I've tracked it down to processList() in metadata/DefaultStatusActions.java - which logic seems bogus. When that chunk :
if (mIds.size() > 0) { // send out the last one sendEmail(email, subject, status, changeDate, changeMessage); }
is reached, 'email' still contains the e-mail of the last reviewer in the userList.
What was the intent of the strange
if (!id.equals(lastUserId) && !first) { // send out list }
test to avoid sending a mail to the first e-mail ? avoid duplicates ? Also, why storing mIds/mid/id/lastUserId ?
To me, the thing could be greatly simplified as this :
for (Element user : userList) { email = user.getChildText("email"); sendEmail(email, subject, status, changeDate, changeMessage); }
Unless there's something obvious i'm missing, i dont understand why the function is so complicated..
Change History (2)
comment:1 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:2 by , 12 years ago
Milestone: | v2.9.0 → v2.8.0 |
---|
Logic got twisted after changes to an earlier incantation that sent metadata ids in status change messages - fix is not quite as simple as you've proposed because the accessmanager calls owners/content reviewers for all metadata records so need to remove duplicates - but certainly can be done much more easily than the original :-)
Fixed in commit 3d16a17599453a62398a11c6e0be72b9323716b6