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

Change Reports Table Display #291

Conversation

davidcui1225
Copy link
Contributor

Issue #, if available:

Description of changes:

  • Report ID column now displays <ReportSourceType>_<UUID>, e.g Dashboard_abcdefghijk123
  • description column in schema now contains the Report source name to be displayed in the Report source field
  • Search bar in Reports table now prompts user to search by Source in addition to Report ID
  • Updated snapshot tests

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

@zhongnansu
Copy link
Member

Is this 7.9.1 specific feature? Do we need to push any changes to dev?

@codecov
Copy link

codecov bot commented Jan 7, 2021

Codecov Report

Merging #291 (952e706) into 7.9.1 (31249c9) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##            7.9.1     #291   +/-   ##
=======================================
  Coverage   72.85%   72.85%           
=======================================
  Files          32       32           
  Lines        1809     1809           
  Branches      357      357           
=======================================
  Hits         1318     1318           
  Misses        486      486           
  Partials        5        5           
Impacted Files Coverage Δ
...bana-reports/public/components/main/main_utils.tsx 66.32% <ø> (ø)
...a-reports/public/components/main/reports_table.tsx 41.66% <ø> (ø)

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 31249c9...952e706. Read the comment docs.

@davidcui1225
Copy link
Contributor Author

Is this 7.9.1 specific feature? Do we need to push any changes to dev?

No, this issue is not present in dev

reportSource: reportParams.report_name.substring(
0, reportParams.report_name.lastIndexOf('_')
), // remove uuid from report name to display report source
reportSource: reportParams.description, // display report source name
Copy link
Member

Choose a reason for hiding this comment

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

Is this a typo? We are assigning description to the source field

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not a typo- we need a place to store the raw report source name without parsing out any special characters. And the description field is not used anywhere else.

@anirudha anirudha merged commit dc36297 into opendistro-for-elasticsearch:7.9.1 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