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

fix(navigation): prevent updating ngrx state when no components received from backend #17673

Merged
merged 5 commits into from
Jul 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,15 @@ describe('Navigation Entry Item Reducer', () => {
const state = fromComponent.reducer(initialState, action);
expect(state).toEqual(mockNodes['testId']);
});

it(`should NOT populate the component state nodes, when no components' details were received`, () => {
const mockPayload = { nodeId: 'testId', components: [] };

const { initialState } = fromComponent;
const action = new CmsActions.LoadCmsNavigationItemsSuccess(mockPayload);
const state = fromComponent.reducer(initialState, action);
expect(state).toEqual(initialState); // check that no nested mutations were done
expect(state).toBe(initialState); // check that the same object reference was returned
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export function reducer(
): NodeItem | undefined {
switch (action.type) {
case CmsActions.LOAD_CMS_NAVIGATION_ITEMS_SUCCESS: {
if (action.payload.components) {
if (action.payload.components.length) {
Zeyber marked this conversation as resolved.
Show resolved Hide resolved
const components = action.payload.components;
const newItem: NodeItem = components.reduce(
(compItems: { [uid_type: string]: any }, component: any) => {
Expand All @@ -24,9 +24,7 @@ export function reducer(
[`${component.uid}_AbstractCMSComponent`]: component,
};
},
{
...{},
}
Comment on lines -27 to -29
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this code cleanup is done by the way. it has nothing to do with the main purpose (bugfix) of this PR

{}
);

return {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* SPDX-FileCopyrightText: 2023 SAP Spartacus team <[email protected]>
*
* SPDX-License-Identifier: Apache-2.0
*/

import { isolateTests } from '../../../support/utils/test-isolation';

context('Cms navigation', { testIsolation: false }, () => {
isolateTests();
it('should not go into infinite loop of loading cms components data, when some details of some component are missing', () => {
/**
* Circuit breaker, just in case if the infinite loop would happen. However, in a successful test pass, infinite loop should not happen.
*/
let circuitBreakerCounter = 20;
function shouldSimulateMissingData(): boolean {
return circuitBreakerCounter-- >= 0;
}

// We want to intercept the HTTP response from `/cms/components` endpoint and simulate missing details of one random CMS component.
// This endpoint is expected to be called a few times, but not infinite number of times.
// Just in case we would go into infinite loop, we have circuit breaker.
cy.intercept('GET', `**/cms/components*`, (req) => {
req.reply((res) => {
if (shouldSimulateMissingData()) {
res.body.component.pop();
}
return res;
});
}).as('getComponents');

cy.visit('/');
cy.wait('@getComponents');
cy.get('cx-category-navigation cx-generic-link').should('exist');

// `/cms/components` endpoint is called for each different Navigation components (e.g. header and footer)
const EXPECTED_MAX_COUNT_LOADING_CMS_COMPONENTS = 4;

// Our heuristic to assert that there is no infinite loop of calling `/cms/components` endpoint
// is to check whether this endpoint was called not too many times during 1st second after the page is rendered.
cy.wait(1_000);
cy.get('@getComponents.all', { timeout: 0 }).should(
'have.length.at.most',
EXPECTED_MAX_COUNT_LOADING_CMS_COMPONENTS
);
});
});
Loading