feat: add drag and drop for templated variables #1112
Conversation
| return "string" | ||
|
|
||
|
|
||
| def convert_if_legacy_datadoc_meta_v0(datadoc_meta: Dict) -> DataDocMeta: |
There was a problem hiding this comment.
can you add some comments to have some example of what v0 look like?
| if any( | ||
| field_name in fields | ||
| for field_name in ["public", "archived", "owner_uid", "title"] | ||
| ): |
There was a problem hiding this comment.
what does this check for? why are environment_id and meta not in the list?
There was a problem hiding this comment.
environment_id is not changeable, meta is not used in elasticsearch
| type="react-select" | ||
| name={`variables.${index}.type`} | ||
| options={ | ||
| TEMPLATED_VAR_SUPPORTED_TYPES as any as string[] |
There was a problem hiding this comment.
why does it need two as here?
There was a problem hiding this comment.
because typescript doesnt let readonly array be casted as string
| interface IDataDocMetaVariableWithId extends IDataDocMetaVariable { | ||
| id: string; | ||
| } | ||
| const templatedVarUniqueIdPrefix = '_tvar'; |
There was a problem hiding this comment.
do you mean tvar_ to make it like tvar_some_id?
There was a problem hiding this comment.
the exact prefix doesn't matter, it only needs to be unique. Changed to tvar_
| name: '', | ||
| value: '', | ||
| type: 'string', | ||
| id: uniqueId(templatedVarUniqueIdPrefix), |
There was a problem hiding this comment.
this means the name could be not unique? Shall we update the Yup and also the backend validation to not allow duplicate names?
There was a problem hiding this comment.
it could be not unique while the user is writing the form
| @@ -58,6 +59,38 @@ export function openDataDoc(docId: number): ThunkResult<Promise<any>> { | |||
| dataDoc, | |||
| }, | |||
| }); | |||
| } else { | |||
| // Even if it is sameOrigin, dervied fields also needs to be updated | |||
There was a problem hiding this comment.
I dont really get why we need this part for same origin. Wouldn't the !isSameOrigin block be able to update the whole doc with derived fields?
There was a problem hiding this comment.
for sameOrigin, we only need to update meta_variables field. All other fields are already updated before the request locally is sent to server
querybook/server/models/datadoc.py
Outdated
| self._meta = new_meta | ||
|
|
||
| @property | ||
| def meta_variables(self): |
There was a problem hiding this comment.
could you add some comments about why we need it?
| onSave={(meta) => { | ||
| setTemplatedVariables(meta); | ||
| variables={templatedVariables} | ||
| onSave={async (newVariables) => { |
There was a problem hiding this comment.
why async? is setTemplatedVariables async? dont see await is used
There was a problem hiding this comment.
onSave expects a promise, i could either mark the function as async or return an empty promise
| @@ -68,7 +68,9 @@ export class QuerySnippetView extends React.PureComponent< | |||
| if (templatedVariables.length) { | |||
| renderedQuery = await renderTemplatedQuery( | |||
| context, | |||
| this.state.templatedQueryForm, | |||
| Object.entries(this.state.templatedQueryForm).map( | |||
There was a problem hiding this comment.
curious why we need to do renderTemplatedQuery when inserting snippet? shouldn't the variables be kept in the query, insert as is?
There was a problem hiding this comment.
when inserting the snippets, the user needs to fill out the template form
* temp * feat: add drag and drop for templated variables * resolved comments * make templated variables in the frontend array based * meta.variables should always be defined
* temp * feat: add drag and drop for templated variables * resolved comments * make templated variables in the frontend array based * meta.variables should always be defined
Changes:
{ "variables": ["name": "foo", "type": "string", "value": "bar", ...] }.metais moved to.meta_variables, which is dynamically computed from.meta