Modify

Opened 5 years ago

Last modified 2 years ago

#6452 new defect

No notification sent to previous owner when a ticket is reassigned

Reported by: rjollos Owned by: hasienda
Priority: normal Component: AnnouncerPlugin
Severity: major Keywords: ticket email notification
Cc: hasienda Trac Release: 0.11

Description

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.

Attachments (0)

Change History (10)

comment:1 Changed 5 years ago by doki_pen

  • Priority changed from normal to high
  • Severity changed from normal to major
  • Status changed from new to assigned

You're right.

comment:2 Changed 4 years ago by hasienda

  • Cc hasienda added

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

comment:3 Changed 4 years ago by doki_pen

  • Priority changed from high to normal

comment:4 Changed 2 years ago by hasienda

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

comment:5 Changed 2 years ago by rjollos

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.

comment:6 Changed 2 years ago by hasienda

  • Keywords ticket email notification added

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

comment:7 Changed 2 years ago by hasienda

(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.

comment:8 follow-up: Changed 2 years ago by hasienda

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

comment:9 Changed 2 years ago by doki_pen

  • Owner changed from doki_pen to hasienda
  • Status changed from assigned to new

comment:10 in reply to: ↑ 8 Changed 2 years ago by rjollos

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.

Add Comment

Modify Ticket

Action
as new The owner will remain hasienda.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.