908469 - Show all metadata in the individual report page

Status: RESOLVED FIXED

(Socorro :: Webapp, task)

Description

[Benjamin Smedberg]

Now that we have an API whitelist: can we please include all the public raw-crash items on the report/index page (for all users), and if possible include all metadata for logged-in users?

Comment 2

[(not currently active) Ted Mielczarek]

It would be cool if the default whitelist was really tiny, so that non-Mozilla users didn't have a bunch of useless fields cluttering up their report/index.

Comment 3

[(not currently active) Ted Mielczarek]

Oh, from the bug you just duped: is the whitelist editable from the admin UI, or is it a config setting that needs tweaking? Because that was the main point of that bug: to allow for adding fields to the display without requesting someone to make a code or config change.

Comment 4

[Benjamin Smedberg]

I don't know whether the whitelist is editable from the admin UI currently, but it was discussed at the workweek.

But I don't understand "the default whitelist was really tiny". The whitelist is as big as possible, for the most part only omitting the fields which we don't know about at all or those which have private data.

Comment 5

[Robert Kaiser]

Yes, the whitelist should be as large as possible (and editable from an admin UI in the long run), but the default set of fields shown on report/list should stay the same small amount we have now (but people will be able to add/remove any field from the whitelist there).

Comment 6

[Benjamin Smedberg]

I strongly disagree, which is why I filed the bug. The set of fields on report/list should be all the public metadata.

Comment 7

[Robert Kaiser]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #6)
> I strongly disagree, which is why I filed the bug. The set of fields on
> report/list should be all the public metadata.

Not the default set of columns, and that's what I meant. All fields should be available to put on there though, yes, we agree on that. The default table should not have a hundred columns or so, though. ;-)

Comment 8

[Benjamin Smedberg]

I'm not sure what "available" means to you. If you're saying that we should display a little table with basic facts and hide the other metadata behind a rollup or in another tab, that's ok. If you're saying that we'd still need to turn other fields on using admin UI or patches, then I disagree quite strongly.

Comment 9

[Peter Bengtsson [:peterbe]]

Ted,
The whitelist is currently neither Admin UI or a config setting. It's hardcoded [0]
The plan is to put these into the database so it can be managed with the Admin UI but no timeline yet. 

Kairo, Benjamin, 
We're talking about the report index page here right? Then each field is a row. On the report LIST each field would be a column. That bug is https://bugzilla.mozilla.org/show_bug.cgi?id=889041.
And here's what it would look like when you can include any of the raw crash fields http://cl.ly/RaJc and  http://cl.ly/Rb0U (note the looong vertical scrolling).

The question is; how do you want to display all other raw and processed fields on the report INDEX. E.g. [1]? I would probably suggest a new tab that shows ALL fields from raw crash and processed crash that you are allowed to see. Without an fancy formatting or handcoded labels. 
Then we get a mix of the most interesting fields first but access to EVERYTHING one click away. If we go down that route we can reduce the current table. For example, do we really need EMCheckCompatibility on the main Details table?

[0] https://github.com/mozilla/socorro/blob/master/webapp-django/crashstats/crashstats/models.py#L818-L881
[1] https://crash-stats.mozilla.com/report/index/1e9ad852-c684-4d6f-a162-5863d2130925#details

Comment 10

[Peter Bengtsson [:peterbe]]

By the way, having a dedicated tab for ALL fields on the report index would save us from having to build something where the user can pick and mix which fields she wants to see.

Comment 11

[Robert Kaiser]

(In reply to Peter Bengtsson [:peterbe] from comment #9)
> Kairo, Benjamin, 
> We're talking about the report index page here right? Then each field is a
> row.

Oh, right, I was indeed confusing this with bug 889041.

> I would probably suggest a new tab
> that shows ALL fields from raw crash and processed crash that you are
> allowed to see. Without an fancy formatting or handcoded labels. 

I actually like that idea, makes this much easier to do.

Comment 12

[Benjamin Smedberg]

Yes, we should show all raw and process fields in a new tab: at the same time we can remove a bunch of fields from the main page, leaving only the following fields in the current table:

Signature, UUID, Date Processed, Product, Version, BuildID, Release Channel, OS, Crash Reason, Crash Address, User Comments.

Comment 13

[Robert Kaiser]

I'm not sure about the removals from the main tab, we should think that through carefully. In addition, it's probably cleaner to do the adding of the new tab and the removal on the main one in two different bugs.

Comment 14

[Peter Bengtsson [:peterbe]]

Attachment: Github PR

Comment 15

[Peter Bengtsson [:peterbe]]

QA Steps to "reproduce"

0. NOT logged in
1. Load a crash report. E.g. https://crash-stats.mozilla.com/report/index/079ac874-1e8b-40a8-8d74-2b6452131115
2. There should be 6 "sub tabs". Second one being "Raw Details"
3. Expect nothing sensitive to be shown on that tab like URL or Email address

0. LOGGED in
1. Same as above
2. Same as above
4. Expect to see URL and Email address fields on the Raw Details tab

Comment 16

[[github robot]]

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

https://github.com/mozilla/socorro/commit/affff2c1f3b98a7980c31672773f50e895c5dcb8
fixes bug 908469 - Show all metadata in the individual report page, r=adrian

Comment 17

[Matt Brandt [:mbrandt]]

Attachment: qa_reopened.png

Reopening:
I see discrepancies in the implementation based off of the spec outlined in comment 15 for the logged out view. Also the spacing of the different fields under the "MetaData" tab should be cleaned up to match the "Details" tab for consistency (see screenshot).

not logged in:
0. goto https://crash-stats.allizom.org/report/index/3052455e-f5bc-4f28-9f22-c87232131121

Expected (per comment 15):
- 6 tabs, 2nd tab is "Raw Details

Actual:
- 5 tabs (Details, Metadata, Modules, Raw Dump, Extensions); expected 6
- 2nd tab is labeled "MetaData," not "RawData"
- field under the "MetaData" tab should have the extra spacing removed

Comment 18

[Robert Kaiser]

(In reply to Matt Brandt [:mbrandt] from comment #17)
> - 2nd tab is labeled "MetaData," not "RawData"

That's an intended change per the discussion in the PR.

Comment 19

[[github robot]]

Commits pushed to master at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/d9179ecae04909e6994fbb3d571638eb9792c7f6
fixes bug 908469 - more Metadata tab fixes

https://github.com/mozilla/socorro/commit/3eac28415f7f30c26f58f4ec1ba1f569a139e074
Merge pull request #1688 from peterbe/bug-908469-more-metadata-tab-fixes

fixes bug 908469 - more Metadata tab fixes