Edgewall Software
Modify

Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#8758 closed enhancement (fixed)

Timeline filtering in Multirepos

Reported by: termim@… Owned by: Christian Boos
Priority: high Milestone: 0.12-multirepos
Component: timeline Version: 0.12dev
Severity: normal Keywords: multirepos
Cc: termim@…, ydirson@…, leho@…
Release Notes:
API Changes:

Description

It would be very handy to have an ability to filter Timeline contents by repositories. Say have a set of check boxes one for each configured repository. The defaults for these check boxes could be set in the trac.ini

Attachments (3)

timeline_filter_repositories.diff (1.3 KB ) - added by Mikhail Terekhov <termim@…> 8 years ago.
Add filtering by repositories to the timeline.
timeline_filter_repositories-sorted.diff (1.4 KB ) - added by termim 8 years ago.
sort by reponames
timeline_filter_repositories-sorted.png (160.6 KB ) - added by Christian Boos 7 years ago.
testing timeline_filter_repositories-sorted.diff

Download all attachments as: .zip

Change History (23)

Changed 8 years ago by Mikhail Terekhov <termim@…>

Add filtering by repositories to the timeline.

comment:1 Changed 8 years ago by Mikhail Terekhov <termim@…>

Keywords: multirepos added; Multirepos removed
Priority: normalhigh
Version: 0.12dev

comment:2 Changed 8 years ago by Christian Boos

Nice, but what if you have dozens of repositories?

I think that rather to add more and more filters next to the checkboxes for enabling the event providers, we should perhaps have a real "Filter options" panel, at the bottom (like http://www.cvstrac.org/cvstrac/timeline, only a bit more Trac than Cvs for the styling ;-) ).

I'll test the patch anyway and give more feedback.

Changed 8 years ago by termim

sort by reponames

comment:3 in reply to:  2 Changed 8 years ago by termim

Replying to cboos: You are right but it is probably too late for 0.12 right now. But IMHO the functionality is really needed for multirepos version. IMO if repositories number is less than one or two dozens then this simple solution is good enough. May be add an option to trac.ini to disable filtering?

As for CvsTrac way of filtering - I'd prefer filtering controls on the top and not too convoluted.

comment:4 Changed 8 years ago by Christian Boos

Convoluted CvsTrac? Nooooo ;-)

But this is precisely why I'd like to put the filters somewhere else.

In 0.12, the timeline option panel at the upper right already starts to be scary:

View changes from _______
and __ days back
done by __________

(filters)

