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

Jump to bottom

Ensure extra columns are not shown in table vis when showPartialRows:true #27154

Merged
merged 4 commits into from
Mar 12, 2019
Merged

Ensure extra columns are not shown in table vis when showPartialRows:true #27154

merged 4 commits into from
Mar 12, 2019

Conversation

lukeelmers
Copy link
Member

@lukeelmers lukeelmers commented Dec 13, 2018 •
edited
Loading

Summary

Fixes #22168, but specifically for 7.0, 7.x, 8.0 (master).
Also closes #3455.

The original issue was resolved for 6.6 in #25690 and backported to 6.5.x in #27421.

A different solution is required here since canvas pipeline in visualize has been merged (#25996). This broke the original solution, and therefore requires a different fix for 7.0 and moving forward.

This will also likely conflict with some of the work that's being done in the visualizations field formatting refactoring, currently WIP in #26951. (Edit: Conflicts resolved to work with #28746).

QA

This PR should resolve #22168 for the master branch. Since the issue was already resolved for 6.6 this technically should be introducing no changes for that issue, just a different implementation of the same bugfix. However, this PR additionally resolves #3455, which should be the only difference that's visible to the end user.

@lukeelmers lukeelmers added WIP Work in progress Feature:Data Table Data table visualization feature v7.0.0 labels Dec 13, 2018
@lukeelmers lukeelmers changed the title Fix/table partial rows v7 [WIP 7.0] Ensure extra columns are not shown in table vis when showPartialRows:true Dec 13, 2018
@elasticmachine

This comment has been minimized.

Sign in to view
@lukeelmers lukeelmers force-pushed the fix/table-partial-rows-v7 branch from a0057fa to 47c5f95 Compare January 9, 2019 00:11
@elasticmachine

This comment has been minimized.

Sign in to view
@lukeelmers lukeelmers changed the title [WIP 7.0] Ensure extra columns are not shown in table vis when showPartialRows:true Ensure extra columns are not shown in table vis when showPartialRows:true Jan 9, 2019
@lukeelmers lukeelmers added review Team:Visualizations Visualization editors, elastic-charts and infrastructure and removed WIP Work in progress labels Jan 9, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

@lukeelmers lukeelmers requested review from ppisljar and timroes January 9, 2019 21:58
@elasticmachine

This comment has been minimized.

Sign in to view
@lukeelmers

This comment has been minimized.

Sign in to view
@elastic elastic deleted a comment from elasticmachine Jan 9, 2019
@elastic elastic deleted a comment from elasticmachine Jan 9, 2019
@elastic elastic deleted a comment from elasticmachine Jan 9, 2019
@elastic elastic deleted a comment from elasticmachine Jan 9, 2019
@elastic elastic deleted a comment from elasticmachine Jan 9, 2019
@elasticmachine

This comment has been minimized.

Sign in to view
@lukeelmers

This comment has been minimized.

Sign in to view
@elasticmachine

This comment has been minimized.

Sign in to view
@lukeelmers

This comment has been minimized.

Sign in to view
@elasticmachine
Copy link
Contributor

elasticmachine commented Jan 10, 2019 •
edited by lukeelmers
Loading

💔 Build Failed

Test Result (2 failures / ±0)
UI Functional Tests.test/functional/apps/dashboard/_embeddable_rendering·js.dashboard app using current data dashboard embeddable rendering data rendered correctly when dashboard is opened from listing page
UI Functional Tests.test/functional/apps/dashboard/_dashboard_filtering·js.dashboard app using current data dashboard filtering adding a filter that excludes all data area, bar and heatmap charts filtered

^^ I think these are flaky (both tests are failing on master for me locally too). I believe the embeddable rendering one is being tracked here: #28274, not sure about the filtering one.

@lukeelmers

This comment has been minimized.

Sign in to view
@elasticmachine

This comment has been minimized.

Sign in to view
@lukeelmers

This comment has been minimized.

Sign in to view
@elasticmachine

This comment has been minimized.

Sign in to view
@lukeelmers
Copy link
Member Author

@ppisljar Now that the new pipeline is moved behind a toggle, I went ahead and finished this up based on your last set of feedback, which made the implementation a lot simpler. Tested with the pipeline toggled on & off, and all seems to be working as expected.

@lukeelmers

This comment has been minimized.

Sign in to view
@elastic elastic deleted a comment from elasticmachine Feb 14, 2019
@lukeelmers

This comment has been minimized.

Sign in to view
@elasticmachine

This comment has been minimized.

Sign in to view
@lukeelmers lukeelmers force-pushed the fix/table-partial-rows-v7 branch from 8a4c801 to 01492a7 Compare February 15, 2019 04:04
@elasticmachine

This comment has been minimized.

Sign in to view
@lukeelmers lukeelmers force-pushed the fix/table-partial-rows-v7 branch from 01492a7 to 75fb8ec Compare February 15, 2019 16:05
@elasticmachine

This comment has been minimized.

Sign in to view
@lukeelmers lukeelmers force-pushed the fix/table-partial-rows-v7 branch from 75fb8ec to 8190cad Compare March 4, 2019 22:33
@elasticmachine

This comment has been minimized.

Sign in to view
@lukeelmers lukeelmers force-pushed the fix/table-partial-rows-v7 branch from 8190cad to eaff1c2 Compare March 8, 2019 19:27
@elasticmachine

This comment has been minimized.

Sign in to view
@lukeelmers lukeelmers force-pushed the fix/table-partial-rows-v7 branch from eaff1c2 to 02da7c4 Compare March 11, 2019 20:49
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@lukeelmers lukeelmers merged commit 08d90d0 into elastic:master Mar 12, 2019
@lukeelmers lukeelmers deleted the fix/table-partial-rows-v7 branch March 12, 2019 03:11
@lukeelmers lukeelmers mentioned this pull request Mar 12, 2019
Merged
lukeelmers added a commit to lukeelmers/kibana that referenced this pull request Mar 12, 2019
@lukeelmers
Ensure extra columns are not shown in table vis when showPartialRows:…
b7e7b66 
…true (elastic#27154)
@lukeelmers lukeelmers mentioned this pull request Mar 12, 2019
Merged
lukeelmers added a commit to lukeelmers/kibana that referenced this pull request Mar 12, 2019
@lukeelmers
Ensure extra columns are not shown in table vis when showPartialRows:…
aa8df47 
…true (elastic#27154)
lukeelmers added a commit that referenced this pull request Mar 12, 2019
@lukeelmers
Ensure extra columns are not shown in table vis when showPartialRows:…
44ffe53 
…true (#27154) (#32961)
lukeelmers added a commit that referenced this pull request Mar 12, 2019
@lukeelmers
Ensure extra columns are not shown in table vis when showPartialRows:…
d842fe6 
…true (#27154) (#32962)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ppisljar ppisljar ppisljar approved these changes

@timroes timroes Awaiting requested review from timroes

Assignees
No one assigned
Labels
Feature:Data Table Data table visualization feature review Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.0.0 v7.2.0 v8.0.0
Projects
None yet
Milestone
No milestone
3 participants
@lukeelmers @elasticmachine @ppisljar