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
Transition data exporting #18062
Transition data exporting #18062
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally seems to work.
Btw. did you try reusing the report export we already have?
See
matomo/plugins/CoreHome/templates/_dataTableActions.twig
Lines 57 to 62 in b66635e
<a class="dataTableAction activateExportSelection" piwik-report-export | |
report-title="{{ properties.title|e('html_attr') }}" request-params="{{ requestParams|e('html_attr') }}" | |
api-method="{{ properties.apiMethodToRequestDataTable }}" report-formats="{{ formats|json_encode|e('html_attr') }}" | |
href='javascript:;' title="{{ 'General_ExportThisReport'|translate|e('html_attr') }}" | |
max-filter-limit="{{ properties.max_export_filter_limit|e('html_attr') }}" | |
><span class="icon-export"></span></a> |
Guess it would need various adjustments, as it currently depends on a datatable, which we don't have for transitions. Also there are some configuration that would need to be hidden, as they aren't relevant here. But it basically does the same and would also allow to see the export url.
plugins/Transitions/angularjs/transitionexporter/transitionexporter.directive.js
Show resolved
Hide resolved
Use var instead of let Co-authored-by: Stefan Giehl <stefan@matomo.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @bx80 worked nicely. Just left one comment to look at just to be safe re security. Otherwise looks good to merge after addressing that comment 👍 🎉
plugins/Transitions/angularjs/transitionexporter/transitionexporter.directive.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to merge @bx80 I would merge directly but tests are still running. Be good to double check tests are still passing (ignoring any random/unrelated test failures) and then merge 🎉
Description:
Fixes #6032
Visualisations and the standard report exporter expect a single datatable structure, so instead of wrapping the transitions in a visualisation, this change adds a new transition exporter directive which provides an export action on both the main transitions map view and on transitions shown by row actions. This export action then calls the API to return the data in the chosen export format.
Review