Edgewall Software
Modify

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#9036 closed enhancement (fixed)

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

Reported by: Ryan Ollos <ryano@…> Owned by: Ryan Ollos <ryano@…>
Priority: normal Milestone: 0.12
Component: roadmap Version: 0.12dev
Severity: normal Keywords:
Cc: Branch:
Release Notes:
API Changes:

Description

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)
    

Attachments (11)

hidenoduedate.patch (2.9 KB ) - added by Ryan Ollos <ryano@…> 10 years ago.
MilestonesFilter.png (4.3 KB ) - added by Ryan Ollos <ryano@…> 10 years ago.
MilestonesFilter2.png (4.1 KB ) - added by Ryan Ollos <ryano@…> 10 years ago.
hidenoduedate.patch2 (2.9 KB ) - added by Ryan Ollos <ryano@…> 10 years ago.
Patch with suggestion from comment:3
hidenoduedate2.patch (2.9 KB ) - added by Ryan Ollos <ryano@…> 10 years ago.
Same patch as contained in hidenoduedate.patch2
hidenoduedate.2.patch (2.9 KB ) - added by Ryan Ollos <ryano@…> 10 years ago.
MilestoneFilter.3.png (4.3 KB ) - added by Ryan Ollos <ryano@…> 10 years ago.
Screen capture corresponding to hidenoduedate.3.patch
hidenoduedate.3.patch (3.3 KB ) - added by Ryan Ollos <ryano@…> 10 years ago.
Patch implementing two additional filters
ImprovedRoadmapFilter.png (7.8 KB ) - added by Ryan Ollos <ryano@…> 10 years ago.
Improved roadmap filter (possibly with too many features ;)
hidenoduedate.4.patch (2.8 KB ) - added by Ryan Ollos <ryano@…> 10 years ago.
Patch implementing only 'Hide if no due date' feature
hidenoduedate.5.patch (2.8 KB ) - added by Ryan Ollos <ryano@…> 10 years ago.
Final version (hopefully) of patch for filter 'Hide if no due date'

Download all attachments as: .zip

Change History (28)

by Ryan Ollos <ryano@…>, 10 years ago

Attachment: hidenoduedate.patch added

by Ryan Ollos <ryano@…>, 10 years ago

Attachment: MilestonesFilter.png added

by Ryan Ollos <ryano@…>, 10 years ago

Attachment: MilestonesFilter2.png added

comment:1 by Ryan Ollos <ryano@…>, 10 years ago

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.

comment:2 by Ryan Ollos <ryano@…>, 10 years ago

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.

in reply to:  description ; comment:3 by Remy Blank, 10 years ago

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

in reply to:  3 ; comment:4 by Ryan Ollos <ryano@…>, 10 years ago

Replying to rblank:

Cleaner, more efficient (your implementation is O(n2)) 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.

by Ryan Ollos <ryano@…>, 10 years ago

Attachment: hidenoduedate.patch2 added

Patch with suggestion from comment:3

comment:5 by Ryan Ollos <ryano@…>, 10 years ago

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

by Ryan Ollos <ryano@…>, 10 years ago

Attachment: hidenoduedate2.patch added

Same patch as contained in hidenoduedate.patch2

by Ryan Ollos <ryano@…>, 10 years ago

Attachment: hidenoduedate.2.patch added

comment:6 by Ryan Ollos <ryano@…>, 10 years ago

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

by Ryan Ollos <ryano@…>, 10 years ago

Attachment: MilestoneFilter.3.png added

Screen capture corresponding to hidenoduedate.3.patch

by Ryan Ollos <ryano@…>, 10 years ago

Attachment: hidenoduedate.3.patch added

Patch implementing two additional filters

comment:7 by Ryan Ollos <ryano@…>, 10 years ago

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.

Screen capture corresponding to hidenoduedate.3.patch

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.

by Ryan Ollos <ryano@…>, 10 years ago

Attachment: ImprovedRoadmapFilter.png added

Improved roadmap filter (possibly with too many features ;)

comment:8 by Ryan Ollos <ryano@…>, 10 years ago

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:

Improved roadmap filter (possibly with too many features ;)

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

in reply to:  4 comment:9 by Remy Blank, 10 years ago

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.

by Ryan Ollos <ryano@…>, 10 years ago

Attachment: hidenoduedate.4.patch added

Patch implementing only 'Hide if no due date' feature

by Ryan Ollos <ryano@…>, 10 years ago

Attachment: hidenoduedate.5.patch added

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

comment:10 by Ryan Ollos <ryano@…>, 10 years ago

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 milestonesShow completed milestones.

comment:11 by Remy Blank, 10 years ago

Owner: set to Remy Blank

I'll test the patch.

comment:12 by Remy Blank, 10 years ago

Resolution: fixed
Status: newclosed

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.

comment:13 by Remy Blank, 10 years ago

Owner: changed from Remy Blank to Ryan Ollos <ryano@…>

in reply to:  12 ; comment:14 by Ryan Ollos <ryano@…>, 10 years ago

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.

in reply to:  14 comment:15 by Remy Blank, 10 years ago

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.

comment:16 by Ryan Ollos <ryano@…>, 10 years ago

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

in reply to:  16 comment:17 by Christian Boos, 10 years ago

Replying to Ryan Ollos <ryano@…>:

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

Nice, thanks!

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan Ollos <ryano@…>.
The resolution will be deleted. Next status will be 'reopened'.
to as closed The owner will be changed from Ryan Ollos <ryano@…> 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.