feat(query): add functions to wrap api calls with typings #555
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/superset/superset-ui/ehq37vtbi |
Codecov Report
@@ Coverage Diff @@
## master #555 +/- ##
==========================================
+ Coverage 23.13% 23.24% +0.11%
==========================================
Files 290 293 +3
Lines 6783 6793 +10
Branches 676 681 +5
==========================================
+ Hits 1569 1579 +10
Misses 5176 5176
Partials 38 38
Continue to review full report at Codecov.
|
| @@ -0,0 +1,28 @@ | |||
| import { SupersetClientInterface, SupersetClient, RequestConfig } from '@superset-ui/connection'; | |||
| import { QueryFormData } from '../../types/QueryFormData'; | |||
There was a problem hiding this comment.
thank god for this custom type... i have mild ptsd from formData lol
|
|
||
| return fetchFunc({ | ||
| ...requestConfig, | ||
| // TODO: Have to transform formData as query string for GET |
There was a problem hiding this comment.
Do you have an example for what a query string could look like?
There was a problem hiding this comment.
There is a utility function in incubator-superset that has to be ported here.
| @@ -0,0 +1,17 @@ | |||
| /* eslint-disable camelcase */ | |||
| export interface V1ChartDataResponseResult { | |||
There was a problem hiding this comment.
I'm not sure about the V1 prefix. Having a prefix makes it look temporary or deprecated. Was ChartDataResponseResult used elsewhere?
There was a problem hiding this comment.
Agree with the doubts about variable names. This was the hardest part about this PR.
I had trouble with naming and went back and forth whether to version or not. Still not fully satisfied TBH. On one hand I want to avoid naming collision if there is v2. On the other hand the V1xxx looks a bit weird.
There was a problem hiding this comment.
I think marking it as v1 is useful in cases where the interface specifically relates to behavior of the v1 api.
There was a problem hiding this comment.
I still don't feel strongly either way so I will merge and adjust later if anybody has more comments.
🏆 Enhancements
Facilitate the calls to
explore_jsonandapi/v1/chart/databy specify required params and provide typescript type for theirresponse. Logics are based on the chartAction.js code inincubator-superset.postChartData()fetchExploreJson()Future plan
incubator-supersetand@superset-ui/chartto use these functions.