Previous owner is not notified when ticket is reassigned since 1.1.3

[Jun Omae]

New notification system breaks #2311 feature. See also comment:4:ticket:11533.

There is NotificationTestCase.test_previous_owner in trac/ticket/tests/notification.py for the feature. However, all tests in the file don't use NotificationSystem and are dysfunctional for trunk.

We should use NotificationSystem in the tests.

[Peter Suter]

Right, the patch in #11533 should fix this.

Yes, the tests still test the old TicketNotify classes. Switching them to test the new code sounds good. (Though some of them seem to fail then.)

[Peter Suter]

In #11934 ([13703]) all tests except test_previous_cc_list and test_previous_owner were converted to use NotificationSystem. This patch also converts (and fixes) these two tests.

(It's an updated version of the patch in #11533.)

[Jun Omae]

Replying to psuter:

In #11934 ([13703]) all tests except test_previous_cc_list and test_previous_owner were converted to use NotificationSystem. This patch also converts (and fixes) these two tests.

Looks good to me except one. We could use Chrome.cc_list() to parse Cc field in CarbonCopySubscriber.matches(). According to the method, ; character is able to use in Cc field.

Two questions:

1. Should we notify previous reporter if the reporter field is changed?
2. Why do not we pass ticket's creation time to TicketChangeEvent when initiating ticket created event?

   -        event = TicketChangeEvent('created', ticket, None, ticket['reporter'])
   +        event = TicketChangeEvent('created', ticket, ticket['time'],
   +                                  ticket['reporter'])
   

[Peter Suter]

Thanks for the review.

Replying to jomae:

We could use Chrome.cc_list() to parse Cc field in CarbonCopySubscriber.matches(). According to the method, ; character is able to use in Cc field.

Sounds good.

1. Should we notify previous reporter if the reporter field is changed?

That would be a new feature, right? I think it could make sense but mainly for "symmetry". (Changing reporter seems rare and I assume the previous reporter is usually obsolete or invalid.)

2. Why do not we pass ticket's creation time to TicketChangeEvent when initiating ticket created event?

   -        event = TicketChangeEvent('created', ticket, None, ticket['reporter'])
   +        event = TicketChangeEvent('created', ticket, ticket['time'],
   +                                  ticket['reporter'])
   

No good reason, I just didn't need it so far. I agree we should pass it. Thanks again.

[Peter Suter]

The attached patch​ should be applied on top of the previous one​.

Attachment deletion​ and batch retargeting after milestone deletion​ or completion​ still pass None instead of deletion / retargeting time, as I don't see an obvious way to get at the appropriate value.

[Jun Omae]

1. Should we notify previous reporter if the reporter field is changed?

That would be a new feature, right? I think it could make sense but mainly for "symmetry". (Changing reporter seems rare and I assume the previous reporter is usually obsolete or invalid.)

Okay. I agree that's rare and previous reporter is usually invalid. It is not needed to notify to previous reporter.

[Peter Suter]

Committed in [13746].