Edit Milestone defaults to moving all open tickets to milestone None

[garyo@…]

If you edit a non-completed milestone, the checkbox to "Retarget associated open tickets to milestone:" defaults to checked, and the target milestone is None. If you don't notice that and just change the milestone text or date or whatever, all of a sudden all your tickets for that milestone are gone (moved to None). And there's no easy way to get them back except one by one.

This is in 0.11dev-r5923 (trunk as of a couple of weeks ago).

I suspect it's this line in milestone_edit.html:

<input type="checkbox" id="retarget" name="retarget" checked="checked" />

I don't think "checked" is a valid value there, maybe it should be a genshi var instead, or just default to off always since it's dangerous.

[ThurnerRupert]

change prio - trivial to fix ...

[cboos]

Please, don't touch the priority field, thanks.

[garyo@…]

Can anyone tell me what the "trivial fix" is? This has bitten us several times now, and I'd be happy to patch my local installation.

[sid]

trivial fix means that it is a very small change. If you create a patch for your local installation, it would be great if you attached the patch to this ticket so it can be reviewed and then (hopefully) merged into trunk.

[cboos]

I don't think the right fix would be "trivial" here: IMO, the default should be to retarget open tickets, so that you won't close a milestone which still contains opened tickets. But if the target is None (actually ""), then an extra confirmation step could be required, as, like garyo said, there would be no easy way to group them back.

[anonymous]

I disagree; there are many reasons to edit a milestone other than closing it. Though I do agree that if someone tries to retarget the open tickets to None it should warn.

We make more non-closing edits than closing for sure. This has bit us already a few times when we just adjust the completion date of an open milestone or add some text to the milestone and forget to uncheck the retarget checkbox.

As for the fix being trivial, I'm happy to patch it but as I mentioned in my submission there are two ways to go: a genshi var referring to the completion state (maybe "${milestone.completed or None}" like the "completed" checkbox above it) or default it off. But actually it should only get set automatically if you're completing the milestone on this edit, not if the milestone's *already* completed, right? And you don't know when you generate the html whether the user's going to complete the milestone this time.

I'm just going to default it off in my local copy for now, pending resolution of this issue.

[cboos]

Er, you mean it does retarget even when the milestone is not going to be completed? Then I agree it's quite buggy.

[garyo@…]

Absolutely. (btw, sorry the above anonymous comment 6 was me.) The checkbox defaults on always (because it says checked="checked"), and the retargeting code only looks at the checkbox value IIRC. I'm not going to try it again right now :-) because then I'd have to move my open tickets back again. There's some Javascript above it, but it doesn't seem to affect things in my world.

[cboos]

Quick note on this: the problem here was that normally the Javascript disables the checkbox field, so the fact that it's checked by default is not a problem.

That's not to say there's nothing to do here:

• the Javascript should still be used to disable the checkbox, but also to set it checked only at the time it gets enabled
• the model code should do some minimal checks as well (like only retarget tickets if the milestone was actually closed)

[kirean@…]

It would be nice to have the change of milestone traceable per ticket.. Can we include that in this ticket or should I create an enhancement ticket for this?

[cboos]

Replying to kirean@gmail.com:

It would be nice to have the change of milestone traceable per ticket.. Can we include that in this ticket or should I create an enhancement ticket for this?

No, there's already a ticket for that ;-)

[kirean@…]

Quick patch to avoid me assigning open tickets to Milestone None.

• Disabled and unchecked the retarget checkbox by default. To retarget open tickets you first need to check the retarget checkbox.
• Disabled milestone drop down list by default. Select milestones should not be available until retarget checkbox is checked.

milestone_edit.html

@@ -68 +68 @@
             </label>
             <py:if test="milestones">
               <br/>
-              <input type="checkbox" id="retarget" name="retarget" checked="checked" />
+              <input type="checkbox" id="retarget" name="retarget" disabled="true" />
               <label>Retarget associated open tickets to milestone:
-                <select id="target" name="target" py:with="t = req.args.get('target')">
+                <select id="target" name="target" disabled="true" py:with="t = req.args.get('target')">
                   <option value="">None</option>
                   <option py:for="m in milestones" selected="${m.name == t or None}">${m.name}</option>
                 </select>

[cboos]

Seems fine, except that this won't work with Javascript disabled... see comment:9.

[garyo@…]

Just updated to latest trunk (r6511) and this behavior is unchanged; still dangerous. The checkbox still defaults on even if no milestone is selected and you're not closing the milestone. I just patched it locally to turn it off by default:

Index: trac/ticket/templates/milestone_edit.html
===================================================================
--- trac/ticket/templates/milestone_edit.html   (revision 6511)
+++ trac/ticket/templates/milestone_edit.html   (working copy)
@@ -66,7 +66,7 @@
             </label>
             <py:if test="milestones">
               <br/>
-              <input type="checkbox" id="retarget" name="retarget" checked="checked" />
+              <input type="checkbox" id="retarget" name="retarget" />
               <label>Retarget associated open tickets to milestone:
                 <select id="target" name="target" py:with="t = req.args.get('target')">
                   <option value="">None</option>

Also for some reason even though I have Javascript enabled in my browser, the JS doesn't turn off that checkbox, so it really and truly is defaulted on in all cases. I do note that I get an error in the Firefox console:

$ is not defined

at line 23 which is:

      $(document).ready(function() {

maybe the weird /*<![CDATA[*/ comment above it is the problem that makes my browser not load/run the document's ready function? In case it's browser-related, I'm running Firefox 2 and 3 and it fails on both.

[osimons]

#6734 closed as a duplicate, confirming both 'edit issue' and 'javascript is actually enabled problem' on current 0.11dev.

BTW: The javascript issue of $ is not defined looks like a missed conversion to new jQuery.noconflict() mode.

[gregoryba@…]

1 vote for this behavior to be fixed. chiming in here to get added to CC (why is there no CC textbox in the 'change properties' section?)

[eric@…]

just got bitten by this. very nasty, hope it will be fixed soon.

[cboos]

Need some more info in order to be able to reproduce this: what Trac version, which browser... ?

[Remy Blank <remy.blank@…>]

I cannot reproduce this on either Firefox 2.0.0.16 or Opera 9.27 with JavaScript enabled: if the "Completed" check box is unchecked, no tickets are reassigned.

However, if I disable JavaScript, and the "Completed" check box is unchecked, then the tickets are reassigned. So the code should at least check if the "completed" check box has changed from unselected to selected before reassigning.

I'll try to propose a fix.

[Remy Blank <remy.blank@…>]

Make editing milestones less dangerous

[Remy Blank <remy.blank@…>]

The patch above makes editing milestones safer:

• Retargeting is only possible when the milestone is completed. This will prevent retargeting if JavaScript is disabled, and the "Completed" check box is unchecked.
• The retargeting check box is disabled by default if the milestone is completed. This doesn't change anything if JavaScript is enabled, but if it is disabled, it will prevent retargeting by default if a milestone is already completed.

It would still be interesting to find out how this problem could appear for people having JavaScript enabled. I still haven't been able to reproduce that.

[cboos]

Patch works fine.

One minor glitch though (already present before the patch) is that the "Retarget" label is not associated with its checkbox, which is inconsistent with the "Completed" one. I'll push a follow-up patch.

[cboos]

Fixed in [7372], see also [7373].