Edgewall Software
Modify

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#10925 closed defect (duplicate)

No ticket change data associated with re-targeting tickets to new milestone

Reported by: Ryan J Ollos <ryan.j.ollos@…> Owned by:
Priority: normal Milestone:
Component: roadmap Version:
Severity: normal Keywords:
Cc: Branch:
Release Notes:
API Changes:

Description

When a milestone is closed and tickets are re-targeted to a new milestone, there are no entries in the ticket_change table associated with the re-targeting and no notifications sent. This can make it difficult to track down mistakes, which is what led to my investigation that resulted in this ticket.

Suppose the following events.

  1. Milestone for a ticket X is changed from milestone A to milestoneB.
  2. Ticket X is open and milestoneB is closed with open tickets re-targeted to milestone C.
  3. Ticket X is changed to milestoneD.

The change history of ticket X will read:

  • Milestone changed from milestoneA to milestoneB
  • Milestone set to milestoneD

Attachments (3)

t10925-r11412-1.patch (2.2 KB ) - added by Ryan J Ollos <ryan.j.ollos@…> 6 years ago.
Patch against r11412 of the trunk.
t10925-r11412-2.patch (1.6 KB ) - added by Ryan J Ollos <ryan.j.ollos@…> 6 years ago.
Patch against r11412 of the trunk.
t10925-r11412-3.patch (3.2 KB ) - added by Ryan J Ollos <ryan.j.ollos@…> 6 years ago.
Patch against r11412 of the trunk that required TICKET_CHGPROP permission to change the ticket's milestone.

Download all attachments as: .zip

Change History (13)

by Ryan J Ollos <ryan.j.ollos@…>, 6 years ago

Attachment: t10925-r11412-1.patch added

Patch against r11412 of the trunk.

by Ryan J Ollos <ryan.j.ollos@…>, 6 years ago

Attachment: t10925-r11412-2.patch added

Patch against r11412 of the trunk.

comment:1 by Ryan J Ollos <ryan.j.ollos@…>, 6 years ago

I accidentally included the patch for #10924 in the first upload, so t10925-r11412-2.patch is the one to look at.

The patch adds a comment to each re-targeted ticket, for example: Tickets associated with milestoneA retargeted to milestoneB. I'm not sure I see the value of the comment and could easily be talked into modifying the patch to remove the comment.

comment:2 by Ryan J Ollos <ryan.j.ollos@…>, 6 years ago

I gave this some more thought, and considered whether a user with MILESTONE_EDIT should be allowed to change the milestone for a ticket, or whether that operation should require TICKET_CHGPROP. The following lightly tested patch checks for TICKET_CHGPROP permission for each resource before allowing the milestone to be changed. I'm just hoping to get some feedback on which direction to go, and then will submit a final patch and look at adding a testcase or two.

by Ryan J Ollos <ryan.j.ollos@…>, 6 years ago

Attachment: t10925-r11412-3.patch added

Patch against r11412 of the trunk that required TICKET_CHGPROP permission to change the ticket's milestone.

comment:3 by Peter Suter, 6 years ago

Resolution: duplicate
Status: newclosed

Looks like a duplicate of #5658. Don't let that discourage you though. :)

comment:4 by Ryan J Ollos <ryan.j.ollos@…>, 6 years ago

I'll leave the patches here for now, and hope to get some feedback. I'd post subsequent patches against #5658.

comment:5 by Peter Suter, 6 years ago

Since this seems similar to batch modify, let's compare with that:

(Of course it doesn't automatically mean that batch modify does all this correctly!)

The comment included in the patch seems a bit redundant to me. But as comment:8:ticket:5658 notes it may be useful to mention the reason for retargeting (here: closed milestone; #4582: renamed milestone).

I guess internationalization support should be added for the comment and the warning.

comment:6 by Remy Blank, 6 years ago

I agree with Peter that the behavior should be the same for batch modification and re-targeting on closing a milestone, should generate the same event in the timeline and the same notification.

comment:7 by Ryan J Ollos <ryan.j.ollos@…>, 6 years ago

I'll refresh the patch with these changes. I'd just note that although batch modify doesn't do a TICKET_CHGPROP check, it does require BATCH_MODIFY, which to me clearly says the user has been granted permission can modify all fields. On the other hand, I wouldn't expect the user to necessarily have the ability to modify any ticket fields given MILESTONE_EDIT permission.

comment:8 by Peter Suter, 6 years ago

Maybe it should actually require TICKET_BATCH_MODIFY?

comment:9 by Ryan J Ollos <ryan.j.ollos@…>, 6 years ago

Sorry I didn't mention earlier, but thank you very much for the feedback Pete and Remy! I have a refreshed patch that batch updates the timeline and sends a batch email notification. I've deferred the permissions issue for now, until someone with more experience tells me that it is worthwhile to implement. I've attached the patch to #5658, so I guess the review should continue in that ticket.

in reply to:  8 comment:10 by Ryan J Ollos <ryan.j.ollos@…>, 6 years ago

Replying to psuter:

Maybe it should actually require TICKET_BATCH_MODIFY?

Yeah, that sort of makes sense to me, and would certainly keep the code simpler.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The ticket will remain with no owner.
The resolution will be deleted.
to The owner will be changed from (none) to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.