Skip to content

Commit

Permalink
fix: [M3-7249] - Linode Landing flickering (#9836)
Browse files Browse the repository at this point in the history
* dont block children from rendering

* Added changeset: Linodes Landing flickering

* clean up more

* fix preference toggle

* comment and clean up types

* remove ambiguous test

* show children after inital fetches

* fix AuthenticationWrapper logic and test

* revert change

* removed unused state

* fix lish

* fix AuthenticationWrapper more

* fix flags flickering

* add some e2e coverage

* Update packages/manager/src/IdentifyUser.tsx

Co-authored-by: Dajahi Wiley <114682940+dwiley-akamai@users.noreply.github.com>

---------

Co-authored-by: Banks Nussman <banks@nussman.us>
Co-authored-by: Dajahi Wiley <114682940+dwiley-akamai@users.noreply.github.com>
  • Loading branch information
3 people authored Oct 25, 2023
1 parent 814dafb commit f26f788
Show file tree
Hide file tree
Showing 9 changed files with 82 additions and 239 deletions.
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-9836-fixed-1698158641264.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Fixed
---

Linodes Landing flickering ([#9836](https://github.com/linode/manager/pull/9836))
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { apiMatcher } from 'support/util/intercepts';

describe('account activation', () => {
/**
* The API will return 403 with the body below for most endpoint except `/v4/profile`.
*
* { "errors": [ { "reason": "Your account must be activated before you can use this endpoint" } ] }
*/
it('should render an activation landing page if the customer is not activated', () => {
cy.intercept('GET', apiMatcher('*'), {
statusCode: 403,
body: {
errors: [
{
reason:
'Your account must be activated before you can use this endpoint',
},
],
},
});

cy.visitWithLogin('/');

cy.findByText('Your account is currently being reviewed.');
cy.findByText('open a support ticket', { exact: false });
});
});
8 changes: 7 additions & 1 deletion packages/manager/src/IdentifyUser.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,15 @@ export const IdentifyUser = () => {
// in maintenance mode, we can't fetch the user's username.
// Therefore, the `if` above won't "setFeatureFlagsLoaded".

// We also need to make sure client is defined. Launch Darkly has a weird API.
// If client is undefined, that means flags are loading. Even if flags fail to load,
// client will become defined and we and allow the app to render.

// If we're being honest, featureFlagsLoading shouldn't be tracked by Redux
// and this code should go away eventually.
setFeatureFlagsLoaded();
if (client) {
setFeatureFlagsLoaded();
}
}
}
}, [client, username, account, accountError]);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,7 @@
import { render, waitFor } from '@testing-library/react';
import * as React from 'react';

import { rest, server } from 'src/mocks/testServer';
import { wrapWithTheme } from 'src/utilities/testHelpers';

import MainContent, {
import {
checkFlagsForMainContentBanner,
checkPreferencesForBannerDismissal,
} from './MainContent';
import { queryClientFactory } from './queries/base';

const queryClient = queryClientFactory();

const mainContentBanner = {
key: 'Test Text Key',
Expand Down Expand Up @@ -52,31 +43,3 @@ describe('checkPreferencesForBannerDismissal', () => {
expect(checkPreferencesForBannerDismissal({}, 'key1')).toBe(false);
});
});

describe('Databases menu item for a restricted user', () => {
it('should not render the menu item', async () => {
server.use(
rest.get('*/account', (req, res, ctx) => {
return res(ctx.json({}));
})
);
const { getByText } = render(
wrapWithTheme(
<MainContent appIsLoading={false} isLoggedInAsCustomer={true} />,
{
flags: { databases: false },
queryClient,
}
)
);

await waitFor(() => {
let el;
try {
el = getByText('Databases');
} catch (e) {
expect(el).not.toBeDefined();
}
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,14 @@ describe('AuthenticationWrapper', () => {
});
it('should render one child when showChildren state is true', () => {
component.setState({ showChildren: true });
expect(component.childAt(0)).toHaveLength(1);
expect(component.childAt(0)).toBeDefined();
});
it('should invoke props.initSession when the component is mounted', () => {
expect(component.instance().props.initSession).toHaveBeenCalledTimes(1);
});

it('should set showChildren state to true when the isAuthenticated prop goes from false to true', () => {
it('should not showChildren initially because they should be shown after makeInitialRequests', () => {
component.setState({ showChildren: false });
component.setProps({ isAuthenticated: true });
expect(component.state('showChildren')).toBeTruthy();
expect(component.state('showChildren')).toBeFalsy();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { handleInitTokens } from 'src/store/authentication/authentication.action
import { handleLoadingDone } from 'src/store/initialLoad/initialLoad.actions';
import { State as PendingUploadState } from 'src/store/pendingUpload';
import { MapState } from 'src/store/types';
import SplashScreen from '../SplashScreen';

interface Props {
children: React.ReactNode;
Expand All @@ -34,11 +35,6 @@ type CombinedProps = Props &
WithApplicationStoreProps;

export class AuthenticationWrapper extends React.Component<CombinedProps> {
state = {
hasEnsuredAllTypes: false,
showChildren: false,
};

componentDidMount() {
const { initSession } = this.props;
/**
Expand All @@ -54,8 +50,6 @@ export class AuthenticationWrapper extends React.Component<CombinedProps> {
* to show the children onMount
*/
if (this.props.isAuthenticated) {
this.setState({ showChildren: true });

this.makeInitialRequests();
}
}
Expand All @@ -72,8 +66,6 @@ export class AuthenticationWrapper extends React.Component<CombinedProps> {
!this.state.showChildren
) {
this.makeInitialRequests();

return this.setState({ showChildren: true });
}

/** basically handles for the case where our token is expired or we got a 401 error */
Expand All @@ -90,8 +82,12 @@ export class AuthenticationWrapper extends React.Component<CombinedProps> {
render() {
const { children } = this.props;
const { showChildren } = this.state;
// eslint-disable-next-line
return <React.Fragment>{showChildren ? children : null}</React.Fragment>;

if (showChildren) {
return children;
}

return <SplashScreen />;
}

static defaultProps = {
Expand All @@ -109,6 +105,7 @@ export class AuthenticationWrapper extends React.Component<CombinedProps> {
makeInitialRequests = async () => {
// When loading Lish we avoid all this extra data loading
if (window.location?.pathname?.match(/linodes\/[0-9]+\/lish/)) {
this.setState({ showChildren: true });
return;
}

Expand Down Expand Up @@ -142,8 +139,13 @@ export class AuthenticationWrapper extends React.Component<CombinedProps> {
/** We choose to do nothing, relying on the Redux error state. */
} finally {
this.props.markAppAsDoneLoading();
this.setState({ showChildren: true });
}
};

state = {
showChildren: false,
};
}

interface StateProps {
Expand Down
Loading

0 comments on commit f26f788

Please sign in to comment.