Merge pull request #944 from plotly/persistence-fix

Persistence fix

Author: alexcjohnson

Diff for: .circleci/config.yml

@@ -108,7 +108,6 @@ jobs:
           name: ️️🏗️ build core
           command: |
               . venv/bin/activate && pip install --no-cache-dir --upgrade -e . --progress-bar off && mkdir packages
-              python setup.py sdist && mv dist/* packages/
               cd dash-renderer && renderer build && python setup.py sdist && mv dist/* ../packages/ && cd ..
               git clone --depth 1 https://github.com/plotly/dash-core-components.git
               cd dash-core-components && npm install --ignore-scripts && npm run build && python setup.py sdist && mv dist/* ../packages/  && cd ..
@@ -200,10 +199,17 @@ jobs:
       - attach_workspace:
           at: ~/dash
       - run:
-          name: 🧪 Run Integration Tests
+          name: ️️🏗️  Install packages
           command: |
             . venv/bin/activate && cd packages && ls -la
-            find . -name "*.gz" | xargs pip install --no-cache-dir --ignore-installed && pip list | grep dash && cd ..
+            find . -name "*.gz" | xargs pip install --no-cache-dir --ignore-installed && cd ..
+            sed -i '/dash/d' requires-install.txt
+            pip install --no-cache-dir --ignore-installed .
+            pip list | grep dash
+      - run:
+          name: 🧪 Run Integration Tests
+          command: |
+            . venv/bin/activate
             TESTFILES=$(circleci tests glob "tests/integration/**/test_*.py" | circleci tests split --split-by=timings)
             pytest --headless --nopercyfinalize --junitxml=test-reports/junit_intg.xml ${TESTFILES}
       - store_artifacts:

Diff for: dash-renderer/CHANGELOG.md

