Skip to content
This repository has been archived by the owner on Aug 9, 2022. It is now read-only.

Fix the selected fields issue in csv report #293

Merged
merged 1 commit into from
Jan 8, 2021

Conversation

zhongnansu
Copy link
Member

Issue #, if available:

selected fields for saved search not showing as expected on csv reports. issues, all the columns are included instead of the selected fields only.

Description of changes:

Root cause

There is a global variable metadata.selectedFields being used in the csv report logic, which doesn't reset when user switches saved searches.

Solution

create scope variable for collecting selected columns and assign to the global variable metadata

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@codecov
Copy link

codecov bot commented Jan 7, 2021

Codecov Report

Merging #293 (2fc4336) into dev (1bf159c) will increase coverage by 0.04%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #293      +/-   ##
==========================================
+ Coverage   77.61%   77.66%   +0.04%     
==========================================
  Files          32       32              
  Lines        1800     1804       +4     
  Branches      356      356              
==========================================
+ Hits         1397     1401       +4     
  Misses        398      398              
  Partials        5        5              
Impacted Files Coverage Δ
...a-reports/server/routes/utils/dataReportHelpers.ts 59.43% <75.00%> (+0.78%) ⬆️
...rts/server/routes/utils/savedSearchReportHelper.ts 98.78% <100.00%> (+0.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1bf159c...2fc4336. Read the comment docs.

Copy link
Member

@dai-chen dai-chen left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! Btw, does the UT for this file still work or not?

@zhongnansu
Copy link
Member Author

Thanks for the fix! Btw, does the UT for this file still work or not?

Yes, the github action passed, which means the tests are fine

Copy link
Contributor

@joshuali925 joshuali925 left a comment

Choose a reason for hiding this comment

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

LGTM

@anirudha anirudha merged commit 3459654 into opendistro-for-elasticsearch:dev Jan 8, 2021
zhongnansu added a commit to zhongnansu/kibana-reports that referenced this pull request Jan 8, 2021
zhongnansu added a commit that referenced this pull request Jan 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants