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
Fix some export links were not working, eg Insights #7133
Conversation
…params that were used to render the datatable. This will not only fix this bug but should also fix potential other bugs where request params were not forwarded. For example an abandonedCarts param in Ecommerce, or hideFutureHoursWhenToday in VisitTime.
Shouldn't request_parameters_to_modify be applied to the parameters so they are modified in the JS (ie, in the dataTableInstance.params var)? Or maybe that's custom_parameters... |
Good point, so we could basically automatically add One problem might be to know which of those custom params should be used for the export link. Eg sometimes Visualizations etc assign custom parameters like |
I'm not exactly sure how this relates to this issue, but my understanding is that request_parameters_to_modify is for the internal request where custom_parameters is for forwarding params to js. Maybe the visualization is doing something strange? |
Not really, as we need to make sure the same request parameters that were used to fetch the datatable in the visualization internally are also used when exporting the datatable. Otherwise the result would not be the same. Eg the |
Perhaps in that case, the parameters should be added to both request_parameters_to_modify and custom_parameters? Assuming that actually works, it seems simpler than changing the DataTable export link code. |
The export link code does not use |
I mean custom_parameters in Visualization is used to set the parameters passed to JavaScript (see Visualization::getClientSideParametersToSet()). In the JavaScript the parameters (from Anyway, I'm not invested in how this issue is solved. |
It wouldn't fix the export link problem since the The only thing I could think of would be to set It would be maybe worth refactoring dataTable.js to have |
FYI, in case it's not clear, I have no issues w/ this pull request being merged. |
I got that. Just tried to understand in case I misunderstood something... would have been nice if there was a better / easier way |
Fix some export links were not working, eg Insights
As you said, angular will provide one ;) |
refs #6790 when exporting a report we need to apply the same request params that were used to render the datatable.
This will not only fix this bug but should also fix potential other bugs
where request params were not forwarded. For example an abandonedCarts
param in Ecommerce, or hideFutureHoursWhenToday in VisitTime.