You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/10/22 03:26:45 UTC
[GitHub] [incubator-superset] nytai opened a new pull request #11382: fix: better error messages for dashboard properties modal
nytai opened a new pull request #11382:
URL: https://github.com/apache/incubator-superset/pull/11382
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
The dashboard properties modal raises a generic error message for most cases. Since the api already sends back status codes and validation errors this PR simply exposes them.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
<img width="1025" alt="Screen Shot 2020-10-21 at 8 20 08 PM" src="https://user-images.githubusercontent.com/10255196/96820672-51701e80-13db-11eb-8a9c-ef85081e651b.png">
<img width="955" alt="Screen Shot 2020-10-21 at 8 20 22 PM" src="https://user-images.githubusercontent.com/10255196/96820676-52a14b80-13db-11eb-8e96-c247253f8baa.png">
### TEST PLAN
<!--- What steps should be taken to verify the changes -->
- manually tested: entering invalid JSON in `json_metadata`, editing a dashboard that user does not own.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] willbarrett commented on a change in pull request #11382: fix: better error messages for dashboard properties modal
Posted by GitBox <gi...@apache.org>.
willbarrett commented on a change in pull request #11382:
URL: https://github.com/apache/incubator-superset/pull/11382#discussion_r512322701
##########
File path: superset-frontend/src/dashboard/components/PropertiesModal.jsx
##########
@@ -165,13 +165,25 @@ class PropertiesModal extends React.PureComponent {
}
async handleErrorResponse(response) {
- const { error, statusText } = await getClientErrorObject(response);
+ const { error, statusText, message } = await getClientErrorObject(response);
+ let errorText = error || statusText || t('An error has occurred');
Review comment:
Agreed, let's treat that as a separate task.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] mistercrunch commented on a change in pull request #11382: fix: better error messages for dashboard properties modal
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #11382:
URL: https://github.com/apache/incubator-superset/pull/11382#discussion_r510606484
##########
File path: superset-frontend/src/dashboard/components/PropertiesModal.jsx
##########
@@ -165,13 +165,25 @@ class PropertiesModal extends React.PureComponent {
}
async handleErrorResponse(response) {
- const { error, statusText } = await getClientErrorObject(response);
+ const { error, statusText, message } = await getClientErrorObject(response);
+ let errorText = error || statusText || t('An error has occurred');
Review comment:
Could be good to refactor that message and similar ones in a constant in a module:
```bash
$ git grep -i "error.*occurred'" superset-frontend/
superset-frontend/src/SqlLab/components/ExploreCtasResultsButton.jsx: this.props.errorMessage || t('An error occurred'),
superset-frontend/src/SqlLab/components/ExploreResultsButton.jsx: this.props.errorMessage || t('An error occurred'),
superset-frontend/src/components/TableLoader.tsx: props.addDangerToast(t('An error occurred'));
superset-frontend/src/dashboard/components/PropertiesModal.jsx: body: error || statusText || t('An error has occurred'),
superset-frontend/src/datasource/DatasourceEditor.jsx: error || statusText || t('An error has occurred'),
superset-frontend/src/datasource/DatasourceModal.tsx: body: error || t('An error has occurred'),
superset-frontend/src/explore/components/DisplayQueryButton.jsx: error: error || statusText || t('Sorry, An error occurred'),
superset-frontend/src/explore/components/PropertiesModal.tsx: body: error || statusText || t('An error has occurred'),
superset-frontend/src/utils/getClientErrorObject.ts: : t('An error occurred');
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] nytai merged pull request #11382: fix: better error messages for dashboard properties modal
Posted by GitBox <gi...@apache.org>.
nytai merged pull request #11382:
URL: https://github.com/apache/incubator-superset/pull/11382
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] mistercrunch commented on a change in pull request #11382: fix: better error messages for dashboard properties modal
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #11382:
URL: https://github.com/apache/incubator-superset/pull/11382#discussion_r510606484
##########
File path: superset-frontend/src/dashboard/components/PropertiesModal.jsx
##########
@@ -165,13 +165,25 @@ class PropertiesModal extends React.PureComponent {
}
async handleErrorResponse(response) {
- const { error, statusText } = await getClientErrorObject(response);
+ const { error, statusText, message } = await getClientErrorObject(response);
+ let errorText = error || statusText || t('An error has occurred');
Review comment:
Could be good to refactor that message and similar ones in a constant:
```bash
$ git grep -i "error.*occurred'" superset-frontend/
superset-frontend/src/SqlLab/components/ExploreCtasResultsButton.jsx: this.props.errorMessage || t('An error occurred'),
superset-frontend/src/SqlLab/components/ExploreResultsButton.jsx: this.props.errorMessage || t('An error occurred'),
superset-frontend/src/components/TableLoader.tsx: props.addDangerToast(t('An error occurred'));
superset-frontend/src/dashboard/components/PropertiesModal.jsx: body: error || statusText || t('An error has occurred'),
superset-frontend/src/datasource/DatasourceEditor.jsx: error || statusText || t('An error has occurred'),
superset-frontend/src/datasource/DatasourceModal.tsx: body: error || t('An error has occurred'),
superset-frontend/src/explore/components/DisplayQueryButton.jsx: error: error || statusText || t('Sorry, An error occurred'),
superset-frontend/src/explore/components/PropertiesModal.tsx: body: error || statusText || t('An error has occurred'),
superset-frontend/src/utils/getClientErrorObject.ts: : t('An error occurred');
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org