Code review comment for lp:~sinzui/launchpad/person-merge-job-1

Hi Robert.

The believe oddness of the getErrorRecipients() interface is caused by PersonTransferJobDerived which breaks the permission chain.

{{{
class PersonTransferJobDerived(BaseRunnableJob):
    delegates(IPersonTransferJob)
}}}

Thank you for looking at the loop to get the the admin email addresses. I see it is repeating the problems of "contact this team" that I wanted to avoid. We want to send an email to the most immediate members who know about the team, the action, and can do something about it. I think there are two issues in the loop.

1 getDirectAdministrators() does not (get direct administrators). I own several teams that I am not a member of. I am *not* an admin, but this method is injecting the team owner into the list. I can use team.adminmembers to get just the members with the status ADMIN. This method is repeating the mistakes of the past where a user want to contact a team, but we walk the hierarchy of owner instead. Mark gets email about teams that are 3 and 4 teams removed from him :( This implementation was chosen because send emails to the individual members of the owner team fails when the owners do not have email address (team). This is a rare case, but it affects prominent teams.

2 If I had written a test where one of the admins was a team without an email address, I would have noticed that the rules do not traverse admin teams as I want. The code needs to locate an email address for the admin team. Choosing to get the contact address of the members will fail for the same reason as owner described above. The code needs to traverse a hierarchy to get a person or team with an email address.

Lets discuss this when you have time later today. I will work on the next branch which is independent of this branch.