fix(key-value): lost url_params after long-url feature#18846
Conversation
| value = GetFormDataCommand(parameters).run() | ||
| if value: | ||
| initial_form_data = json.loads(value) | ||
| initial_form_data = json.loads(value) if value else {} |
zhaoyongjie
changed the title
| additionalParam[URL_PARAMS.datasetId.name] = datasetId; | ||
| } | ||
|
|
||
| const urlParams = payload?.url_params || {}; |
There was a problem hiding this comment.
Wouldn't be cleaner to just initialize additionalParam with payload?.url_params? If some of the parameters are already in the URL they will be overridden anyway. This way you don't need the forEach and the if check.
There was a problem hiding this comment.
Wouldn't be cleaner to just initialize additionalParam with payload?.url_params?
formDataKeyin urlParams- it isn't known which url_params don't need to be put into
additionalParamsin the future.
If some of the parameters are already in the URL they will be overridden anyway. This way you don't need the forEach and the if check.
There needs to be a pattern that controls which params can be put in, and which do not.
BTW, chartId and datasetId come from elsewhere even though they are the same as urlParams. (link)
villebro
left a comment
villebro
left a comment
There was a problem hiding this comment.
LGTM, thanks for fixing this!
(cherry picked from commit 4c16586)
|
🏷 preset:2022.7 |
mistercrunch
added
🏷️ bot
5 participants
SUMMARY
Currently, the parameters in the browser can't show in URL. This PR resolved it.
partial fix #16650
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
After
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION