44 bytes, text/x-github-pull-request
|Details | Review | Splinter Review|
For the Signature report page, Reports tab, please could you add "install time" as a default field for the crash reports displayed. When doing triage of nightly builds (see https://wiki.mozilla.org/Platform/Uptime/NightlyCrashTriage/2017Q1), one of the first things I want to see with any signature is the install times, so as to figure out whether this is a single-install crash or not. Currently I (and other triagers) have to request display of this field by hand every time, which slows the triage down, and is hard on the hands in what is already a mouse-intensive activity. This is surely just a question of adding "install time" to some python table of initially displayed fields, or something like that, right?
I attempted this, but failed. I thought that adding 'install_time' to DEFAULT_COLUMNS in webapp-django/crashstats/signature/views.py would do it. But then I saw this line in signature_report(): > context['columns'] = columns or DEFAULT_COLUMNS If I remove the |columns or| then it works. |columns| is coming from the '_columns' parameter in the request, but I can't work out where that is set. Adrian, any suggestions? Thanks.
Defaults or not, I thought the selection was saved in localStorage?! Perhaps something has regressed. I'd favor fixing the localStorage functionality first before discussing what ought to be the default for everyone. Adrian, I can dig into this one if you want.
peterbe: as per comment 1, I had to modify a line of code to get DEFAULT_COLUMNS applied. I couldn't work out where the '_columns' parameter was coming from; localStorage is a possibility.
I opened the code, hoping I can apply a quick fix. The way the `_columns` works is non-trivial. Perhaps Adrian, who wrote it, will be quicker to solve it but if he prefers I can take over and learn the code and drive a change. So, leaving the needinfo on Adrian.
Nicholas, you were on the right track. Your change was the right one, simply, if you test by refreshing the page you are already on, then columns are already set in the URL and your change to the default columns won't be taken. You'll want to test by returning to SuperSearch or TopCrashers and clicking a signature link again, and then it will work as you expected. However, I don't really want to add yet another field to the default list of columns on that page. Instead, I would propose that we add an aggregation on install_time by default in the Aggregations tab. Would that work for you? I think it would make it even better for you, since you can very very quickly see if it's a problem that happens on just one install or not.
Assignee: nobody → adrian
Flags: needinfo?(adrian) → needinfo?(jseward)
(In reply to Adrian Gaudebert [:adrian] from comment #5) Adrian, thank you for looking into this. Did you mean to ni? me or Nick? > However, I don't really want to add yet another field to the default list of > columns on that page. Instead, I would propose that we add an aggregation on > install_time by default in the Aggregations tab. Would that work for you? I > think it would make it even better for you, since you can very very quickly > see if it's a problem that happens on just one install or not. At least speaking for myself -- I would prefer to have it added to the default list for the Reports tab (in other words, as proposed in comment #0). If you add an entry for it in the Aggregations tab then we will need to click on two things; first the Aggregrations tab and then the Crashes tab. In comparison, the comment #0 proposal requires only one click to see all the important data. Many of the crashes are low volume and so it's easy to see just scan the install dates by eye and see if there are duplicates. You could *also* add it in the Aggregations tab, and I think that would be useful, but adding it *only* to the Aggregations tab would not be my preference. Others in the nightly triage rota may have other opinions.
(In reply to Peter Bengtsson [:peterbe] from comment #2) > Defaults or not, I thought the selection was saved in localStorage?! Perhaps > something has regressed. > > I'd favor fixing the localStorage functionality first before discussing what > ought to be the default for everyone. > > Adrian, I can dig into this one if you want. I don't think that ever existed for the Signature report page. That was on the report/list/ page but never was ported as far as I can remember. I think we talked about it when we were discussing our user preferences back-end and said we would wait for that to exist before implementing the feature. You know what happened next. :) I'm going for the option of adding the install time to the Reports tab and the Aggregations tab. If that's bothering other users, we can rework it afterwards.
Created attachment 8844957 [details] [review] Link to Github pull-request: https://github.com/mozilla/socorro/pull/3693
_columns coming from the URL makes total sense now that I think about it. Thank you for fixing this, Adrian!
Commit pushed to master at https://github.com/mozilla/socorro https://github.com/mozilla/socorro/commit/2b551320c35a49ba8cc628630d3122fb1df1f9a4 Fixes bug 1344451 - Added install_time to default columns and aggregations in Signature report. (#3693)
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.