Skip to content

Conversation

@nytai
Copy link
Member

@nytai nytai commented Oct 22, 2020

SUMMARY

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 to the user.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen Shot 2020-10-21 at 8 20 08 PM

Screen Shot 2020-10-21 at 8 20 22 PM

TEST PLAN

  • manually tested: entering invalid JSON in json_metadata, editing a dashboard that user does not own.

ADDITIONAL INFORMATION

  • 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

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');
Copy link
Member

@mistercrunch mistercrunch Oct 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be good to refactor that message and similar ones in a constant in a module:

$ 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');

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, let's treat that as a separate task.

@nytai nytai merged commit 2227b13 into apache:master Oct 26, 2020
@nytai nytai deleted the tai/edit-dashboard-errormsg branch October 26, 2020 23:37
auxten pushed a commit to auxten/incubator-superset that referenced this pull request Nov 20, 2020
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.0.0 First shipped in 1.0.0 labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/S 🚢 1.0.0 First shipped in 1.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants