Add warning to widgets when column is missing #427
Conversation
|
This pull request has been linked to Shortcut Story #234842: Add warning to widgets when used column is not available at source in c4r. |
Pull Request Test Coverage Report for Build 2489152843
💛 - Coveralls |
| this.name = NAME; | ||
| } | ||
|
|
||
| static is(error) { |
There was a problem hiding this comment.
I don't know.
This whole mechanism of detecting type of exception by details of how react-workers pass errors seems fishy.
First of all, this is not "general" check for InvalidColumnError, but only for errors from worker which seem to have "Uncaught ${error.name} ..." injected at beginning of message.
It would be really better to have error codes, and proper error propagation between workers and main thread.
Anyway, i. am worried that all errors throws by "remote" api calls don't have "Uncaught: InvalidColumnError" prefix and thus don't pass this test ... and as result don't trigger warning.
|
|
||
| modelFn.mockRejectedValue( | ||
| new InvalidColumnError('Uncaught InvalidColumnError: Invalid column') | ||
| ); |
There was a problem hiding this comment.
As per comment above, please add test for "new InvalidColumnError("whatever") - those errors will be thrown by "fromRemote" calls as they will go through dealWithApiError error handling
There was a problem hiding this comment.
I've just changed this error to ' new InvalidColumnError('FooBar')' and it fails to show warning.
| static message = | ||
| 'The column selected for this widget is no longer available in the data source.'; | ||
|
|
||
| constructor(message) { |
There was a problem hiding this comment.
My proposition would be to change this to
super(`ERR_INVALID_COLUMN: ${message || default}`)
and in is just test for existence of 'ERR_INVALID_COLUMN: ' in error message.
This is still string test, but at least based on something that looks like error code and is universal and doesn't depend on particular details like adding "Uncaught..." by particular runtime.
|
|
||
| export class InvalidColumnError extends Error { | ||
| static message = | ||
| 'The column selected for this widget is no longer available in the data source.'; |
There was a problem hiding this comment.
Minor: i don't think message copy belongs to "react-core" and exception type, it's purely widgets & UI matter (and it's used only once there) so maybe move it there?
This should be in [useWidgetFetch.js] (or in UI component).
There was a problem hiding this comment.
It used in @carto/react-api also.
There was a problem hiding this comment.
Oh, now I read it correctly. I can move the message to the useWidgetFetch, ok.
…mn-is' of github.com:CartoDB/carto-react into feature/sc-234842/add-warning-to-widgets-when-used-column-is
stats.jsandfeatures.worker.js.