Refactor disable preview toggle to be only front end (#2067)

* Revert "Add disable preview toggle in settings panel " Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com> * Update RELEASE.md Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com> * Update settings-modal.test.js Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com> * minor fix Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com> * simplify condition Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com> --------- Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
kedro-org · Sep 4, 2024 · 5764d09 · 5764d09
1 parent 023a05b
commit 5764d09
Show file tree

Hide file tree

Showing 14 changed files with 30 additions and 226 deletions.
diff --git a/package/kedro_viz/api/rest/requests.py b/package/kedro_viz/api/rest/requests.py
@@ -10,9 +10,3 @@ class DeployerConfiguration(BaseModel):
     is_all_previews_enabled: bool = False
     endpoint: str
     bucket_name: str
-
-
-class UserPreference(BaseModel):
-    """User preferences for Kedro Viz."""
-
-    showDatasetPreviews: bool
diff --git a/package/kedro_viz/api/rest/router.py b/package/kedro_viz/api/rest/router.py
@@ -7,13 +7,12 @@
 from fastapi import APIRouter
 from fastapi.responses import JSONResponse
 
-from kedro_viz.api.rest.requests import DeployerConfiguration, UserPreference
+from kedro_viz.api.rest.requests import DeployerConfiguration
 from kedro_viz.constants import PACKAGE_REQUIREMENTS
 from kedro_viz.integrations.deployment.deployer_factory import DeployerFactory
 
 from .responses import (
     APIErrorMessage,
-    DataNodeMetadata,
     GraphAPIResponse,
     NodeMetadataAPIResponse,
     PackageCompatibilityAPIResponse,
@@ -50,36 +49,6 @@ async def get_single_node_metadata(node_id: str):
     return get_node_metadata_response(node_id)
 
 
-@router.post("/preferences")
-async def update_preferences(preferences: UserPreference):
-    try:
-        DataNodeMetadata.set_is_all_previews_enabled(preferences.showDatasetPreviews)
-        return JSONResponse(
-            status_code=200, content={"message": "Preferences updated successfully"}
-        )
-    except Exception as exception:
-        logger.error("Failed to update preferences: %s", str(exception))
-        return JSONResponse(
-            status_code=500,
-            content={"message": "Failed to update preferences"},
-        )
-
-
-@router.get("/preferences", response_model=UserPreference)
-async def get_preferences():
-    try:
-        show_dataset_previews = DataNodeMetadata.is_all_previews_enabled
-        return JSONResponse(
-            status_code=200, content={"showDatasetPreviews": show_dataset_previews}
-        )
-    except Exception as exception:
-        logger.error("Failed to fetch preferences: %s", str(exception))
-        return JSONResponse(
-            status_code=500,
-            content={"message": "Failed to fetch preferences"},
-        )
-
-
 @router.get(
     "/pipelines/{registered_pipeline_id}",
     response_model=GraphAPIResponse,

diff --git a/package/tests/test_api/test_rest/test_router.py b/package/tests/test_api/test_rest/test_router.py
@@ -86,45 +86,3 @@ def test_get_package_compatibilities(
 
     assert response.status_code == expected_status_code
     assert response.json() == expected_response
-
-
-def test_update_preferences_success(client, mocker):
-    mocker.patch(
-        "kedro_viz.api.rest.responses.DataNodeMetadata.set_is_all_previews_enabled"
-    )
-    response = client.post("api/preferences", json={"showDatasetPreviews": True})
-
-    assert response.status_code == 200
-    assert response.json() == {"message": "Preferences updated successfully"}
-
-
-def test_update_preferences_failure(client, mocker):
-    mocker.patch(
-        "kedro_viz.api.rest.responses.DataNodeMetadata.set_is_all_previews_enabled",
-        side_effect=Exception("Test Exception"),
-    )
-    response = client.post("api/preferences", json={"showDatasetPreviews": True})
-
-    assert response.status_code == 500
-    assert response.json() == {"message": "Failed to update preferences"}
-
-
-def test_get_preferences_success(client, mocker):
-    mocker.patch(
-        "kedro_viz.api.rest.responses.DataNodeMetadata", is_all_previews_enabled=True
-    )
-    response = client.get("/api/preferences")
-
-    assert response.status_code == 200
-    assert response.json() == {"showDatasetPreviews": True}
-
-
-def test_get_preferences_failure(client, mocker):
-    mocker.patch(
-        "kedro_viz.api.rest.responses.DataNodeMetadata.is_all_previews_enabled",
-        side_effect=Exception("Test Exception"),
-    )
-    response = client.get("/api/preferences")
-
-    assert response.status_code == 500
-    assert response.json() == {"message": "Failed to fetch preferences"}
diff --git a/src/actions/index.js b/src/actions/index.js
@@ -141,6 +141,15 @@ export function toggleShowFeatureHints(showFeatureHints) {
   };
 }
 
+export const TOGGLE_SHOW_DATASET_PREVIEWS = 'TOGGLE_SHOW_DATASET_PREVIEWS';
+
+export function toggleShowDatasetPreviews(showDatasetPreviews) {
+  return {
+    type: TOGGLE_SHOW_DATASET_PREVIEWS,
+    showDatasetPreviews,
+  };
+}
+
 export const TOGGLE_SIDEBAR = 'TOGGLE_SIDEBAR';
 
 /**

diff --git a/src/actions/preferences.js b/src/actions/preferences.js
diff --git a/src/components/metadata/metadata.js b/src/components/metadata/metadata.js
@@ -40,6 +40,7 @@ const MetaData = ({
   theme,
   visible = true,
   visibleCode,
+  showDatasetPreviews,
 }) => {
   const { toSelectedPipeline } = useGeneratePathname();
   const { toExperimentTrackingPath, toMetricsViewPath } =
@@ -57,7 +58,7 @@ const MetaData = ({
   const isDataNode = metadata?.type === 'data';
   const isParametersNode = metadata?.type === 'parameters';
   const nodeTypeIcon = getShortType(metadata?.datasetType, metadata?.type);
-  const hasPreview = metadata?.preview;
+  const hasPreview = showDatasetPreviews && metadata?.preview;
   const hasPlot = hasPreview && metadata?.previewType === 'PlotlyPreview';
   const hasImage = hasPreview && metadata?.previewType === 'ImagePreview';
   const hasTrackingData =
@@ -392,6 +393,7 @@ export const mapStateToProps = (state, ownProps) => ({
   theme: state.theme,
   visible: getVisibleMetaSidebar(state),
   visibleCode: state.visible.code,
+  showDatasetPreviews: state.showDatasetPreviews,
   ...ownProps,
 });
 

diff --git a/src/components/settings-modal/settings-modal.js b/src/components/settings-modal/settings-modal.js
@@ -1,21 +1,17 @@
-import React, { useEffect, useState, useCallback } from 'react';
+import React, { useEffect, useState } from 'react';
 import { connect } from 'react-redux';
 import {
   changeFlag,
   toggleShowFeatureHints,
   toggleIsPrettyName,
   toggleSettingsModal,
+  toggleShowDatasetPreviews,
 } from '../../actions';
-import {
-  getPreferences,
-  updateUserPreferences,
-} from '../../actions/preferences';
 import { getFlagsState } from '../../utils/flags';
 import SettingsModalRow from './settings-modal-row';
 import { settings as settingsConfig, localStorageName } from '../../config';
 import { saveLocalStorage } from '../../store/helpers';
 import { localStorageKeyFeatureHintsStep } from '../../components/feature-hints/feature-hints';
-import { updatePreferences } from '../../utils/preferences-api';
 
 import Button from '../ui/button';
 import Modal from '../ui/modal';
@@ -39,7 +35,6 @@ const SettingsModal = ({
   onToggleIsPrettyName,
   onToggleShowDatasetPreviews,
   showSettingsModal,
-  getPreferences,
   visible,
 }) => {
   const flagData = getFlagsState();
@@ -60,20 +55,6 @@ const SettingsModal = ({
     setShowDatasetPreviewsValue(showDatasetPreviews);
   }, [showDatasetPreviews]);
 
-  useEffect(() => {
-    if (visible.settingsModal) {
-      getPreferences();
-    }
-  }, [visible.settingsModal, getPreferences]);
-
-  const handleSavePreferences = useCallback(async () => {
-    try {
-      await updatePreferences(showDatasetPreviewsValue);
-    } catch (error) {
-      console.error('Error updating preferences:', error);
-    }
-  }, [showDatasetPreviewsValue]);
-
   useEffect(() => {
     let modalTimeout, resetTimeout;
 
@@ -91,10 +72,10 @@ const SettingsModal = ({
           return onToggleFlag(name, value);
         });
 
-        handleSavePreferences();
         onToggleIsPrettyName(isPrettyNameValue);
         onToggleShowFeatureHints(showFeatureHintsValue);
         onToggleShowDatasetPreviews(showDatasetPreviewsValue);
+
         setHasNotInteracted(true);
         setHasClickApplyAndClose(false);
 
@@ -117,7 +98,6 @@ const SettingsModal = ({
     onToggleShowDatasetPreviews,
     showSettingsModal,
     toggleFlags,
-    handleSavePreferences,
   ]);
 
   const resetStateCloseModal = () => {
@@ -253,17 +233,14 @@ export const mapStateToProps = (state) => ({
   flags: state.flags,
   showFeatureHints: state.showFeatureHints,
   isPrettyName: state.isPrettyName,
-  showDatasetPreviews: state.userPreferences.showDatasetPreviews,
+  showDatasetPreviews: state.showDatasetPreviews,
   visible: state.visible,
 });
 
 export const mapDispatchToProps = (dispatch) => ({
   showSettingsModal: (value) => {
     dispatch(toggleSettingsModal(value));
   },
-  getPreferences: () => {
-    dispatch(getPreferences());
-  },
   onToggleFlag: (name, value) => {
     dispatch(changeFlag(name, value));
   },
@@ -274,7 +251,7 @@ export const mapDispatchToProps = (dispatch) => ({
     dispatch(toggleShowFeatureHints(value));
   },
   onToggleShowDatasetPreviews: (value) => {
-    dispatch(updateUserPreferences({ showDatasetPreviews: value }));
+    dispatch(toggleShowDatasetPreviews(value));
   },
 });
 

diff --git a/src/components/settings-modal/settings-modal.test.js b/src/components/settings-modal/settings-modal.test.js
@@ -45,10 +45,10 @@ describe('SettingsModal', () => {
         exportModal: expect.any(Boolean),
         settingsModal: expect.any(Boolean),
       }),
-      showDatasetPreviews: expect.any(Boolean),
       flags: expect.any(Object),
       isPrettyName: expect.any(Boolean),
       showFeatureHints: expect.any(Boolean),
+      showDatasetPreviews: expect.any(Boolean),
     };
     expect(mapStateToProps(mockState.spaceflights)).toEqual(expectedResult);
   });
@@ -77,8 +77,8 @@ describe('SettingsModal', () => {
 
     mapDispatchToProps(dispatch).onToggleShowDatasetPreviews(false);
     expect(dispatch.mock.calls[3][0]).toEqual({
-      type: 'UPDATE_USER_PREFERENCES',
-      payload: { showDatasetPreviews: false },
+      type: 'TOGGLE_SHOW_DATASET_PREVIEWS',
+      showDatasetPreviews: false,
     });
   });
 });
diff --git a/src/reducers/index.js b/src/reducers/index.js
@@ -23,8 +23,8 @@ import {
   UPDATE_ZOOM,
   TOGGLE_EXPAND_ALL_PIPELINES,
   UPDATE_STATE_FROM_OPTIONS,
+  TOGGLE_SHOW_DATASET_PREVIEWS
 } from '../actions';
-import userPreferences from './preferences';
 import { TOGGLE_PARAMETERS_HOVERED } from '../actions';
 
 /**
@@ -82,7 +82,6 @@ const combinedReducer = combineReducers({
   modularPipeline,
   visible,
   runsMetadata,
-  userPreferences,
   // These props don't have any actions associated with them
   display: createReducer(null),
   dataSource: createReducer(null),
@@ -118,6 +117,12 @@ const combinedReducer = combineReducers({
     TOGGLE_EXPAND_ALL_PIPELINES,
     'shouldExpandAllPipelines'
   ),
+  showDatasetPreviews: createReducer(
+    true,
+    TOGGLE_SHOW_DATASET_PREVIEWS,
+    'showDatasetPreviews'
+),
+
 });
 
 const rootReducer = (state, action) => {

diff --git a/src/reducers/preferences.js b/src/reducers/preferences.js
diff --git a/src/reducers/reducers.test.js b/src/reducers/reducers.test.js
@@ -36,7 +36,6 @@ import {
 import { UPDATE_ACTIVE_PIPELINE } from '../actions/pipelines';
 import { TOGGLE_MODULAR_PIPELINE_ACTIVE } from '../actions/modular-pipelines';
 import { TOGGLE_GRAPH_LOADING } from '../actions/graph';
-import { UPDATE_USER_PREFERENCES } from '../actions/preferences';
 
 describe('Reducer', () => {
   it('should return an Object', () => {
@@ -142,19 +141,6 @@ describe('Reducer', () => {
     });
   });
 
-  describe('UPDATE_USER_PREFERENCES', () => {
-    it('should update the value of showDatasetPreviews', () => {
-      const newState = reducer(mockState.spaceflights, {
-        type: UPDATE_USER_PREFERENCES,
-        payload: { showDatasetPreviews: true },
-      });
-      expect(mockState.spaceflights.userPreferences.showDatasetPreviews).toBe(
-        true
-      );
-      expect(newState.userPreferences.showDatasetPreviews).toBe(true);
-    });
-  });
-
   describe('TOGGLE_TEXT_LABELS', () => {
     it('should toggle the value of textLabels', () => {
       const newState = reducer(mockState.spaceflights, {

diff --git a/src/store/initial-state.js b/src/store/initial-state.js
@@ -24,9 +24,7 @@ export const createInitialState = () => ({
   expandAllPipelines: false,
   isPrettyName: settings.isPrettyName.default,
   showFeatureHints: settings.showFeatureHints.default,
-  userPreferences: {
-    showDatasetPreviews: settings.showDatasetPreviews.default,
-  },
+  showDatasetPreviews: settings.showDatasetPreviews.default,
   ignoreLargeWarning: false,
   loading: {
     graph: false,

diff --git a/src/store/store.js b/src/store/store.js
@@ -58,7 +58,7 @@ const saveStateToLocalStorage = (state) => {
     theme: state.theme,
     isPrettyName: state.isPrettyName,
     showFeatureHints: state.showFeatureHints,
-    userPreferences: state.userPreferences,
+    showDatasetPreviews: state.showDatasetPreviews,
     flags: state.flags,
     expandAllPipelines: state.expandAllPipelines,
   });