I think that before adding more, we should seriously think about alternatives like an advanced panel (and then possibly move the done by ___ a.k.a. filtering by user (#1198) there as well).

comment:5 Changed 8 years ago by Christian Boos

To be clear, what I have in mind is something like:

 * Page Blah     +-----------------------+
   edited        | View changes from ___ |
                 | and __ days back      |
 * Ticket #3232  |[x] Changesets         |
   created       |[x] Tickets            |
                 +-----------------------+
 * changeset [xyz] by uzer

 ...

 ...

 +-(V) Additional filters -----------------+
 | View changes made by: ___________       |
 | View changesets from repositories:      |
 | [x] repo1  [x] repo2 [x] repo3 ...      |
 | ..................................      |
 |  [x] repo31  [x] repo32 [x] repo33      |
 +-----------------------------------------+

With the Additional filters being a collapsed panel like the Columns one in the Custom Query view.

One disadvantage would be that those two panels would each have its own Update button…

comment:6 Changed 7 years ago by Christian Boos

The patch works almost fine, but here are a few remarks.

The repositories checkboxes have no relations to the Repository checkins one. Even visually, they appear as part of the other "filters", which is not OK. At least they should appear indented, at best they get disabled/enabled when the Repository checkins checkbox is toggled.

Also, the default repos appears as a checkbox with no label, and enabling it has no effect. So either handle that case specially or let the alias do the job. In fact, forcing the alias to be used is even better, as this way, all changesets will have the "in …" qualifier. The problem is that there's not always an alias defined for the default repository.

All in all, I think now that this approach is worth implementing as a first step.

comment:7 in reply to:  5 Changed 7 years ago by Mikhail Terekhov <termim@…>

Replying to cboos:

With the Additional filters being a collapsed panel like the Columns one in the Custom Query view.

One disadvantage would be that those two panels would each have its own Update button…

I like the idea of collapsible panel. But I don't like controls scattered all over the page. Also a consistency with other pages is important IMHO. May be add collapsible part to the existing panel? Something like "Additional filters …". Unfortunately I don't know how to do that :(.

comment:8 Changed 7 years ago by Christian Boos

Owner: set to Christian Boos
Status: newassigned

Slightly improved version of timeline_filter_repositories-sorted.diff committed in r8775. Simple all-in-one panel approach won ;-)

Thanks Mikhail for the patch.

A few more fine tunings will follow.

comment:9 Changed 7 years ago by Christian Boos

In r8776, I made sure the timeline events for the default repository are also generated when there's only the default repository.

A minor problem is that in case of an error with the provider, the TracError message will read:

Repository checkins, &nbsp;&sdot;&nbsp;linux, (... the whole list goes here ...) event provider (ChangesetModule) failed  ...

For this reason and other (e.g. do the grouping of checkbox in a more structured way than just the visual hint given by the  ⋅  prefix), the ITimelineEventProvider should be refactored to properly identify providers and filters. The current get_timeline_filters methods should be renamed get_timeline_providers and a new get_timeline_provider_filters method should be added for returning the extra filters associated with a provider.

This would allow in the UI to disable/hide the filters when the provider is unchecked, and to enable/show them when the provider is checked back. This would also allow for other kind of controls than checkboxes. But that's for next-major ;-)

Now trying to fix the tricky case of default repository having no aliases…

comment:10 Changed 7 years ago by Christian Boos

Resolution: fixed
Status: assignedclosed

Additional cases handled in [8780].

comment:11 Changed 7 years ago by Remy Blank

How should the .hidden attribute work in relation to the timeline? Should we hide events coming from hidden repositories? Should we remove the checkbox for hidden repositories?

comment:12 in reply to:  11 Changed 7 years ago by Christian Boos

Replying to rblank:

Should we hide events coming from hidden repositories? Should we remove the checkbox for hidden repositories?

That's how it (should) already work, in the current code.

comment:13 Changed 7 years ago by Christian Boos

… but I agree that the logic there starts to be complex, so I documented the rules while adding one more use case in r8781.

Hope everything is clear (and correct!) now.

comment:14 Changed 7 years ago by Christian Boos

Cc: ydirson@… added

Already a duplicate for this one (#8844). So yes, given that not so many people are currently using multirepos, I think this was a needed feature.

Yann, feel free to give us some feedback here about how you like (or not) the implementation of this feature.

comment:15 Changed 7 years ago by Yann Dirson <ydirson@…>

It just feels like the feature I was missing, thx. Will keep you informed as I use it, in case I feel something would need improvements.

comment:16 Changed 7 years ago by lkraav <leho@…>

Cc: leho@… added

i just set up my first multirepos install off r8870. looking good so far. this timeline filtering issue popped up quickly. from what i gather, the "repository checkins" checkmark doesn't do anything by itself, nor does it really seem do anything when a multirepo name is checked. my desire is to have a single switch that would give me all multirepos=on and perhaps have a singlemultirepo=off or something. kinda sucky to start changing RSS URLs when adding/removing repos off the list.

or am i missing something?

comment:17 in reply to:  16 ; Changed 7 years ago by Christian Boos

Resolution: fixed
Status: closedreopened

Replying to lkraav <leho@…>:

am i missing something?

No, you're right. We could change the "repository checkins" filter into an "all repository checkins", and that would force to take all the repositories into account, regardless of the values of the other filters. Let me experiment a bit with this.

comment:18 in reply to:  17 Changed 7 years ago by Christian Boos

Resolution: fixed
Status: reopenedclosed

Replying to lkraav <leho@…>:

Please try out r8876, it should now work as you expected (let me know).

Thank you for the suggestion!

comment:19 Changed 7 years ago by lkraav <leho@…>

very nice, i appreciate it. couple of notes:

  • url has changeset, ui has checkins - issue perhaps (semantics, hci, whatnot)?
  • too bad repo:name gets escaped into fuglyworld of %3A

other than that, all bueno.

comment:20 Changed 7 years ago by Christian Boos

  • "Checkins" was traditionally used here in the timeline, but it must be the only place. Let's fix that.
  • Well, we only need a kind of unique prefix, so repo- would do as well.

Done in r8877.

Modify Ticket

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