Skip to content

Commit

Permalink
Merge branch 'main' into feature/gha-release
Browse files Browse the repository at this point in the history
  • Loading branch information
jitu5 authored May 21, 2024
2 parents d7e1206 + 1e208b3 commit fc73b54
Show file tree
Hide file tree
Showing 15 changed files with 157 additions and 32 deletions.
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

0 comments on commit fc73b54

Please sign in to comment.