A couple of parameters-on-public-dashboards loose ends #3988
Conversation
…isualization and therefore do not return any promise
…se something needs to be done with it at a later time, and it's the right thing to do anyway
|
@rauchy I think a better approach is to make sure We can return an already resolved promise: load(force, maxAge) {
if (!this.visualization) {
return Promise.resolve();
} |
Totally a better approach. Was just afraid of breaking other usages, but everything seems to work. |
| @@ -44,6 +44,8 @@ const PublicDashboardPage = { | |||
| if (!isSafe) { | |||
| error.errorMessage = 'This query contains potentially unsafe parameters and cannot be executed on a publicly shared dashboard.'; | |||
| } | |||
|
|
|||
| throw error; | |||
There was a problem hiding this comment.
Makes sense but does it have any affect on the flow?
There was a problem hiding this comment.
It doesn't right now, but as a good practice it's best to re-throw. (You never know if someone would want to handle this in the future)
There was a problem hiding this comment.
This won't blow up the dashboard load?
There was a problem hiding this comment.
Nope - gets caught by original promise.
redash/client/app/services/widget.js
Lines 139 to 142 in c3cc65a
* avoid catching errors on text widgets' load(), as they don't have a visualization and therefore do not return any promise * throw error when failing to load widgets on public dashboards - in case something needs to be done with it at a later time, and it's the right thing to do anyway * use Promise.resolve instead of checking for undefined
What type of PR is this? (check all applicable)
Description
Related Tickets & Documents
Follow up for #3659