feat(explore): more toast feedback on user actions in Explore (apache…

…#18108) * feat(explore): add toasts feedback when user copies chart url * Show toast message when updating chart properties * Change toast type to success when saving chart * Use success toast from props * Fix tests * Use withToasts instead of dispatch * Use PropertiesModalProps instead of any
nielsen-oss · Feb 8, 2022 · 4d4542c · 4d4542c
1 parent 0545c32
commit 4d4542c
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 16 deletions.
diff --git a/superset-frontend/src/explore/components/ExploreActionButtons.tsx b/superset-frontend/src/explore/components/ExploreActionButtons.tsx
@@ -48,6 +48,7 @@ type ExploreActionButtonsProps = {
   queriesResponse: {};
   slice: { slice_name: string };
   addDangerToast: Function;
+  addSuccessToast: Function;
 };
 
 const VIZ_TYPES_PIVOTABLE = ['pivot_table', 'pivot_table_v2'];
@@ -98,6 +99,7 @@ const ExploreActionButtons = (props: ExploreActionButtonsProps) => {
     latestQueryFormData,
     slice,
     addDangerToast,
+    addSuccessToast,
   } = props;
 
   const copyTooltipText = t('Copy chart URL to clipboard');
@@ -111,8 +113,10 @@ const ExploreActionButtons = (props: ExploreActionButtonsProps) => {
       const shortUrl = await getShortUrl();
       await copyTextToClipboard(shortUrl);
       setCopyTooltip(t('Copied to clipboard!'));
+      addSuccessToast(t('Copied to clipboard!'));
     } catch (error) {
       setCopyTooltip(t('Sorry, your browser does not support copying.'));
+      addDangerToast(t('Sorry, your browser does not support copying.'));
     }
   };
 

diff --git a/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx b/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx
@@ -22,7 +22,7 @@ import { Slice } from 'src/types/Chart';
 import { render, screen, waitFor } from 'spec/helpers/testing-library';
 import fetchMock from 'fetch-mock';
 import userEvent from '@testing-library/user-event';
