Use question metadata for generateQueryDescription
#29331
Conversation
| it("should work with multiple aggregations", () => { | ||
| const question = base_question.setDatasetQuery({ | ||
| query: { | ||
| "source-table": ORDERS.id, | ||
| aggregation: [["count"], ["sum", ["field", 1, null]]], | ||
| aggregation: [["count"], ["sum", ["field", 6, null]]], | ||
| }, | ||
| }); | ||
| expect(question.generateQueryDescription(mockTableMetadata)).toEqual( | ||
| expect(question.generateQueryDescription()).toEqual( | ||
| "Orders, Count and Sum of Total", | ||
| ); |
There was a problem hiding this comment.
base_question uses metadata from the sample database fixture. Field 1 there is actually "Created At", so changing the ID to 6 to correctly reference the "Total" column
|
You have successfully added a new CodeQL configuration |
kulyk
changed the title
generateQueryDescriptiongenerateQueryDescription
| // DEPRECATED: replaced with `query` | ||
| tableMetadata: PropTypes.object, | ||
| datasetQuery: PropTypes.object, | ||
|
|
There was a problem hiding this comment.
Not used directly. Also there's no props spread, so child components don't seem to need them either
| @@ -122,7 +121,6 @@ const mapStateToProps = (state, props) => { | |||
| databases: getDatabasesList(state), | |||
| nativeDatabases: getNativeDatabases(state), | |||
| tables: getTables(state), | |||
| tableMetadata: getTableMetadata(state), | |||
There was a problem hiding this comment.
SaveQuestionModal seems to be the only component that actually needed this
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## master #29331 +/- ##
==========================================
- Coverage 68.49% 68.48% -0.01%
==========================================
Files 2790 2790
Lines 96508 96498 -10
Branches 12286 12286
==========================================
- Hits 66101 66090 -11
+ Misses 25204 25203 -1
- Partials 5203 5205 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
|
No failed tests 🎉 |
kulyk
force-pushed
the
anton/MLv2-query-descriptions-metadata
branch
from
360b5f1
to
ee44d85
Compare
kulyk
force-pushed
the
anton/MLv2-query-descriptions-metadata
branch
from
ee44d85
to
24907f0
Compare
Patch for #29200
It seems like
generateQueryDescriptionshould be free to use its internalMetadatainstance. We used to generate these descriptions in a separate function, so it made sense to pass it externally. But now we're calling the method for a question we'd like to get a description for. At that point, the question should have all the necessary metadata loaded.The PR also removes the
tableMetadatafrom a few query builder components. It seems like it was added toQueryBuilder'smapStateToPropsonly to be propagated down to theSaveQuestionModal.