Skip to content
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.

Sign up for GitHub

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

[Visualize] Fix export table for table export links #71249

Merged
merged 7 commits into from
Jul 14, 2020
Merged

[Visualize] Fix export table for table export links #71249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3b80c01
:bug: Fix export name for the csv file to match table name
dej611 Jul 9, 2020
8940633
:camera_flash: Update snapshot for table_vis_fn test
dej611 Jul 9, 2020
2db3c10
Merge branch 'master' into fix/csv-export-filename
elasticmachine Jul 9, 2020
c7a846a
Merge branch 'master' into fix/csv-export-filename
elasticmachine Jul 13, 2020
deea1c8
:camera_flash: Add title to test and updated snapshot
dej611 Jul 14, 2020
60b02e3
:bug: Pick the title from the params rather than the vis
dej611 Jul 14, 2020
8f6047e
:fire: Remove unused props
dej611 Jul 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/plugins/vis_type_table/public/__snapshots__/table_vis_fn.test.ts.snap
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/plugins/vis_type_table/public/agg_table/agg_table.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export function KbnAggTable(config, RecursionHelper) {
return;
}

self.csv.filename = (exportTitle || table.title || 'table') + '.csv';
self.csv.filename = (exportTitle || table.title || 'unsaved') + '.csv';
$scope.rows = table.rows;
$scope.formattedColumns = [];

Expand Down
1 change: 1 addition & 0 deletions src/plugins/vis_type_table/public/table_vis_fn.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ describe('interpreter/functions#table', () => {
columns: [{ id: 'col-0-1', name: 'Count' }],
};
const visConfig = {
title: 'My Chart title',
perPage: 10,
showPartialRows: false,
showMetricsAtAllLevels: false,
Expand Down
12 changes: 11 additions & 1 deletion src/plugins/vis_type_table/public/vis_controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,18 @@ export function getTableVisualizationControllerClass(
if (!this.$scope) {
return;
}

// How things get into this $scope?
// To inject variables into this $scope there's the following pipeline of stuff to check:
// - visualize_embeddable => that's what the editor creates to wrap this Angular component
// - build_pipeline => it serialize all the params into an Angular template compiled on the fly
// - table_vis_fn => unserialize the params and prepare them for the final React/Angular bridge
// - visualization_renderer => creates the wrapper component for this controller and passes the params
//
// In case some prop is missing check into the top of the chain if they are available and check
// the list above that it is passing through
this.$scope.vis = this.vis;
this.$scope.visState = { params: visParams };
this.$scope.visState = { params: visParams, title: visParams.title };
this.$scope.esResponse = esResponse;

this.$scope.visParams = visParams;
Expand Down
1 change: 1 addition & 0 deletions src/plugins/visualizations/public/expressions/visualization_renderer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export const visualization = () => ({
const visType = config.visType || visConfig.type;

const vis = new ExprVis({
title: config.title,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to be unnecessary line

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be unnecessary for this case, but ExprVis has a title prop already configured, the title position was not in the right location so I fixed while there.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, but the config doesn't have a title in any kind of visualization.
The config is the result of fn of a visualization (in case of table vis that's the result of table_vis_fn.ts -> fn function).
If you check configs of other visualizations, you'll see that any config doesn't have a title.
So this line do nothing

type: visType as string,
params: visConfig as VisParams,
});
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/visualizations/public/legacy/build_pipeline.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ export const buildPipeline = async (
const { indexPattern, searchSource } = vis.data;
const query = searchSource!.getField('query');
const filters = searchSource!.getField('filter');
const { uiState } = vis;
const { uiState, title } = vis;

// context
let pipeline = `kibana | kibana_context `;
Expand Down Expand Up @@ -519,7 +519,7 @@ export const buildPipeline = async (
timefilter: params.timefilter,
});
if (buildPipelineVisFunction[vis.type.name]) {
pipeline += buildPipelineVisFunction[vis.type.name](vis.params, schemas, uiState);
pipeline += buildPipelineVisFunction[vis.type.name]({ title, ...vis.params }, schemas, uiState);
} else if (vislibCharts.includes(vis.type.name)) {
const visConfig = { ...vis.params };
visConfig.dimensions = await buildVislibDimensions(vis, params);
Expand Down