-import PropertiesModal from '.';
+import PropertiesModal, { PropertiesModalProps } from '.';
 
 const createProps = () => ({
   slice: {
@@ -68,6 +68,7 @@ const createProps = () => ({
   show: true,
   onHide: jest.fn(),
   onSave: jest.fn(),
+  addSuccessToast: jest.fn(),
 });
 
 fetchMock.get('glob:*/api/v1/chart/318', {
@@ -160,10 +161,13 @@ afterAll(() => {
   fetchMock.resetBehavior();
 });
 
+const renderModal = (props: PropertiesModalProps) =>
+  render(<PropertiesModal {...props} />, { useRedux: true });
+
 test('Should render null when show:false', async () => {
   const props = createProps();
   props.show = false;
-  render(<PropertiesModal {...props} />);
+  renderModal(props);
 
   await waitFor(() => {
     expect(
@@ -174,7 +178,7 @@ test('Should render null when show:false', async () => {
 
 test('Should render when show:true', async () => {
   const props = createProps();
-  render(<PropertiesModal {...props} />);
+  renderModal(props);
 
   await waitFor(() => {
     expect(
@@ -185,7 +189,7 @@ test('Should render when show:true', async () => {
 
 test('Should have modal header', async () => {
   const props = createProps();
-  render(<PropertiesModal {...props} />);
+  renderModal(props);
 
   await waitFor(() => {
     expect(screen.getByText('Edit Chart Properties')).toBeVisible();
@@ -196,7 +200,7 @@ test('Should have modal header', async () => {
 
 test('"Close" button should call "onHide"', async () => {
   const props = createProps();
-  render(<PropertiesModal {...props} />);
+  renderModal(props);
 
   await waitFor(() => {
     expect(props.onHide).toBeCalledTimes(0);
@@ -212,7 +216,7 @@ test('"Close" button should call "onHide"', async () => {
 
 test('Should render all elements inside modal', async () => {
   const props = createProps();
-  render(<PropertiesModal {...props} />);
+  renderModal(props);
   await waitFor(() => {
     expect(screen.getAllByRole('textbox')).toHaveLength(5);
     expect(screen.getByRole('combobox')).toBeInTheDocument();
@@ -240,7 +244,7 @@ test('Should render all elements inside modal', async () => {
 
 test('Should have modal footer', async () => {
   const props = createProps();
-  render(<PropertiesModal {...props} />);
+  renderModal(props);
 
   await waitFor(() => {
     expect(screen.getByText('Cancel')).toBeVisible();
@@ -254,7 +258,7 @@ test('Should have modal footer', async () => {
 
 test('"Cancel" button should call "onHide"', async () => {
   const props = createProps();
-  render(<PropertiesModal {...props} />);
+  renderModal(props);
 
   await waitFor(() => {
     expect(props.onHide).toBeCalledTimes(0);
@@ -270,7 +274,7 @@ test('"Cancel" button should call "onHide"', async () => {
 
 test('"Save" button should call only "onSave"', async () => {
   const props = createProps();
-  render(<PropertiesModal {...props} />);
+  renderModal(props);
   await waitFor(() => {
     expect(props.onSave).toBeCalledTimes(0);
     expect(props.onHide).toBeCalledTimes(0);
@@ -294,7 +298,7 @@ test('Empty "Certified by" should clear "Certification details"', async () => {
       certified_by: '',
     },
   };
-  render(<PropertiesModal {...noCertifiedByProps} />);
+  renderModal(noCertifiedByProps);
 
   expect(
     screen.getByRole('textbox', { name: 'Certification details' }),

diff --git a/superset-frontend/src/explore/components/PropertiesModal/index.tsx b/superset-frontend/src/explore/components/PropertiesModal/index.tsx
@@ -26,14 +26,16 @@ import rison from 'rison';
 import { t, SupersetClient, styled } from '@superset-ui/core';
 import Chart, { Slice } from 'src/types/Chart';
 import { getClientErrorObject } from 'src/utils/getClientErrorObject';
+import withToasts from 'src/components/MessageToasts/withToasts';
 
-type PropertiesModalProps = {
+export type PropertiesModalProps = {
   slice: Slice;
   show: boolean;
   onHide: () => void;
   onSave: (chart: Chart) => void;
   permissionsError?: string;
   existingOwners?: SelectValue;
+  addSuccessToast: (msg: string) => void;
 };
 
 const FormItem = Form.Item;
@@ -46,11 +48,12 @@ const StyledHelpBlock = styled.span`
   margin-bottom: 0;
 `;
 
-export default function PropertiesModal({
+function PropertiesModal({
   slice,
   onHide,
   onSave,
   show,
+  addSuccessToast,
 }: PropertiesModalProps) {
   const [submitting, setSubmitting] = useState(false);
   const [form] = Form.useForm();
@@ -157,6 +160,7 @@ export default function PropertiesModal({
         id: slice.slice_id,
       };
       onSave(updatedChart);
+      addSuccessToast(t('Chart properties updated'));
       onHide();
     } catch (res) {
       const clientError = await getClientErrorObject(res);
@@ -308,3 +312,5 @@ export default function PropertiesModal({
     </Modal>
   );
 }
+
+export default withToasts(PropertiesModal);
diff --git a/superset/views/core.py b/superset/views/core.py
@@ -975,11 +975,11 @@ def save_or_overwrite_slice(
         if action == "saveas" and slice_add_perm:
             ChartDAO.save(slc)
             msg = _("Chart [{}] has been saved").format(slc.slice_name)
-            flash(msg, "info")
+            flash(msg, "success")
         elif action == "overwrite" and slice_overwrite_perm:
             ChartDAO.overwrite(slc)
             msg = _("Chart [{}] has been overwritten").format(slc.slice_name)
-            flash(msg, "info")
+            flash(msg, "success")
 
         # Adding slice to a dashboard if requested
         dash: Optional[Dashboard] = None
@@ -1008,7 +1008,7 @@ def save_or_overwrite_slice(
                 _("Chart [{}] was added to dashboard [{}]").format(
                     slc.slice_name, dash.dashboard_title
                 ),
-                "info",
+                "success",
             )
         elif new_dashboard_name:
             # Creating and adding to a new dashboard
@@ -1030,7 +1030,7 @@ def save_or_overwrite_slice(
                 _(
                     "Dashboard [{}] just got created and chart [{}] was added " "to it"
                 ).format(dash.dashboard_title, slc.slice_name),
-                "info",
+                "success",
             )
 
         if dash and slc not in dash.slices: