[Patch] Add a filter to the roadmap page for "milestones with no due date"

[Ryan Ollos <ryano@…>]

I'd find it useful to have a filter on the roadmap page for Milestones with no due date. The attached patch implements this by adding a Filter named Hide milestones with no due date.

My justification for this feature is that we have a lot of future milestones defined on my Trac instance, and usually we want to ignore these when viewing the roadmap.

At first, I was thinking that it made more sense to have a checkbox Show milestones with no due date.

To preserve the existing behavior you would want that checkbox to be enabled by default. I'm not sure how to go about that.

I quickly implemented the patch today so that I could get some feedback and over the weekend make any additional modifications that will be needed.

There are some issues:

• The checkbox Hide milestones with no due date does not stay selected when the page refreshes.
• I haven't set up my devenv to run the unit tests … I need to read-up on how to do this and can resubmit the patch with any necessary additions/modifications to associated unit tests, if necessary.
• The implementation might be cleaner if there was a show_noduedate parameter for Milestone.select method. In that case, the following block of code would not be necessary:

        if hide_noduedate:
            for milestone in milestones:
                if milestone.due is None:
                    milestones.remove(milestone)
  

[Ryan Ollos <ryano@…>]

One other comment is that I see how someone could add a filter to the Timeline page via get_timeline_filters in the ITimelineEventProvider interface, but I don't see a similar extension point for the roadmap. It seems like it would be useful to have an interface such that someone could write a plugin to extend the roadmap filter. I'm sure this wouldn't be nearly as useful as adding filters to the Timeline page, but could be useful in some circumstances.

[Ryan Ollos <ryano@…>]

Please ignore the first bullet:

• The checkbox Hide milestones with no due date does not stay selected when the page refreshes.

I fixed that issue before submitting the patch and forgot to remove the comment from the draft of my ticket.

[Remy Blank]

Replying to Ryan Ollos <ryano@…>:

• The implementation might be cleaner if there was a show_noduedate parameter for Milestone.select method. In that case, the following block of code would not be necessary:

        if hide_noduedate:
            for milestone in milestones:
                if milestone.due is None:
                    milestones.remove(milestone)
  

Cleaner, more efficient (your implementation is O(n²)) and more correct (mutating a list while iterating is almost never a good idea):

if hide_noduedate:
    milestones = [m for m in milestones if m.due is not None]

About the roadmap extension point, I think it would be an over-generalization, as the roadmap will only ever list milestones (as opposed to the timeline, which can display arbitrary events from multiple providers).

[Ryan Ollos <ryano@…>]

Replying to rblank:

Cleaner, more efficient (your implementation is O(n²)) and more correct (mutating a list while iterating is almost never a good idea):

if hide_noduedate:
    milestones = [m for m in milestones if m.due is not None]

Thanks, that change is implemented in hidenoduedate.patch2 and I tested it out a bit.

Do you have a preference on using show vs hide?

About the roadmap extension point, I think it would be an over-generalization, as the roadmap will only ever list milestones (as opposed to the timeline, which can display arbitrary events from multiple providers).

I'm interested in potentially adding the following to the filter box on the roadmap page:

1. View milestones due between date X and date Y
2. Hide overdue milestones
3. Show milestones matching a specified pattern

1 and 2 would be most useful for me since we have a lot of milestones in my Trac instance. We use milestones to denote software sub-iterations: there are typically 4 sub-iterations in every two week iteration and we have about 40 open entries on the roadmap page at the moment.

Since there is not an extension point, I don't see how to implement these filters through a plugin. If you think any of these features would be of general use, then I'd implement them in Trac, but my concern is that they might not and would just make the interface more complicated for other users.

[Ryan Ollos <ryano@…>]

Patch with suggestion from comment:3

[Ryan Ollos <ryano@…>]

I see that files ended in .patch are nicely syntax highlighted, so I renamed the previous patch to hidenoduedate2.patch.

[Ryan Ollos <ryano@…>]

Same patch as contained in hidenoduedate.patch2

[Ryan Ollos <ryano@…>]

Sorry, I messed up the previous patches. attachment:hidenoduedate.2.patch​ is the correct one.

[Ryan Ollos <ryano@…>]

Screen capture corresponding to hidenoduedate.3.patch

