[perf logging] Add extra logging for new/editMode dash #9745
Conversation
Codecov Report
@@ Coverage Diff @@
## master #9745 +/- ##
==========================================
+ Coverage 70.74% 70.75% +0.01%
==========================================
Files 585 586 +1
Lines 30429 30435 +6
Branches 3115 3117 +2
==========================================
+ Hits 21526 21534 +8
+ Misses 8789 8787 -2
Partials 114 114
Continue to review full report at Codecov.
|
graceguo-supercat
force-pushed
the
gg-ImproveLogging
branch
from
0e0dec3
to
1b6fc79
Compare
| @@ -0,0 +1,41 @@ | |||
| /** | |||
There was a problem hiding this comment.
could you make this TypeScript since it's a new file?
| @@ -0,0 +1,27 @@ | |||
| /** | |||
There was a problem hiding this comment.
Could you make this TypeScript since it's a new file?
| if (isDashboardEmpty(this.props.layout)) { | ||
| eventData.is_empty = true; | ||
| } |
There was a problem hiding this comment.
why not do eventData.is_empty: isDashboardEmpty(this.props.layout)? Then we could also do this up on line 98 instead too
| const eventData = {}; | ||
| const { dashboardState } = this.props; | ||
| const eventData = { | ||
| is_editMode: dashboardState.editMode, |
There was a problem hiding this comment.
let's either do snake_case or camelCase, not a mix of both
| it('should return true for empty dashboard', () => { | ||
| expect(isDashboardEmpty(emptyLayout)).toBe(true); | ||
| }); | ||
| it('should return false for empty dashboard', () => { |
There was a problem hiding this comment.
nit: should return false for a non-empty dashboard
graceguo-supercat
force-pushed
the
gg-ImproveLogging
branch
2 times, most recently
from
29a53a4
to
8bbff7c
Compare
| export default function isDashboardEmpty(layout: any): boolean { | ||
| // has at least one chart or markdown component | ||
| return !Object.values(layout).some( | ||
| (item: any) => item.type && USER_CONTENT_COMPONENT_TYPE.includes(item.type), |
There was a problem hiding this comment.
not necessary this time, but you could type item as { type?: string } instead of any if you wanted to get a bit more specific
graceguo-supercat
force-pushed
the
gg-ImproveLogging
branch
from
8bbff7c
to
d5b6774
Compare
mistercrunch
added
🏷️ bot
CATEGORY
Choose one
SUMMARY
This PR is to add extra logging data for new and/or edit mode dashboard.
TEST PLAN
Manual test
REVIEWERS
@etr2460