Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include expandAllPipelines flag in initial state #1896

Merged
merged 24 commits into from
May 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
0e7f24d
merge main from remote
ravi-kumar-pilla Apr 25, 2024
c1aae75
Merge branch 'main' of https://github.com/kedro-org/kedro-viz
ravi-kumar-pilla Apr 26, 2024
177ccbc
merging remote
ravi-kumar-pilla May 1, 2024
8ecf9bf
Merge branch 'main' of https://github.com/kedro-org/kedro-viz
ravi-kumar-pilla May 2, 2024
37f3bf4
Merge branch 'main' of https://github.com/kedro-org/kedro-viz
ravi-kumar-pilla May 8, 2024
9715a24
add expandAllPipelines to initial state flags
ravi-kumar-pilla May 9, 2024
c5c7cd9
update file permissions
ravi-kumar-pilla May 9, 2024
894e0a2
fix tests and add release note
ravi-kumar-pilla May 9, 2024
499460c
make expandAllPipelines outside of flags
ravi-kumar-pilla May 14, 2024
fc81126
revert previous change
ravi-kumar-pilla May 14, 2024
fa450c4
merge main
ravi-kumar-pilla May 14, 2024
40a3418
add expandAllPipelines to nonPipelineState as before
ravi-kumar-pilla May 14, 2024
d37c1c9
update comments
ravi-kumar-pilla May 14, 2024
9c43280
testing strawberry fix
ravi-kumar-pilla May 14, 2024
1f5f871
testing strawberry fix
ravi-kumar-pilla May 14, 2024
40480a1
fix strawberry graphql 0.228.0 release doc
ravi-kumar-pilla May 14, 2024
478031a
fix boolean check
ravi-kumar-pilla May 14, 2024
0b5a886
update tests
ravi-kumar-pilla May 14, 2024
ecbba35
Merge branch 'main' into fix/undefined-expandAllFlag
ravi-kumar-pilla May 16, 2024
425c92f
test ci failure
ravi-kumar-pilla May 16, 2024
d17749f
update release note
ravi-kumar-pilla May 16, 2024
9727e3f
merge main
ravi-kumar-pilla May 16, 2024
db43f42
Merge branch 'main' into fix/undefined-expandAllFlag
ravi-kumar-pilla May 17, 2024
597dbbb
Merge branch 'main' into fix/undefined-expandAllFlag
ravi-kumar-pilla May 20, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions RELEASE.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Please follow the established format:
- Refactor backend integration with Kedro by replacing bootstrap_project with configure_project. (#1796)
- Enhance kedro-viz doc integration. (#1874)
- Fix Kedro-Viz waiting for valid Kedro project. (#1871)
- Include expandAllPipelines in initial state. (#1896)
- Enhance Kedro-Viz documentation by using Kedro-sphinx-theme. (#1898)
- Remove default props from functional components. (#1906)
- Fix for schema change in strawberry-graphql JSON scalar. (#1903)
Expand Down
15 changes: 14 additions & 1 deletion src/actions/actions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
TOGGLE_CODE,
TOGGLE_MODULAR_PIPELINE_FOCUS_MODE,
TOGGLE_HOVERED_FOCUS_MODE,
TOGGLE_EXPAND_ALL_PIPELINES,
changeFlag,
resetData,
toggleIgnoreLargeWarning,
Expand All @@ -32,6 +33,7 @@ import {
updateChartSize,
toggleFocusMode,
toggleHoveredFocusMode,
toggleExpandAllPipelines,
} from '../actions';
import {
TOGGLE_NODE_CLICKED,
Expand Down Expand Up @@ -75,7 +77,18 @@ describe('actions', () => {
expect(toggleLayers(visible)).toEqual(expectedAction);
});

it('should create an action to toggle whether to show layers', () => {
it('should create an action to toggle whether to expand all modular pipelines or collapse', () => {
const shouldExpandAllPipelines = false;
const expectedAction = {
type: TOGGLE_EXPAND_ALL_PIPELINES,
shouldExpandAllPipelines,
};
expect(toggleExpandAllPipelines(shouldExpandAllPipelines)).toEqual(
expectedAction
);
});

it('should create an action to toggle whether to show minimap', () => {
const visible = false;
const expectedAction = {
type: TOGGLE_MINIMAP,
Expand Down
13 changes: 13 additions & 0 deletions src/actions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,19 @@ export function toggleLayers(visible) {
};
}

export const TOGGLE_EXPAND_ALL_PIPELINES = 'TOGGLE_EXPAND_ALL_PIPELINES';

/**
* Toggle whether to expand all modular pipelines or collapse
* @param {Boolean} shouldExpandAllPipelines
*/
export function toggleExpandAllPipelines(shouldExpandAllPipelines) {
return {
type: TOGGLE_EXPAND_ALL_PIPELINES,
shouldExpandAllPipelines,
};
}

export const TOGGLE_EXPORT_MODAL = 'TOGGLE_EXPORT_MODAL';

/**
Expand Down
10 changes: 5 additions & 5 deletions src/actions/pipelines.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export function loadInitialPipelineData() {
// obtain the status of expandAllPipelines to decide whether it needs to overwrite the
// list of visible nodes
const expandAllPipelines =
state.display.expandAllPipelines || state.flags.expandAllPipelines;
state.display.expandAllPipelines || state.expandAllPipelines;
let newState = await loadJsonData(url).then((data) =>
preparePipelineState(data, true, expandAllPipelines)
);
Expand All @@ -122,7 +122,7 @@ export function loadInitialPipelineData() {
*/
export function loadPipelineData(pipelineID) {
return async function (dispatch, getState) {
const { dataSource, pipeline, display, flags } = getState();
const { dataSource, pipeline, display, expandAllPipelines } = getState();

if (pipelineID && pipelineID === pipeline.active) {
return;
Expand All @@ -136,10 +136,10 @@ export function loadPipelineData(pipelineID) {
active: pipelineID,
});

const expandAllPipelines =
display.expandAllPipelines || flags.expandAllPipelines;
const shouldExpandAllPipelines =
display.expandAllPipelines || expandAllPipelines;
const newState = await loadJsonData(url).then((data) =>
preparePipelineState(data, false, expandAllPipelines)
preparePipelineState(data, false, shouldExpandAllPipelines)
);

// Set active pipeline here rather than dispatching two separate actions,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
toggleLayers,
toggleSidebar,
toggleTextLabels,
changeFlag,
toggleExpandAllPipelines,
} from '../../actions';
import { loadInitialPipelineData } from '../../actions/pipelines';
import IconButton from '../ui/icon-button';
Expand Down Expand Up @@ -110,7 +110,7 @@ export const mapStateToProps = (state) => ({
textLabels: state.textLabels,
visible: state.visible,
visibleLayers: Boolean(getVisibleLayerIDs(state).length),
expandedPipelines: state.flags.expandAllPipelines,
expandedPipelines: state.expandAllPipelines,
});

export const mapDispatchToProps = (dispatch) => ({
Expand All @@ -127,7 +127,7 @@ export const mapDispatchToProps = (dispatch) => ({
dispatch(toggleTextLabels(Boolean(value)));
},
onToggleExpandAllPipelines: (isExpanded) => {
dispatch(changeFlag('expandAllPipelines', isExpanded));
dispatch(toggleExpandAllPipelines(isExpanded));
dispatch(loadInitialPipelineData());
},
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ describe('PrimaryToolbar', () => {
const expectedResult = {
disableLayerBtn: expect.any(Boolean),
textLabels: expect.any(Boolean),
expandedPipelines: expect.any(Boolean),
displaySidebar: true,
visible: expect.objectContaining({
exportBtn: expect.any(Boolean),
Expand Down Expand Up @@ -127,9 +128,8 @@ describe('PrimaryToolbar', () => {
const dispatch = jest.fn();
mapDispatchToProps(dispatch).onToggleExpandAllPipelines(true);
expect(dispatch.mock.calls[0][0]).toEqual({
name: 'expandAllPipelines',
type: 'CHANGE_FLAG',
value: true,
type: 'TOGGLE_EXPAND_ALL_PIPELINES',
shouldExpandAllPipelines: true,
});
});
});
Expand Down
4 changes: 2 additions & 2 deletions src/components/flowchart-wrapper/flowchart-wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,9 @@ export const FlowChartWrapper = ({
disabledKeys && toSetQueryParam(params.types, mappedDisabledNodes);
}
},
flags: (value) => {
expandAllPipelines: (value) => {
if (!searchParams.has(params.expandAll)) {
toSetQueryParam(params.expandAll, value.expandAllPipelines);
toSetQueryParam(params.expandAll, value);
}
},
};
Expand Down
6 changes: 6 additions & 0 deletions src/reducers/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
TOGGLE_THEME,
UPDATE_CHART_SIZE,
UPDATE_ZOOM,
TOGGLE_EXPAND_ALL_PIPELINES,
} from '../actions';
import { TOGGLE_PARAMETERS_HOVERED } from '../actions';

Expand Down Expand Up @@ -95,6 +96,11 @@ const combinedReducer = combineReducers({
TOGGLE_HOVERED_FOCUS_MODE,
'hoveredFocusMode'
),
expandAllPipelines: createReducer(
false,
TOGGLE_EXPAND_ALL_PIPELINES,
'shouldExpandAllPipelines'
),
});

const rootReducer = (state, action) =>
Expand Down
11 changes: 11 additions & 0 deletions src/reducers/reducers.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
TOGGLE_THEME,
UPDATE_CHART_SIZE,
TOGGLE_HOVERED_FOCUS_MODE,
TOGGLE_EXPAND_ALL_PIPELINES,
} from '../actions';
import {
TOGGLE_NODE_CLICKED,
Expand Down Expand Up @@ -241,6 +242,16 @@ describe('Reducer', () => {
});
});

describe('TOGGLE_EXPAND_ALL_PIPELINES', () => {
it('should toggle whether to expand all modular pipelines or collapse', () => {
const newState = reducer(mockState.spaceflights, {
type: TOGGLE_EXPAND_ALL_PIPELINES,
shouldExpandAllPipelines: true,
});
expect(newState.expandAllPipelines).toEqual(true);
});
});

describe('TOGGLE_SIDEBAR', () => {
it('should toggle whether the sidebar is open', () => {
const newState = reducer(mockState.spaceflights, {
Expand Down
72 changes: 56 additions & 16 deletions src/store/initial-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import deepmerge from 'deepmerge';
import { loadLocalStorage } from './helpers';
import normalizeData from './normalize-data';
import { getFlagsFromUrl, Flags } from '../utils/flags';
import { mapNodeType } from '../utils';
import { mapNodeType, isValidBoolean } from '../utils';
import {
settings,
sidebarWidth,
Expand All @@ -21,6 +21,7 @@ export const createInitialState = () => ({
flags: Flags.defaults(),
textLabels: true,
theme: 'dark',
expandAllPipelines: false,
isPrettyName: settings.isPrettyName.default,
showFeatureHints: settings.showFeatureHints.default,
ignoreLargeWarning: false,
Expand Down Expand Up @@ -67,19 +68,20 @@ const parseUrlParameters = () => {
nodeTagInUrl: search.get(params.tags)
? search.get(params.tags).split(',')
: [],
expandAllPipelinesInUrl: search.get(params.expandAll),
};
};

/**
* Applies URL parameters to the application state.
* This function modifies the state based on URL parameters such as
* Applies URL parameters to the application pipeline state.
* This function modifies the state based on the URL parameters such as
* pipeline ID, node ID, node name, node type presence, and tag presence.
*
* @param {Object} state The current application state.
* @param {Object} state The current application pipeline state.
* @param {Object} urlParams An object containing parsed URL parameters.
* @returns {Object} The new state with modifications applied based on URL parameters.
* @returns {Object} The new state with modifications applied based on the URL parameters.
*/
const applyUrlParametersToState = (state, urlParams) => {
const applyUrlParametersToPipelineState = (state, urlParams) => {
const {
pipelineIdFromURL,
nodeIdFromUrl,
Expand Down Expand Up @@ -130,6 +132,24 @@ const applyUrlParametersToState = (state, urlParams) => {
return newState;
};

/**
* Applies URL parameters to the application non-pipeline state.
* This function modifies the state based on the URL parameters such as
* expandAllPipelines presence.
*
* @param {Object} state The current application non-pipeline state.
* @param {Object} urlParams An object containing parsed URL parameters.
* @returns {Object} The new state with modifications applied based on the URL parameters.
*/
const applyUrlParametersToNonPipelineState = (state, urlParams) => {
const { expandAllPipelinesInUrl } = urlParams;
let newState = { ...state };
if (expandAllPipelinesInUrl && isValidBoolean(expandAllPipelinesInUrl)) {
newState.expandAllPipelines = JSON.parse(expandAllPipelinesInUrl);
}
return newState;
};

/**
* Load values from localStorage and combine with existing state,
* but filter out any unused values from localStorage
Expand All @@ -142,7 +162,7 @@ export const mergeLocalStorage = (state) => {
localStorageRunsMetadata
);
Object.keys(localStorageState).forEach((key) => {
if (!state[key]) {
if (!(key in state)) {
delete localStorageState[key];
}
});
Expand All @@ -162,18 +182,28 @@ export const mergeLocalStorage = (state) => {
* Exactly when it runs depends on whether the data is loaded asynchronously or not.
* @param {Object} data Data prop passed to App component
* @param {Boolean} applyFixes Whether to override initialState
* @param {Boolean} expandAllPipelines Whether to expand all the modular pipelines
* @param {Object} urlParams An object containing parsed URL parameters.
* @returns {Object} The new pipeline state with modifications applied.
*/
export const preparePipelineState = (data, applyFixes, expandAllPipelines) => {
export const preparePipelineState = (
data,
applyFixes,
expandAllPipelines,
urlParams
) => {
let state = mergeLocalStorage(normalizeData(data, expandAllPipelines));
const urlParams = parseUrlParameters();

if (applyFixes) {
// Use main pipeline if active pipeline from localStorage isn't recognised
if (!state.pipeline.ids.includes(state.pipeline.active)) {
state.pipeline.active = state.pipeline.main;
}
}
state = applyUrlParametersToState(state, urlParams);
if (urlParams) {
state = applyUrlParametersToPipelineState(state, urlParams);
}

return state;
};

Expand All @@ -182,17 +212,25 @@ export const preparePipelineState = (data, applyFixes, expandAllPipelines) => {
* will persist if the pipeline data is reset.
* Merge local storage and add custom state overrides from props etc
* @param {object} props Props passed to App component
* @return {object} Updated initial state
* @param {Object} urlParams An object containing parsed URL parameters.
* @returns {Object} The new non-pipeline state with modifications applied.
*/
export const prepareNonPipelineState = (props) => {
const state = mergeLocalStorage(createInitialState());
export const prepareNonPipelineState = (props, urlParams) => {
let state = mergeLocalStorage(createInitialState());
let newVisibleProps = {};

if (props.display?.sidebar === false || state.display.sidebar === false) {
newVisibleProps['sidebar'] = false;
}

if (props.display?.minimap === false || state.display.miniMap === false) {
newVisibleProps['miniMap'] = false;
}

if (urlParams) {
state = applyUrlParametersToNonPipelineState(state, urlParams);
}

return {
...state,
flags: { ...state.flags, ...getFlagsFromUrl() },
Expand All @@ -210,16 +248,18 @@ export const prepareNonPipelineState = (props) => {
* @return {Object} Initial state
*/
const getInitialState = (props = {}) => {
const nonPipelineState = prepareNonPipelineState(props);
const urlParams = parseUrlParameters();
const nonPipelineState = prepareNonPipelineState(props, urlParams);

const expandAllPipelines =
nonPipelineState.display.expandAllPipelines ||
nonPipelineState.flags.expandAllPipelines;
nonPipelineState.expandAllPipelines;

const pipelineState = preparePipelineState(
props.data,
props.data !== 'json',
expandAllPipelines
expandAllPipelines,
urlParams
);

return {
Expand Down
Loading
Loading