[Ryan Ollos <ryano@…>]

Patch implementing two additional filters

[Ryan Ollos <ryano@…>]

Patch attachment:hidenoduedate.3.patch​ adds another filter for Hide overdue milestones, where overdue milestones are those that are late and not completed.

I also fixed a silly mistake I made earlier in attachment:hidenoduedate.3.patch​ with the hide_noduedate filter selecting from the db rather than operating on the existing list.

If you think that the additional options might be useful to some, but not all users, perhaps we could add an option to enable the extra filters similar to ticket_show_details in TracIni#timeline-section.

[Ryan Ollos <ryano@…>]

Improved roadmap filter (possibly with too many features ;)

[Ryan Ollos <ryano@…>]

Replying to Ryan Ollos <ryano@…>:

I'm interested in potentially adding the following to the filter box on the roadmap page:

1. View milestones due between date X and date Y
2. Hide overdue milestones
3. Show milestones matching a specified pattern

This would look something like:

I'd implement the rest of these features in another ticket, if it is believed to be worthwhile.

[Remy Blank]

Replying to Ryan Ollos <ryano@…>:

Since there is not an extension point, I don't see how to implement these filters through a plugin. If you think any of these features would be of general use, then I'd implement them in Trac, but my concern is that they might not and would just make the interface more complicated for other users.

You should be able to use the IRequestFilter and ITemplateStreamFilter interfaces for that. In IRequestFilter.post_process_request() you have access to the request arguments and the template data, so you can filter the list of milestones there. And in ITemplateStreamFilter.filter_stream(), you can filter the generated HTML and add the additional fields to the form.

I agree with you that the complete set of filters is a tad too specific to be included in core, and would clutter the interface unnecessarily for most people. The "hide milestones with no due date" OTOH I would find generally useful.

[Ryan Ollos <ryano@…>]

Patch implementing only 'Hide if no due date' feature

[Ryan Ollos <ryano@…>]

Final version (hopefully) of patch for filter 'Hide if no due date'

[Ryan Ollos <ryano@…>]

Thanks for the feedback and suggestions. I'll implement the other features in a plugin.

attachment:hidenoduedate.5.patch​ implements only the Hide if no due date filter. It is pretty much the same as attachment:hidenoduedate.2.patch​, but contains a fix for a mistake I made earlier.

It also renames the existing filter Show already completed milestones → Show completed milestones.

[Remy Blank]

I'll test the patch.

[Remy Blank]

Slightly adapted patch applied in [9237]:

• Use multiple show= URL arguments instead of arguments with distinct names.
• Keep compatibility with the previous show=all URL argument.
• When showing completed milestones, also show completed milestones with no due date, even if the "Hide milestones with no due date" checkbox is checked.

[Ryan Ollos <ryano@…>]

Replying to rblank:

Slightly adapted patch applied in [9237]:

Thanks for implementing the modified patch, its nice to get some feedback so I can improve upon my next patch.

• Keep compatibility with the previous show=all URL argument.

I assume the intent of this is so that existing iCalendar subscriptions do not break during an upgrade, which is something I hadn't considered.

• When showing completed milestones, also show completed milestones with no due date, even if the "Hide milestones with no due date" checkbox is checked.

Do you think this is worth a brief mention in the documentation, TracRoadmap#TheRoadmapView, or would that be "too much information"?

I would keep any mention brief,

---

The Roadmap can be filtered to show/hide completed milestones and milestones with no due date. In the case that both show completed milestones and hide milestones with no due date are selected, completed milestones with no due date will be shown.

---

At the very least, this might avoid 1 or 2 bug reports when someone believes that the behavior is unintended.

[Remy Blank]

Replying to Ryan Ollos <ryano@…>:

Do you think this is worth a brief mention in the documentation, TracRoadmap#TheRoadmapView, or would that be "too much information"?

Yes, that's an excellent idea. Please edit 0.12/TracRoadmap, though, as that's where we keep the documentation for 0.12.

[Ryan Ollos <ryano@…>]

Just for reference, the changes to the documentation can be seen here.

[Christian Boos]

Replying to Ryan Ollos <ryano@…>:

Just for reference, the changes to the documentation can be seen here.

Nice, thanks!