557556 - support variable display ranges

Status: RESOLVED FIXED

(Tree Management Graveyard :: TBPL, defect)

Description

[Dietrich Ayala (:dietrich)]

currently TBPL shows 12 hour increments of pushes.

however, some trees have little activity, so the default display is empty. also, to go back 3 days to the last data, for example, requires a bunch of clicking and waiting.

in these cases, a "last X pushes" model would work better. for some trees or users a 24hr display range would be nice. tinderbox has this option.

i'm not sure yet how to surface this in the UI, but this patch begins the process of abstracting the display interval out, and making it configurable.

Comment 1

[Dietrich Ayala (:dietrich)]

Attachment: v1

Comment 2

[Arpad Borsos [:Swatinem]]

Comment on attachment 437343 [details] [diff] [review]
v1

>diff --git a/PushlogHTMLParser.js b/PushlogHTMLParser.js
>-    return "fetchraw.php?site=pushlog&url=" + repoName + "/pushloghtml%3Fstartdate=" + startDate + "%26enddate=" + endDate;
>+    return "http://tests.themasta.com/tinderboxpushlog/fetchraw.php?site=pushlog&url=" + repoName + "/pushloghtml%3Fstartdate=" + startDate + "%26enddate=" + endDate;

please don't hardcore urls

>diff --git a/UserInterface.js b/UserInterface.js
>-      '<span class="date">' + self._getDisplayDate(push.date) + '</span></h2>\n' +
>+      '<span class="date">' + self._getDisplayDate(push.date) + '</span>' +
>+      ' (compare: <input class="revsToCompare" id="compareRevs" type="checkbox" value="' + push.patches[0].rev + '" onclick="compare()">)' +
>+      '</h2>\n' +

seems like this got mixed up with the patch from bug 557552

That bug also abstracts the url handling which is nice, maybe you could split that out into a separate patch?

Looks nice apart from those small nits.

Comment 3

[Dietrich Ayala (:dietrich)]

Attachment: v2

comments addressed

Comment 4

[Markus Stange [:mstange]]

Comment on attachment 437349 [details] [diff] [review]
v2

The name "displayInterval" seems a little confusing next to "loadInterval" in Config.js, but I don't really have a better name... maybe displayTimeRange? Anyway, your choice.

Comment 5

[Phil Ringnalda (:philor)]

?hours=n seems like a good enough way to start exposing it - if we had that right now, I would be able to actually get tbpl to load (since over an imperfect connection, the 3-4MB json doesn't load in 2 minutes, at which point another and then another request piles up, and before long I'm failing to download 6 or 8 or 10 copies at a time).

Comment 6

[Dietrich Ayala (:dietrich)]

Not working on this at the moment.

Comment 7

[Arpad Borsos [:Swatinem]]

This is basically in bug 566870 as Config.goBackHours. But as tinderbox’s cached json has 12 hours of data, it would need to be changed in accordance to that.