-
Notifications
You must be signed in to change notification settings - Fork 3
Do not show datasources which have spotter disabled #70
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -510,23 +510,44 @@ describe('thoughtspot-service', () => { | |||||
type: 'WORKSHEET', | ||||||
name: 'Sales Data', | ||||||
id: 'ws1', | ||||||
description: 'Sales information' | ||||||
description: 'Sales information', | ||||||
aiAnswerGenerationDisabled: false | ||||||
} | ||||||
}, | ||||||
{ | ||||||
metadata_header: { | ||||||
type: 'WORKSHEET', | ||||||
name: 'Revenue Data', | ||||||
id: 'ws2', | ||||||
description: 'Revenue information' | ||||||
description: 'Revenue information', | ||||||
aiAnswerGenerationDisabled: false | ||||||
} | ||||||
}, | ||||||
{ | ||||||
metadata_header: { | ||||||
type: 'LOGICAL_TABLE', // This should be filtered out | ||||||
type: 'WORKSHEET', | ||||||
name: 'Revenue Data aiAnswerGenerationDisabled', | ||||||
id: 'ws3', | ||||||
description: 'Revenue information', | ||||||
aiAnswerGenerationDisabled: true | ||||||
} | ||||||
}, | ||||||
{ | ||||||
metadata_header: { | ||||||
type: 'LOGICAL_TABLE', // This should be filtered out due to aiAnswerGenerationDisabled: true | ||||||
name: 'Other Data', | ||||||
id: 'lt1', | ||||||
description: 'Other information' | ||||||
description: 'Other information', | ||||||
aiAnswerGenerationDisabled: true | ||||||
} | ||||||
}, | ||||||
{ | ||||||
metadata_header: { | ||||||
type: 'LOGICAL_TABLE', // This should be filtered out due to aiAnswerGenerationDisabled: true | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment is misleading. It states that this data source should be filtered out, but
Suggested change
|
||||||
name: 'Other Data 2', | ||||||
id: 'lt2', | ||||||
description: 'Other information', | ||||||
aiAnswerGenerationDisabled: false | ||||||
} | ||||||
} | ||||||
]; | ||||||
|
@@ -554,6 +575,11 @@ describe('thoughtspot-service', () => { | |||||
name: 'Revenue Data', | ||||||
id: 'ws2', | ||||||
description: 'Revenue information' | ||||||
}, | ||||||
{ | ||||||
name: 'Other Data 2', | ||||||
id: 'lt2', | ||||||
description: 'Other information' | ||||||
} | ||||||
]); | ||||||
}); | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current filter
d.metadata_header.aiAnswerGenerationDisabled === false
will exclude datasources whereaiAnswerGenerationDisabled
isundefined
ornull
. If the absence of this property implies that AI answer generation is enabled, this filter is too strict and might hide datasources that should be visible.A more robust approach would be to check for explicit disablement, like so:
d.metadata_header.aiAnswerGenerationDisabled !== true
. This ensures that only datasources explicitly marked as disabled are filtered out, while those withfalse
,null
, orundefined
values are correctly included.To ensure this behavior is covered, you could also add a test case in
test/thoughtspot/thoughtspot-service.spec.ts
for a datasource whereaiAnswerGenerationDisabled
is missing.