626522 - Crash List Graph Dates are Wrong

Status: VERIFIED FIXED

(Socorro :: General, task)

Description

[Ryan Snyder [:ryansnyder] [:rsnyder] [:rysny]]

In the list of crash reports for a crash signature, the graph dates are incorrect.

See:

http://crash-stats.mozilla.com/report/list?range_value=2&range_unit=weeks&signature=UserCallWinProcCheckWow&version=Firefox%3A3.6.13

http://crash-stats.mozilla.com/report/list?range_value=2&range_unit=weeks&signature=SkypeFfComponent.dll%400x440c3&version=Firefox%3A3.6.13

The graphs for these pages are labeled 11/22 through 12/3, when they should be today minus 2 weeks, e.g. 1/3 through 1/17.

Comment 1

[Schalk Neethling [:espressive]]

This seems to be happening in do_list.php

Comment 2

[Robert Kaiser]

The graph dates are not wrong, it's just misleading as there's no description of what is graphed. The graph is by build date/ID, not by crash date.

Comment 3

[Schalk Neethling [:espressive]]

Attachment: Slight 'redesign' of the graph

Comment 4

[Robert Kaiser]

Comment on attachment 566791 [details]
Slight 'redesign' of the graph

Umm could you describe what you "redesigned"? To me, the screen shot looks just the same as what I'm seeing in production...

Comment 5

[Schalk Neethling [:espressive]]

Attachment: Production screen shot

Here is what I am seeing in production. So in the revised version the legend does not have the borders around each cell, they are more spaced apart, not as tightly on top of the graph and the graph's width is extended.

Comment 6

[Robert Kaiser]

OK, those changes are nice in terms of the look (I take it that having one more date listed in the graph is a data glitch/difference) but they don't address the original concern of this bug in any way, I guess that's what surprised me. ;-)

The additional width is not present where we actually have a large amount of dates, right?
An extreme one with a ton of dates is https://crash-stats.mozilla.com/report/list?range_value=4&range_unit=weeks&signature=js%3A%3Agc%3A%3APushMarkStack

Comment 7

[Schalk Neethling [:espressive]]

[:[e-mail]] Wow, on that one the dates are completely unreadable. The current code will only allow the graph to stretch to a max width of 800px.

Comment 8

[Schalk Neethling [:espressive]]

Even stretching it to 1200px does not help much, I assume those are very rare?

Comment 9

[Schalk Neethling [:espressive]]

Adding support for zooming to that graph might help when the data is so dense, not sure if you are keen on something such as that. 

See here for an example:
http://people.iola.dk/olau/flot/examples/zooming.html

Comment 10

[Robert Kaiser]

Well, the graph itself on that page is OK for what we need it for (e.g. detecting spikes and tendencies), but you're right that the dates on it become unreadable - caring about that instead of zooming the graph itself is probably more helpful. But then this is probably yet another bug orthogonal to the report here. :)

Comment 11

[Schalk Neethling [:espressive]]

[:[e-mail]] Would this not be an option:

http://people.iola.dk/olau/flot/examples/interacting.html (test it with tooltips turned on). One can then remove the dates from the x axis and show them when hovering over a point.

We can then also have some text below the graph explaining how it works, for exmaple:

"Hover over a point above to see the build date"

Comment 12

[Robert Kaiser]

I think tooltips might be a good option here, yes, and removing only overlapping dates on the axis, is that doable?

Also, to address the actual issue this bug was reported for some more, let's make the title "Crashes by Build Date", i.e. add "Date" to it.

Comment 13

[Schalk Neethling [:espressive]]

[:[e-mail]] "Crashes by Build Date" - This change has been already, not on the screen shot I sent though.

Will implement the tooltips and look at the possibility of only removing overlapping dates however, I have a feeling this is going to be an either or situation.

Comment 14

[Schalk Neethling [:espressive]]

Attachment: New graph with tooltips

I am hiding the dates on the xAxis by moving them way offscreen, not doing siaply:none for accessibility reasons. The reason I am hiding them as aposed to removing every second one when there is an overlap is two fold:

1) There is not real way to detect when the labels start to overlap
2) This is the bigger issue, any item(tick) that is removed from the xAxis will not be plotted so, it does not just hide the date text, it physically skips plotting it on the graph.

Comment 15

[Schalk Neethling [:espressive]]

[:[e-mail]] Each date has a width of 55px, seems a lot but, makes the job of the flot easier. With that, I would say, as soon as the number of dates on the x axis exceeds 20 we need to hide them. Thoughts?

Comment 16

[Robert Kaiser]

20 as a limit sounds good to me, that makes "every day in two weeks" still have dates displayed, which is also helpful.

Comment 17

[Schalk Neethling [:espressive]]

[:[e-mail]] Sent pull request: 
https://github.com/mozilla/socorro/pull/84

Comment 18

[[github robot]]

Commit pushed to https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/b9a3ddefe7fdecdb707d5a4454f0770c3b05b6b5
Merge pull request #84 from ossreleasefeed/crash-list-graph

Fixes to crash list graph, fixes bug 626522

Comment 19

[Laura Thomson :laura]

Will want merge to release branch after we push today's release, too.

Comment 20

[Laura Thomson :laura]

Whoops, accidentally unresolved.

Comment 21

[Laura Thomson :laura]

Argh, why does that keep happening?

Comment 22

[Matt Brandt [:mbrandt]]

Attachment: Incorrect date range

The graph redesign look great, its quite a bit more readable and the tool tips are a nice touch.

Steps to reproduce:
1. goto https://crash-stats-dev.allizom.org/report/list?range_value=2&range_unit=weeks&signature=UserCallWinProcCheckWow&version=Firefox%3A3.6.13
2. goto the graph tab

Expected:
The date range should for the previous 2-weeks.

Actual:
The date range on the graph is 11/22 - 12/03

Comment 23

[Robert Kaiser]

(In reply to Matt Brandt [:mbrandt] from comment #22)
> Expected:
> The date range should for the previous 2-weeks.
> 
> Actual:
> The date range on the graph is 11/22 - 12/03

Wrong. Those are BUILD dates, not CRASH dates...

Comment 24

[Matt Brandt [:mbrandt]]

(In reply to Robert Kaiser (:[e-mail]) from comment #23)
> Wrong. Those are BUILD dates, not CRASH dates...
Thx for setting the record straight. Upon closer look you clearly noted this in comment 2.

Bumping to QA verified. The graph changes look great.

Comment 25

[Justin Dolske [:Dolske]]

Clearing unassigned review requests from this bug since it's already marked as fixed. If there are remaining issues, please file a new bug with new patch, and request review from a specific reviewer.