Merge pull request #5038 from metabase/dont-add-duplicate-card

Remove the `add` parameter from url after adding a card to dashboard

frontend/src/metabase/containers/AddToDashSelectDashModal.jsx

@@ -45,7 +45,7 @@ export default class AddToDashSelectDashModal extends Component {
 
     addToDashboard = (dashboard: Dashboard) => {
         // we send the user over to the chosen dashboard in edit mode with the current card added
-        this.props.onChangeLocation(Urls.dashboard(dashboard.id)+"?add="+this.props.card.id);
+        this.props.onChangeLocation(Urls.dashboard(dashboard.id, {addCardWithId: this.props.card.id}));
     }
 
     createDashboard = async(newDashboard: Dashboard) => {

frontend/src/metabase/dashboard/containers/DashboardApp.jsx

@@ -16,6 +16,7 @@ import { getUserIsAdmin } from "metabase/selectors/user";
 
 import * as dashboardActions from "../dashboard";
 import {archiveDashboard} from "metabase/dashboards/dashboards"
+import {parseHashOptions} from "metabase/lib/browser";
 
 const mapStateToProps = (state, props) => {
   return {
@@ -34,8 +35,6 @@ const mapStateToProps = (state, props) => {
       editingParameter:     getEditingParameter(state, props),
       parameters:           getParameters(state, props),
       parameterValues:      getParameterValues(state, props),
-      addCardOnLoad:        props.location.query.add ? parseInt(props.location.query.add) : null,
-
       metadata:             getMetadata(state)
   }
 }
@@ -48,10 +47,25 @@ const mapDispatchToProps = {
     onChangeLocation: push
 }
 
+type DashboardAppState = {
+    addCardOnLoad: number|null
+}
+
 @connect(mapStateToProps, mapDispatchToProps)
 @title(({ dashboard }) => dashboard && dashboard.name)
 export default class DashboardApp extends Component {
+    state: DashboardAppState = {
+        addCardOnLoad: null
+    };
+
+    componentWillMount() {
+        let options = parseHashOptions(window.location.hash);
+        if (options.add) {
+            this.setState({addCardOnLoad: parseInt(options.add)})
+        }
+    }
+
     render() {
-        return <Dashboard {...this.props} />;
+        return <Dashboard addCardOnLoad={this.state.addCardOnLoad} {...this.props} />;
     }
 }

frontend/src/metabase/dashboard/hoc/DashboardControls.jsx

@@ -97,8 +97,14 @@ export default (ComposedComponent: ReactClass<any>) =>
                 setValue("refresh", this.state.refreshPeriod);
                 setValue("fullscreen", this.state.isFullscreen);
                 setValue("theme", this.state.isNightMode ? "night" : null);
+
                 delete options.night; // DEPRECATED: options.night
 
+                // Delete the "add card to dashboard" parameter if it's present because we don't
+                // want to add the card again on page refresh. The `add` parameter is already handled in
+                // DashboardApp before this method is called.
+                delete options.add;
+
                 let hash = stringifyHashOptions(options);
                 hash = hash ? "#" + hash : "";
 
@@ -106,7 +112,7 @@ export default (ComposedComponent: ReactClass<any>) =>
                 if (hash !== location.hash) {
                     replace({
                         pathname: location.pathname,
-                        earch: location.search,
+                        search: location.search,
                         hash
                     });
                 }

frontend/src/metabase/lib/urls.js

@@ -23,8 +23,10 @@ export function question(cardId, hash = "", query = "") {
         : `/question${query}${hash}`;
 }
 
-export function dashboard(dashboardId) {
-    return `/dashboard/${dashboardId}`;
+export function dashboard(dashboardId, {addCardWithId} = {}) {
+    return addCardWithId != null
+        ? `/dashboard/${dashboardId}#add=${addCardWithId}`
+        : `/dashboard/${dashboardId}`;
 }
 
 export function modelToUrl(model, modelId) {

frontend/test/e2e/dashboards/dashboards.utils.js

@@ -3,7 +3,6 @@ export const incrementDashboardCount = () => {
     dashboardCount += 1;
 }
 export const getLatestDashboardUrl = () => {
-    console.log(`/dashboard/${dashboardCount}`)
     return `/dashboard/${dashboardCount}`
 }
 export const getPreviousDashboardUrl = (nFromLatest) => {

frontend/test/e2e/parameters/questions.spec.js

@@ -106,22 +106,27 @@ describeE2E("parameters", () => {
             // public url
             await d.get(publicUrl);
             await d::checkScalar(COUNT_ALL);
+            await d.sleep(1000); // making sure that the previous api call has finished
 
             // manually click parameter
             await d::setCategoryParameter("Doohickey");
             await d::checkScalar(COUNT_DOOHICKEY);
+            await d.sleep(1000);
 
             // set parameter via url
             await d.get(publicUrl + "?category=Gadget");
             await d::checkScalar(COUNT_GADGET);
+            await d.sleep(1000);
 
             // embed
             await d.get(embedUrl);
             await d::checkScalar(COUNT_ALL);
+            await d.sleep(1000);
 
             // manually click parameter
             await d::setCategoryParameter("Doohickey");
             await d::checkScalar(COUNT_DOOHICKEY);
+            await d.sleep(1000);
 
             // set parameter via url
             await d.get(embedUrl + "?category=Gadget");

frontend/test/e2e/query_builder/query_builder.spec.js

@@ -56,14 +56,11 @@ describeE2E("query_builder", () => {
             } catch (e) {
             }
 
-            // get the card ID from the URL
-            const cardId = (await d.wd().getCurrentUrl()).match(/\/question\/(\d+)/)[1];
-
             await d.select("#CreateDashboardModal input[name='name']").wait().sendKeys("Main Dashboard");
             await d.select("#CreateDashboardModal .Button.Button--primary").wait().click().waitRemoved(); // wait for the modal to be removed
             incrementDashboardCount();
 
-            await d.waitUrl(getLatestDashboardUrl() + "?add=" + cardId);
+            await d.waitUrl(getLatestDashboardUrl());
 
             // save dashboard
             await d.select(".EditHeader .Button.Button--primary").wait().click();