Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#1199 closed defect (fixed)

metadata/DefaultStatusActions.java:processList() doesnt send the e-mail to all users

Reported by: landry Owned by: geonetwork-devel@…
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 simonp, 11 years ago

Resolution: fixed
Status: newclosed

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

comment:2 by ianwallen, 11 years ago

Milestone: v2.9.0v2.8.0
Note: See TracTickets for help on using tickets.