No notification sent to previous owner when a ticket is reassigned

[Ryan J Ollos]

When a user is removed from the CC list, they receive the a notification email showing the change to the CC list. This is the last email they receive unless they are also the owner or reporter (or component owner and have opted in to notifications).

It seems to me that the same behavior should also be observed when a ticket is reassigned. If the owner is changed, the previous owner should receive one final email showing them that the ticket was reassigned.

However, I have tested and this is not the behavior I am seeing. If another user reassigns a ticket that I own, I receive no notification of that change unless I'm also the ticket owner or on the CC list.

[Robert Corsaro]

You're right.

[Steffen Hoffmann]

​TracNotification (see ​Trac #2311) shows equal misbehavior, but priority to fix it is not so high over there.

[Steffen Hoffmann]

(In [12333]) TracAnnouncer: Notify previous owner, when a ticket is reassigned, refs #6452.

[Ryan J Ollos]

I'll need to do some testing to see if I can make this fail, but from just looking at the code I see the possibility of an unbound local variable exception at lines 142 and 147 when the condition part of the control statement if ticket['owner'] and ticket['owner'] != 'anonymous' evaluates to false, due to the use of the undefined variable sid.

Possibly related, I'm not sure why sid is being set to None here:

if ticket['owner'] and ticket['owner'] != 'anonymous':
    if re.match(r'^[^@]+@.+', ticket['owner']):
        sid, auth, addr = None, 0, ticket['owner']
    else:
        sid, auth, addr = ticket['owner'], 1, None 
    sid = None

Perhaps that code should be modified to?:

if ticket['owner'] and ticket['owner'] != 'anonymous':
    if re.match(r'^[^@]+@.+', ticket['owner']):
        sid, auth, addr = None, 0, ticket['owner']
    else:
        sid, auth, addr = ticket['owner'], 1, None 
else:
    sid = None

Moving sid = None to within an else statement seems to avoid the possibility of an unbound local variable exception.

Just some initial thoughts ... I'll do some testing and follow-up. I'll be looking to see if I can invoke an exception in that case that the ticket is assigned to anonymous.

[Steffen Hoffmann]

Sure, you're right, must have been too late - but I see this now, no need to test.

[Steffen Hoffmann]

(In [12334]) TracAnnouncer: Prevent unbound local variable exception in code from [12333], refs #6452.

Thanks to Ryan for changeset review and spotting this really fast.

[Steffen Hoffmann]

The new code should work under all circumstances, and it's a bit shorter too.

[Ryan J Ollos]

Replying to hasienda:

The new code should work under all circumstances, and it's a bit shorter too.

Yes, that change looks very nice, both for readability and robustness.