@@ -2,6 +2,10 @@
 All notable changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](http://semver.org/).
 
+# Unreleased
+### Fixed
+- [#944](https://github.com/plotly/dash/pull/944) fixed bug with persistence being toggled on/off on an existing component
+
 ## [1.1.0] - 2019-09-17
 ### Added
 - [#903](https://github.com/plotly/dash/pull/903) enables props edited by the user to persist across recreating the component or reloading the page. Components need to define three new props: `persistence`, `persisted_props`, and `persistence_type` as described in the lead comment of `src/persistence.js`. App developers then enable this behavior by, in the simplest case, setting `persistence: true` on the component. First use case is table, see [dash-table#566](https://github.com/plotly/dash-table/pull/566)

Diff for: dash-renderer/src/persistence.js

@@ -280,38 +280,41 @@ const getValsKey = (id, persistedProp, persistence) =>
     `${id}.${persistedProp}.${JSON.stringify(persistence)}`;
 
 const getProps = layout => {
-    const {props} = layout;
-    const {id, persistence} = props;
-    if (!id || !persistence) {
-        // This component doesn't have persistence. To make downstream
-        // tests more efficient don't return either one, so we just have to
-        // test for truthy persistence.
-        // But we still need to return props for consumers that look for
-        // nested components
+    const {props, type, namespace} = layout;
+    if (!type || !namespace) {
+        // not a real component - just need the props for recursion
         return {props};
     }
+    const {id, persistence} = props;
 
     const element = Registry.resolve(layout);
-    const persisted_props =
-        props.persisted_props || element.defaultProps.persisted_props;
-    const persistence_type =
-        props.persistence_type || element.defaultProps.persistence_type;
-    if (!persisted_props || !persistence_type) {
-        return {props};
-    }
-    return {id, props, element, persistence, persisted_props, persistence_type};
+    const getVal = prop => props[prop] || (element.defaultProps || {})[prop];
+    const persisted_props = getVal('persisted_props');
+    const persistence_type = getVal('persistence_type');
+    const canPersist = id && persisted_props && persistence_type;
+
+    return {
+        canPersist,
+        id,
+        props,
+        element,
+        persistence,
+        persisted_props,
+        persistence_type,
+    };
 };
 
 export function recordUiEdit(layout, newProps, dispatch) {
     const {
+        canPersist,
         id,
         props,
         element,
         persistence,
         persisted_props,
         persistence_type,
     } = getProps(layout);
-    if (!persistence) {
+    if (!canPersist || !persistence) {
         return;
     }
 
@@ -378,6 +381,7 @@ function modProp(key, storage, element, props, persistedProp, update, undo) {
 
 function persistenceMods(layout, component, path, dispatch) {
     const {
+        canPersist,
         id,
         props,
         element,
@@ -387,7 +391,7 @@ function persistenceMods(layout, component, path, dispatch) {
     } = getProps(component);
 
     let layoutOut = layout;
-    if (persistence) {
+    if (canPersist && persistence) {
         const storage = getStore(persistence_type, dispatch);
         const update = {};
         forEach(
@@ -443,6 +447,7 @@ function persistenceMods(layout, component, path, dispatch) {
  */
 export function prunePersistence(layout, newProps, dispatch) {
     const {
+        canPersist,
         id,
         props,
         persistence,
@@ -455,7 +460,7 @@ export function prunePersistence(layout, newProps, dispatch) {
         propName in newProps ? newProps[propName] : prevVal;
     const finalPersistence = getFinal('persistence', persistence);
 
-    if (!persistence && !finalPersistence) {
+    if (!canPersist || !(persistence || finalPersistence)) {
         return newProps;
     }
 
@@ -471,6 +476,8 @@ export function prunePersistence(layout, newProps, dispatch) {
 
     const update = {};
 
+    let depersistedProps = props;
+
     if (persistenceChanged && persistence) {
         // clear previously-applied persistence
         const storage = getStore(persistence_type, dispatch);
@@ -487,6 +494,7 @@ export function prunePersistence(layout, newProps, dispatch) {
                 ),
             filter(notInNewProps, persisted_props)
         );
+        depersistedProps = mergeRight(props, update);
     }
 
     if (finalPersistence) {
@@ -497,10 +505,10 @@ export function prunePersistence(layout, newProps, dispatch) {
             forEach(
                 persistedProp =>
                     modProp(
-                        getValsKey(id, persistedProp, persistence),
+                        getValsKey(id, persistedProp, finalPersistence),
                         finalStorage,
                         element,
-                        props,
+                        depersistedProps,
                         persistedProp,
                         update
                     ),
@@ -516,11 +524,17 @@ export function prunePersistence(layout, newProps, dispatch) {
             if (propTransforms) {
                 for (const propPart in propTransforms) {
                     finalStorage.removeItem(
-                        getValsKey(id, `${propName}.${propPart}`, persistence)
+                        getValsKey(
+                            id,
+                            `${propName}.${propPart}`,
+                            finalPersistence
+                        )
                     );
                 }
             } else {
-                finalStorage.removeItem(getValsKey(id, propName, persistence));
+                finalStorage.removeItem(
+                    getValsKey(id, propName, finalPersistence)
+                );
             }
         }
     }

Diff for: dash/CHANGELOG.md

@@ -1,7 +1,9 @@
 ## Unreleased
 
 ### Added
-
+- [#944](https://github.com/plotly/dash/pull/944)
+  - relevant `dash.testing` methods can now be called with either an element or a CSS selector: `select_dcc_dropdown`, `multiple_click`, `clear_input`, `zoom_in_graph_by_ratio`, `click_at_coord_fractions`.
+  - Three new `dash.testing` methods: `clear_local_storage`, `clear_session_storage`, and `clear_storage` (to clear both together)
 - [#937](https://github.com/plotly/dash/pull/937) `dash.testing` adds two APIs `zoom_in_graph_by_ratio` and `click_at_coord_fractions` about advanced interactions using mouse `ActionChain`
 - [#938](https://github.com/plotly/dash/issues/938) Adds debugging traces to dash backend about serving component suites, so we can use it to verify the installed packages whenever in doubt.
 

Diff for: dash/testing/browser.py

@@ -145,6 +145,11 @@ def find_elements(self, selector):
         """
         return self.driver.find_elements_by_css_selector(selector)
 
+    def _get_element(self, elem_or_selector):
+        if isinstance(elem_or_selector, str):
+            return self.find_element(elem_or_selector)
+        return elem_or_selector
+
     def _wait_for(self, method, args, timeout, msg):
         """abstract generic pattern for explicit WebDriverWait"""
         _wait = (
@@ -262,8 +267,8 @@ def wait_for_page(self, url=None, timeout=10):
                 )
             )
 
-    def select_dcc_dropdown(self, selector, value=None, index=None):
-        dropdown = self.driver.find_element_by_css_selector(selector)
+    def select_dcc_dropdown(self, elem_or_selector, value=None, index=None):
+        dropdown = self._get_element(elem_or_selector)
         dropdown.click()
 
         menu = dropdown.find_element_by_css_selector("div.Select-menu-outer")
@@ -416,13 +421,15 @@ def _get_firefox(self):
     def _is_windows():
         return sys.platform == "win32"
 
-    def multiple_click(self, selector, clicks):
+    def multiple_click(self, elem_or_selector, clicks):
         """multiple_click click the element with number of `clicks`"""
         for _ in range(clicks):
-            self.find_element(selector).click()
+            self._get_element(elem_or_selector).click()
 
-    def clear_input(self, elem):
+    def clear_input(self, elem_or_selector):
         """simulate key press to clear the input"""
+        elem = self._get_element(elem_or_selector)
+
         (
             ActionChains(self.driver)
             .click(elem)
@@ -434,12 +441,18 @@ def clear_input(self, elem):
         ).perform()
 
     def zoom_in_graph_by_ratio(
-        self, elem, start_fraction=0.5, zoom_box_fraction=0.2, compare=True
+        self,
+        elem_or_selector,
+        start_fraction=0.5,
+        zoom_box_fraction=0.2,
+        compare=True
     ):
         """zoom out a graph with a zoom box fraction of component dimension
         default start at middle with a rectangle of 1/5 of the dimension
         use `compare` to control if we check the svg get changed
         """
+        elem = self._get_element(elem_or_selector)
+
         prev = elem.get_attribute("innerHTML")
         w, h = elem.size["width"], elem.size["height"]
         try:
@@ -455,7 +468,9 @@ def zoom_in_graph_by_ratio(
                 "innerHTML"
             ), "SVG content should be different after zoom"
 
-    def click_at_coord_fractions(self, elem, fx, fy):
+    def click_at_coord_fractions(self, elem_or_selector, fx, fy):
+        elem = self._get_element(elem_or_selector)
+
         ActionChains(self.driver).move_to_element_with_offset(
             elem, elem.size["width"] * fx, elem.size["height"] * fy
         ).click().perform()

Diff for: dash/testing/dash_page.py

@@ -49,3 +49,13 @@ def get_session_storage(self, session_id="session"):
                 session_id
             )
         )
+
+    def clear_local_storage(self):
+        self.driver.execute_script("window.localStorage.clear()")
+
+    def clear_session_storage(self):
+        self.driver.execute_script("window.sessionStorage.clear()")
+
+    def clear_storage(self):
+        self.clear_local_storage()
+        self.clear_session_storage()