Opened 2 years ago

Closed 23 months ago

#3073 closed New feature (fixed)

Flattening Tables via API and UI

Reported by: EZdesign Owned by: EZdesign
Priority: major Milestone: 1.12.x - Piwik 1.12.x
Component: Core Keywords:
Cc: Sensitive: no

Description

Provide a mechanism to make nested tables into a single flat table. This will allow users to compare (i.e. sort) rows from different data tables that make up a nested report.

  • Introduce a new API parameter flat=1
  • Add a UI for flattening tables
  • The CSV export does something very similar if expanded=1 is set. Can we reuse the flattener for the CSV export?

Change History (37)

comment:1 Changed 2 years ago by EZdesign (BeezyT)

(In [6124]) refs #3073 data table flattener and test cases

comment:2 Changed 2 years ago by EZdesign (BeezyT)

The previous commit introduces a base class for manipulating data tables. The idea is that the label filter uses it as a base as well.

Please review.

comment:3 Changed 2 years ago by EZdesign (BeezyT)

(In [6125]) refs #3073 svn properties for previous commit

comment:4 Changed 2 years ago by EZdesign (BeezyT)

(In [6126]) refs #3073 proper class name for tests

comment:5 Changed 2 years ago by EZdesign (BeezyT)

There's a number of things that the UI mechanism should provide.

  • The obvious one is to switch flattening on/off.
  • Since there are multiple version of flattening planned, we need a way to select that as well.
  • Also there's the option to include inner nodes or not. We could provide good defaults for including inner node via meta data but people might want different things.

How about a dialog to configure the way the table should be displayed?

We could have some explanation of the different options there to make it easier than more icons at the bottom of the table. Also, the selected behavior from the dialog could influence the way the export is rendered. So most of the stuff we discussed about an export dialog would be valid here as well.

Here' a very rough sketch of what I have in mind:

[ ] Flatten the report

---

( ) Concatene labels

( ) Keep labels and indent

---

( ) Include inner nodes

( ) Only show the leafs

What do you think?

I know this is not the way we usually do things but adding more icons will become confusing at some point. With this dialog, we can easily add more options in the future. Or is there a more simple way for adding the functionality we need?

comment:6 Changed 2 years ago by matt (mattab)

  • Milestone set to 1.7.x - Piwik 1.7.2

Code review:

  • Good work & useful visualization!
  • I think the flatten modification could be implemented as a standard datatable filter.
    • This would avoid adding new directory datatableManipulator
    • It would be consistent to have it as filter
  • in the tests, please generate URLs with a few sub directories
  • It would be very very nice if we could refactor the css code that is similar to this one! Is it possible?

comment:7 Changed 2 years ago by matt (mattab)

Also there's the option to include inner nodes or not. We could provide good defaults for including inner node via meta data but people might want different things.

What would the difference be exactly when this is enabled or disabled?

Since there are multiple version of flattening planned, we need a way to select that as well.

For this, we could for example name each combination, and put a screenshot of what the data will look like when this is ticked. I saw a very good example of such UI in Songbird software. It shows you screenshot of the directory/file organization based on the automatic filename pattern you choose.

  • will there be 4 combinations available?

comment:8 follow-up: Changed 2 years ago by EZdesign (BeezyT)

For what including inner nodes means, see http://dev.piwik.org/trac/ticket/2714#comment:4

In your review, what css code are you talking about?

So should we add a dialog? I would suggest to make it appear between the table and the icons when you click the (leftmost) table icon.

Filters and what I called Manipulators are different (btw: the label filter is a manipulator as well, I want to refactor it into the new directory):

  • Filters mainly change rows and keep the overall structure intact - Manipulators change the entire thing.
  • Filters work on data in memory; the base class loads from Piwik_DataTable_Manager - Manipulators work on archived data; the base class Piwik_API_DataTableManipulator loads them from the API
  • Filters are used in the APIs of plugins - Manipulators are used in ResponseBuilder

Are you sure we should combine those two things? IMO, only loading subtables from different places makes them different enough to keep them separate.

The alternative would be to force expanded=1 and use them in API_DataTableGenericFilter. Then they would work like the other filters. But that's very wasteful for the label filter becuase at the moment, it only loads the subtables from the archive it really needs. That makes a huge difference on reports like the page urls report.

comment:9 in reply to: ↑ 8 Changed 2 years ago by matt (mattab)

Replying to EZdesign:

For what including inner nodes means, see http://dev.piwik.org/trac/ticket/2714#comment:4

Thanks. we have to change the name... How about "keepParents" or "keepExpanded" ?


In your review, what css code are you talking about?

I meant, it would be nice to use this new code in the CSV exporter which does some of the same logic.

So should we add a dialog?

I have one idea among others:

  • on hover of the table, display an icon, outside the datatable, on the bottom right of the table

It's just an idea, not sure how to integrate this easily yet discreetly as it will not be used a lot and cannot add a new icon visible all the time?

I would suggest to make it appear between the table and the icons when you click the (leftmost) table icon.

agreed, rather than popover..

Filters and what I called Manipulators are different (btw: the label filter is a manipulator as well, I want to refactor it into the new directory):

  • Filters mainly change rows and keep the overall structure intact - Manipulators change the entire thing.
  • Filters work on data in memory; the base class loads from Piwik_DataTable_Manager - Manipulators work on archived data; the base class Piwik_API_DataTableManipulator loads them from the API
  • Filters are used in the APIs of plugins - Manipulators are used in ResponseBuilder

Thanks. Can you please write this as a comment in the manipulator?

Are you sure we should combine those two things? IMO, only loading subtables from different places makes them different enough to keep them separate.

OK as long as code is not duplicated a lot between these 2 similar concepts (but they're different so OK)

The alternative would be to force expanded=1 and use them in API_DataTableGenericFilter. Then they would work like the other filters. But that's very wasteful for the label filter becuase at the moment, it only loads the subtables from the archive it really needs. That makes a huge difference on reports like the page urls report.

Can you please elaborate on this problem?

Cheers

comment:10 Changed 2 years ago by EZdesign (BeezyT)

(In [6159]) refs #3073 flatten table UI, option to include aggregate rows (this option will replace includeInnerNodes in the CSV renderer)

comment:11 follow-up: Changed 2 years ago by EZdesign (BeezyT)

I thought long and hard about how we could add these new options to the footer that already is pretty packed. So I added a dialog similar to the selection of how many rows you want to display. I put exclude low population and options for flattening in there. It can easily be extended in the future.

I renamed includeInnerNodes to includeAggregateRows (since you suggested to pick a different name). Is that more clear? When the option is selected, the labels of the aggregate rows are suffixed with [aggregate] to make it clear. The option is only there in the UI if the report is flat.

Matt, you mentioned two kinds of flattening in an email to me. If I understood what you meant correctly, both are implemented now. Please check.

comment:12 Changed 2 years ago by matt (mattab)

UX Review:

  • well done. You found a good solution to a hard problem. I like this implementation and direction.
  • I replaced icon with a more discreet one, but we can definitely find better
  • Please open the submenu on hover, without requiring a click, as this should be easy to access yet not taking place
  • Aggregate rows: Here I think we could find a way to clearly highlight aggregate rows without displaying text
    • Aggregate rows in bold
    • Prepend the plus + logo
    • It should then be clear that the row is aggregate.
      • Maybe we could also left pad the sub rows: this would show the hierarchy.
  • text 'exclude aggregate rows' -> Hide aggregate rows
  • What other options do we plan to include in this window later on?

comment:13 in reply to: ↑ 11 Changed 2 years ago by matt (mattab)

I renamed includeInnerNodes to includeAggregateRows (since you suggested to pick a different name). Is that more clear?

Yes thanks. However there are some "InnerNodes" left in code ini many places can you please change everywhere for consitency?

Matt, you mentioned two kinds of flattening in an email to me. If I understood what you meant correctly, both are implemented now. Please check.

  • That is very cool. How ever the graph don't work with flattened, but it would be very good to be able to plot these tables. It would allow for first time to plot in particular custom variable values, which I know many users are interested in.
  • it should also be fowrarded in all clicks in footer (API, tableGoals, tableAllColumns etc)

comment:14 follow-up: Changed 2 years ago by matt (mattab)

(In [6162]) Refs #3073 New icon more discreet

comment:15 follow-up: Changed 2 years ago by matt (mattab)

One more UX suggestion:

  • when we start recording the full status of each view (exclude low pop, flatten, etc.) to restore the table as they were, I think it will be very important to highlight the fact that the table is currently flattened / or that low populations are excluded (or both).
    • when any option was clicked or initialized in the new menu (btw we need a name for it...), Could you please put the icon in yellow? I think this would at least help users understand a bit why the view is the way it is.
    • it would be also very nice if we could somehow display somewhere "Report is flattened - un-flatten report" or "Rows with few visits are hidden - Show all rows" so it is clear, just by reading the menu, what is the status of the report.

Thoughts?

comment:16 in reply to: ↑ 15 Changed 2 years ago by EZdesign (BeezyT)

Replying to matt:

  • Please open the submenu on hover, without requiring a click, as this should be easy to access yet not taking place

I did it on click because to number of rows selection opens on click too. They look similar and are located next to eachother. Thus, they should work in a similar fashion. So do we do both on hover or both on click? The problem with hover is that the number of rows selection is pretty narrow so it would be easy to mouseout on accident.

  • What other options do we plan to include in this window later on?

I'm sure we will come up with new things ;-)

Replying to matt:

Matt, you mentioned two kinds of flattening in an email to me. If I understood what you meant correctly, both are implemented now. Please check.

  • That is very cool. How ever the graph don't work with flattened, but it would be very good to be able to plot these tables. It would allow for first time to plot in particular custom variable values, which I know many users are interested in.

I don't understand. Can you clarify? Do you mean row evolution? What is special about the custom variables in this context?

Replying to matt:

  • when we start recording the full status of each view (exclude low pop, flatten, etc.) to restore the table as they were, I think it will be very important to highlight the fact that the table is currently flattened / or that low populations are excluded (or both).
    • when any option was clicked or initialized in the new menu (btw we need a name for it...), Could you please put the icon in yellow? I think this would at least help users understand a bit why the view is the way it is.
    • it would be also very nice if we could somehow display somewhere "Report is flattened - un-flatten report" or "Rows with few visits are hidden - Show all rows" so it is clear, just by reading the menu, what is the status of the report.

Maybe we could show the modified state of the table on hover over the icon. So you'd have a highlighted icon on the bottom of the table. You'd hover over it and the tooltip would show "Report is flat. Aggregate rows are excluded". On click, the menu would open and give you the options. This way, you could quickly see what is going on with the table on hover.

comment:17 in reply to: ↑ 14 Changed 2 years ago by EZdesign (BeezyT)

Replying to matt:

(In [6162]) Refs #3073 New icon more discreet

Are you sure this is an improvement? I don't think the faded style fits well with the other icons.

comment:18 Changed 2 years ago by EZdesign (BeezyT)

(In [6181]) refs #2714, refs #3073

  • flat settings are passed to graph views and exports
  • csv renderer does not do flattening anymore. the new api parameters can be used instead - js and tests updated accordingly
  • aggregate rows are italic (imo, the plus logo would be misleading because it would suggest clickability)
  • disable row evolution on flat tables for performance reasons
  • minor language adjustments

comment:19 Changed 2 years ago by EZdesign (BeezyT)

When you flatten the websites report and then display it as a bar chart, you get weird labels like www.example.org/http://example.org/thePage.html. This is because the flattener gets a data table with the full URLs as labels for the second level. It does not happen when the report is displayed as a regular data table. Do you know why?

comment:20 follow-up: Changed 2 years ago by matt (mattab)

Feedback on new commit:

  • so good to see the csv going away :) I'm so glad you wrote these tests... !
  • graph should always request data with include_aggregate_rows = 0 otherwise graphs are not correct, as they include more data than the sum (this is especially true for pie charts but also incorrect to do so in bar charts)
  • in the code I see includeAggregateRows and include_aggregate_rows -- should they not be both includeAggregateRows for consistency?

disable row evolution on flat tables for performance reasons

Is "flat" working OK with row evolution? if it's working but the only problem is performance, I would be very keen to leave the feature in, and we can improve it later (i'm hoping to look into row evolution performance at some point).

I see some very useful use case where you could see the evolution over time of a particular custom variable name-value pair... very meaningful & powerful that would be!

I don't understand. Can you clarify? Do you mean row evolution? What is special about the custom variables in this context?

Sorry, I didn't identify the problem well, you fixed the issue by forwarding the "flat" parameter.

I did it on click because to number of rows selection opens on click too. They look similar and are located next to eachother. Thus, they should work in a similar fashion. So do we do both on hover or both on click? The problem with hover is that the number of rows selection is pretty narrow so it would be easy to mouseout on accident.

My thoughts are that:

  • it is more often used during analysis to "exclude low population" and "flatten table" and is part of the "flow" of analysing data
    • however, changing number of rows is more a one off thing - I think most users would change to what they like and leave it as it is.
  • the "settings" icon is not very visible -- therefore showing the menu on hover on these few pixels will help users discover these features.
    • As opposed to the row selector which is more visible with grey border -- which is good btw, because it displays the number of rows in the table which is useful

You'd hover over it and the tooltip would show "Report is flat. Aggregate rows are excluded". On click, the menu would open and give you the options. This way, you could quickly see what is going on with the table on hover.

Im definitely more keen for a one fit all approach: show on hover the menu that shows both the current status and links to change status ie.

  • "Report is flattened - un-flatten report"
  • "Rows with few visits are hidden - Show all rows"
    • Maybe it could look nice, for example having the "Report is flattened" text in black not linked, then below it have a link with an arrow "→ Flatten report"

Maybe you can try it and if you don't like it we can all try it and discuss it. I think it will make the feature more accessible and generally better :)

Are you sure this is an improvement? I don't think the faded style fits well with the other icons.

I'm not entirely convinced indeed... but I prefer it than the previous one.

  • Because, the feature allows to "update" the current view (keeping the existing "view" tableGoals tableAllColumns etc.).
  • As opposed to other icons on the left, which reload the view and change the columns or load graphs.

However we should keep open mind as there might be better solution ? thoughts?

aggregate rows are italic (imo, the plus logo would be misleading because it would suggest clickability)

I thought initially about using this icon which would be consistent with the Pages report: http://demo.piwik.org/themes/default/images/minus.png ? I think it would look nice and easier to understand.
But then I realized that aggregated rows are not kept together but are sorted by metric. So, the minus icon would be very confusing in this case.

Displaying in italic is definitely an improvement!

When you flatten the websites report and then display it as a bar chart, you get weird labels like www.example.org/http://example.org/thePage.html. This is because the flattener gets a data table with the full URLs as labels for the second level. It does not happen when the report is displayed as a regular data table. Do you know why?

I took a quick 5minutes look and couldn't find the cause. I'll definitely check before the release if you haven't found the bug then.

I'm sure we will come up with new things ;-)

Definitely... I already thought of one: "Expand all rows" #1040 which would be very cool and useful for example for the Downloads/Outlinks reports (usually a few hosts only) or for the page URLs report (on small websites).

comment:21 in reply to: ↑ 20 ; follow-up: Changed 2 years ago by EZdesign (BeezyT)

Replying to matt:

Is "flat" working OK with row evolution? if it's working but the only problem is performance, I would be very keen to leave the feature in, and we can improve it later (i'm hoping to look into row evolution performance at some point).
I see some very useful use case where you could see the evolution over time of a particular custom variable name-value pair... very meaningful & powerful that would be!

Row evoution does work on custom variable name-value pairs. Just open the subtable and click the icon. Using it on flat tables does not add any functionality. It's just orders of magnitue slower. Also, it doesn't work without further code modifications.

comment:22 Changed 2 years ago by EZdesign (BeezyT)

Regarding the icon, take a look at this: http://www.iconfinder.com/search/?q=iconset%3Asilk2+cog

We could show the first one (maybe faded out a little) by default. On hover, the menu opens and it's shown opaquely. When low population is excluded or the report is flat, we can show one of the other icons, maybe the one with the small yellow (!) sign?

comment:23 Changed 2 years ago by EZdesign (BeezyT)

(In [6182]) refs #3073

  • graph views of data tables always request data without aggregate rows
  • csv export js fix: includeAggregateRows => include_aggregate_rows
  • the config menu is more verbose now (added new lang strings rather than reusing existing ones for low population - this way translators will notice the change)
  • refactoring to reduce duplication: label filter becomes a data table manipulator
  • integration test for include_aggregate_rows parameter

comment:24 Changed 2 years ago by EZdesign (BeezyT)

(In [6183]) refs #3073

  • data table manipulator applies queued filters. this solves the issue with displaying the websites report as a bar/pie chart.
  • temporarily added removed lang strings again. will be removed once we agreed to do so.
  • fixed bug introduced by label filter refactoring.

comment:25 Changed 2 years ago by EZdesign (BeezyT)

(In [6184]) refs #3073 svn properties for new file

comment:26 Changed 2 years ago by EZdesign (BeezyT)

(In [6185]) refs #3073: extended test suite FlattenReports

comment:27 in reply to: ↑ 21 Changed 2 years ago by matt (mattab)

KABOOM! it looks beautiful!!

Row evoution does work on custom variable name-value pairs. Just open the subtable and click the icon. Using it on flat tables does not add any functionality. It's just orders of magnitue slower. Also, it doesn't work without further code modifications.

That makes sense. Perfect as it is.

Regarding the icon, take a look at this: http://www.iconfinder.com/search/?q=iconset%3Asilk2+cog
We could show the first one (maybe faded out a little) by default. On hover, the menu opens and it's shown opaquely. When low population is excluded or the report is flat, we can show one of the other icons, maybe the one with the small yellow (!) sign?

First one looks good (agreed on fading out as it's too dark).

for the "enabled state", the (!) wouldn't work as it triggers a "warning" feeling. I would be more inclined to have a "highlight" effect (for example yellow hallow around the icon, maybe needs a new icon image) that shows that something is enabled, but should not distract the user, but I'm not sure what would work nice...
thoughts?

worst case scenario we could use: http://www.iconfinder.com/icondetails/9568/22/cog_face_female_gearhead_icon

Feedback:

  • Could you please append the string "(default)" after each blue link, when the feature currently enabled is not the default case?
    • For example "Make it hierarchical (default)"
    • this would reduce user frustration as to losing track of what the repotr should look like. because, we make sure the default report looks the best it should "by default" so it's nice to remind it.
  • OK to remove translation strings, the new UI makes the feature much easier to understand.

comment:28 Changed 2 years ago by EZdesign (BeezyT)

(In [6191]) refs #3073 new icon, highlighted icon when the configuration differs from default, showing (default) in options

comment:29 Changed 2 years ago by EZdesign (BeezyT)

(In [6192]) refs #3073 removing left over lang string

comment:30 Changed 2 years ago by EZdesign (BeezyT)

(In [6194]) refs #3073: cross-browser ie789, safari, chrome, firefox

comment:31 Changed 2 years ago by EZdesign (BeezyT)

My to do list for this feature is empty now.

Are there any more suggestions? If not, we can close the ticket.

comment:32 Changed 2 years ago by matt (mattab)

Well done, Looks good to me :)

comment:33 Changed 2 years ago by EZdesign (BeezyT)

  • Resolution set to fixed
  • Status changed from new to closed

comment:34 Changed 2 years ago by SteveG (sgiehl)

(In [6242]) refs #3073 save new options in dashboard, so that they are restored

comment:35 Changed 23 months ago by matt (mattab)

  • Priority changed from normal to major

comment:36 Changed 23 months ago by matt (mattab)

  • Resolution fixed deleted
  • Status changed from closed to reopened

Small bug noticed: the cog icon should appear on the graph, since the graph plotting flattened custom variables is very useful in some cases, but better have a way to disable the flattening.

comment:37 Changed 23 months ago by matt (mattab)

  • Resolution set to fixed
  • Status changed from reopened to closed

Fixed in [6424]

Note: See TracTickets for help on using tickets.