[PATCH] Searching in milestones

[Remy Blank <remy.blank@…>]

The search function currently doesn't search in milestones. The attached patch adds an ISearchSource to MilestoneModule and allows searching in the milestone name and description.

The patch depends on #2561 for searching in milestone attachment metadata as well. Should the former not be accepted, the last section starting with the # Attachments comment can simply be dropped.

[Remy Blank <remy.blank@…>]

Patch against 0.11-stable [7346] adding searching in milestones

[Remy Blank <remy.blank@…>]

A few comments:

• The author field of milestone search results is set to the empty string. This is consistent with the data returned for timeline events. However, for milestone events in the timeline, the author is not displayed at all, whereas in search results, the author appears as anonymous. Maybe this should be fixed in the search template in the same way as in the timeline.

• The time field of milestone search results is set to the completed date if it is set, otherwise to the due date if it is set, otherwise to "now". I'm not quite sure about this last case, but setting it to 0 makes all milestones with no due or completed date come last, as the search result list is sorted in descending time order. And setting it to an arbitrary date in the future (e.g. in 1 month) doesn't sound convincing either.

[Remy Blank <remy.blank@…>]

Do not show author in search results if it is empty

[cboos]

<py:if test="result.author"><span class="author">...

can also be written:

<span py:if="result.author" class="author">...

[Remy Blank <remy.blank@…>]

Not if you include the &mdash; at the end of the line ;-)

But apart from that, does it make sense to remove the author when it is empty, or does some other functionality rely on it being there?

[cboos]

Replying to Remy Blank <remy.blank@pobox.com>:

Not if you include the &mdash; at the end of the line ;-)

Morning reviewing not a good thing ;-)

But apart from that, does it make sense to remove the author when it is empty, or does some other functionality rely on it being there?

I think it's OK to remove it.

[cboos]

0.11.1, as any other minor release, is "Bug fixes and minor improvements to 0.11.", so I think this feature (as well as #2561) is fine for being included there.

[cboos]

Both patches applied, in [7351] and [7352].