New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scheduled PDF Reports: Report Referrer Type shows only keywords but not detailed titles #11323

Closed
mihadd opened this Issue Feb 7, 2017 · 10 comments

Comments

Projects
None yet
4 participants
@mihadd

mihadd commented Feb 7, 2017

The referrer type report shows only keywords like search engines, websites, ... for the entries. It should show detailed titles.
grafik

@mattab mattab added the Bug label Feb 20, 2017

@mattab mattab added this to the 3.0.3 milestone Feb 20, 2017

@sgiehl

This comment has been minimized.

Member

sgiehl commented Mar 19, 2017

This is a general bug that occurs when showing the report "flat"

@sgiehl

This comment has been minimized.

Member

sgiehl commented Mar 19, 2017

Thinking a bit about this actually points out, that flattening this report doesn't really make sense, as it would show the same as the report displaying all referrers. I'll try to disable flattening for this report.

@sgiehl sgiehl assigned sgiehl and unassigned sgiehl Mar 19, 2017

@sgiehl

This comment has been minimized.

Member

sgiehl commented Mar 20, 2017

That won't be as easy as I thought. We currently set the flat parameter for all reports rendered for scheduled reports. This triggers the flattener to flatten the report, even if the report isn't able to handle that in any way.

I guess we need to find a way to define if a report can/should be shown flat in scheduled reports, but I'm not sure right now how to solve this best.

@mattab @tsteur any suggestions?

@tsteur

This comment has been minimized.

Member

tsteur commented Mar 20, 2017

The only way I can think of is to change the API method and remove flatten parameter or to keep API always set it to false in the API method if possible?

Problem is that scheduled reports doesn't consult viewDataTable about which features are enabled etc

@sgiehl

This comment has been minimized.

Member

sgiehl commented Mar 21, 2017

@tsteur The API method doesn't have any flat param.
The DataTablePostProcessor handles this using the request parameter: https://github.com/piwik/piwik/blob/3.x-dev/core/API/DataTablePostProcessor.php#L172
And I didn't find a way to manipulate the request from within the API.

@tsteur

This comment has been minimized.

Member

tsteur commented Mar 21, 2017

I just had a look and it is tricky indeed.

I did make it work with this logic:

        if (Common::getRequestVar('flat', 0, 'int') && $idSubtable === false) {
            $dataTable->queueFilter('GroupBy', array('label'));
        }

but not sure about any side effects. To boost performance it would also need to set expanded=1 when flat is loaded.

Another workaround, and probably better and faster solution is to add a new method to report class like function supportsFlatten() {return true}. I think in the post processor we have the report already anyway and could call this method. Ideally, we would then also in ViewDataTable configure the config property $view->config->show_flatten_table automatically but not sure if that would work or if we'd break something as we would maybe suddenly enable flatten table somewhere where it was disabled before (I don't think so since it is configured after config creation in ViewDataTable).

@tsteur

This comment has been minimized.

Member

tsteur commented Mar 22, 2017

I haven't tested it but there should be an easier solution like

     if (Common::getRequestVar('flat', 0, 'int') && $idSubtable === false) {
            $dataTable->disableFilter('Flatten');
        }

and then we only need to make sure to put logic of postprocessor into a filter like this:

            $flattener = new Flattener($this->apiModule, $this->apiMethod, $this->request);
            if ($this->includeAggregateRows) {
                $flattener->includeAggregateRows();
            }

            $recursiveLabelSeparator = ' - ';
            if ($this->report) {
                $recursiveLabelSeparator = $this->report->getRecursiveLabelSeparator();
            }

            $dataTable = $flattener->flatten($dataTable, $recursiveLabelSeparator);

We are also doing this for other things where we need to disable sort etc and I think this may be the best solution, if we can decide in the API method itself when to disable filter "flatten"

@sgiehl

This comment has been minimized.

Member

sgiehl commented Mar 22, 2017

sounds good. Will give it a try.

@sgiehl

This comment has been minimized.

Member

sgiehl commented Mar 26, 2017

It's not as easy as it seemed to be. The filter method of a DataTable filter gets the datatable as parameter, so it can be used to manipulate it using the objects methods. But it can't be easily replace. But that's what the flattener currently does: creating an empty clone and creating a new table. So we might need to manipulate it in another way. Will check that next week

@mattab

This comment has been minimized.

Member

mattab commented Mar 29, 2017

Referrer type is now not flattened:

referrer type

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment