Modify the adding sample data part for timeline#6919
Modify the adding sample data part for timeline#6919zhyuanqi merged 1 commit intoopensearch-project:mainfrom
Conversation
d62d922 to
d8fc98e
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6919 +/- ##
=======================================
Coverage 67.42% 67.43%
=======================================
Files 3443 3443
Lines 67782 67806 +24
Branches 11027 11031 +4
=======================================
+ Hits 45705 45723 +18
- Misses 19412 19417 +5
- Partials 2665 2666 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
4732241 to
dcb93eb
Compare
| const replaceCallback = (match: string, funcName: string, args: string) => { | ||
| if (!args.includes('data_source_name')) { | ||
| args = args.trim(); | ||
| args = `${args}, data_source_name=${dataSourceTitle}`; |
There was a problem hiding this comment.
We should wrap the dataSourceTitle in "" since titles can have spaces (ex: dataSourceTitle="Some DataSource A")
| args = `${args}, data_source_name=${dataSourceTitle}`; | |
| args = `${args}, data_source_name="${dataSourceTitle}"`; |
There was a problem hiding this comment.
And update relevant test cases as well
There was a problem hiding this comment.
Thanks for pointing this out! Did not realize we can have name with space. Would modify and update test case
|
|
||
| const isVegaVisualization = (savedObject: SavedObject) => { | ||
| export const extractTimelineExpression = (savedObject: SavedObject) => { | ||
| if (confirmVisualizationType(savedObject, 'timelion')) { |
There was a problem hiding this comment.
Nit: Logic should be the other way around. We should terminate early and return undefined if the visualization is not timelion
There was a problem hiding this comment.
Sure. Would make the change
0150d0e to
dc94610
Compare
bf4ab28 to
8aa3af9
Compare
| const replaceCallback = (match: string, funcName: string, args: string) => { | ||
| if (!args.includes('data_source_name')) { | ||
| args = args.trim(); | ||
| args = `${args}, data_source_name="${dataSourceTitle}"`; |
There was a problem hiding this comment.
As best practice, we should not mutate the input, can we assign a new variable?
There was a problem hiding this comment.
Sure. Let me change the variable name.
f88aca7 to
df5591d
Compare
Signed-off-by: Yuanqi(Ella) Zhu <zhyuanqi@amazon.com>
Signed-off-by: Yuanqi(Ella) Zhu <zhyuanqi@amazon.com> (cherry picked from commit 48144c8) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Signed-off-by: Yuanqi(Ella) Zhu <zhyuanqi@amazon.com> (cherry picked from commit 48144c8) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
(cherry picked from commit 48144c8) Signed-off-by: Yuanqi(Ella) Zhu <zhyuanqi@amazon.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Description
Modify the adding sample data part for timeline to add data source name if customer has enabled MDS
Issues Resolved
#6123
Screenshot
Screen.Recording.2024-06-05.at.12.26.50.AM.mov
Testing the changes
Please see screenshot
Changelog
Check List
yarn test:jestyarn test:jest_integration