Skip to content

Commit

Permalink
Fix some test and non-test warnings (#5294)
Browse files Browse the repository at this point in the history
This fixes 2 warnings:
1. When running ProfileName.test. Here the problem was that we were
calling `button.click` instead of `fireEvent.click`. The former isn't
wrapped by `act` while the latter is.
2. When running MenuButtons.test. Here the problem was the change in the
order of the state changes apply (redux vs react components). Fixed by removing one state.
  • Loading branch information
julienw authored Jan 10, 2025
2 parents 64e89aa + 61f1010 commit 6366efb
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 33 deletions.
54 changes: 23 additions & 31 deletions src/components/app/MenuButtons/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ type DispatchProps = {|

type Props = ConnectedProps<OwnProps, StateProps, DispatchProps>;
type State = $ReadOnly<{|
metaInfoPanelState: 'initial' | 'delete-confirmation' | 'profile-deleted',
metaInfoPanelState: 'initial' | 'delete-confirmation',
|}>;

class MenuButtonsImpl extends React.PureComponent<Props, State> {
Expand Down Expand Up @@ -129,9 +129,6 @@ class MenuButtonsImpl extends React.PureComponent<Props, State> {

_onProfileDeleted = () => {
this.props.profileRemotelyDeleted();
this.setState({
metaInfoPanelState: 'profile-deleted',
});
};

_resetMetaInfoState = () => {
Expand Down Expand Up @@ -196,42 +193,37 @@ class MenuButtonsImpl extends React.PureComponent<Props, State> {
);
}

case 'delete-confirmation': {
if (!currentProfileUploadedInformation) {
throw new Error(
`We're in the state "delete-confirmation" but there's no stored data for this profile, this should not happen.`
);
}

const { name, profileToken, jwtToken } =
currentProfileUploadedInformation;

if (!jwtToken) {
throw new Error(
`We're in the state "delete-confirmation" but there's no JWT token for this profile, this should not happen.`
case 'delete-confirmation':
if (currentProfileUploadedInformation) {
const { name, profileToken, jwtToken } =
currentProfileUploadedInformation;

if (!jwtToken) {
throw new Error(
`We're in the state "delete-confirmation" but there's no JWT token for this profile, this should not happen.`
);
}

const slicedProfileToken = profileToken.slice(0, 6);
const profileName = name ? name : `Profile #${slicedProfileToken}`;
return (
<ProfileDeletePanel
profileName={profileName}
profileToken={profileToken}
jwtToken={jwtToken}
onProfileDeleted={this._onProfileDeleted}
onProfileDeleteCanceled={this._resetMetaInfoState}
/>
);
}

const slicedProfileToken = profileToken.slice(0, 6);
const profileName = name ? name : `Profile #${slicedProfileToken}`;
return (
<ProfileDeletePanel
profileName={profileName}
profileToken={profileToken}
jwtToken={jwtToken}
onProfileDeleted={this._onProfileDeleted}
onProfileDeleteCanceled={this._resetMetaInfoState}
/>
);
}
// The profile data has been deleted

case 'profile-deleted':
// Note that <ProfileDeletePanel> can also render <ProfileDeleteSuccess>
// in some situations. However it's not suitable for this case, because
// we still have to pass jwtToken / profileToken, and we don't have
// these values anymore when we're in this state.
return <ProfileDeleteSuccess />;

default:
throw assertExhaustiveCheck(metaInfoPanelState);
}
Expand Down
4 changes: 2 additions & 2 deletions src/test/components/ProfileName.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ describe('ProfileName', function () {
expect(getProfileNameFromUrl(getState())).toBe(null);

// Click the button to activate it.
button.click();
fireEvent.click(button);
const input = getByDisplayValue(defaultName);

expect(queryByText('Custom name')).not.toBeInTheDocument();
Expand All @@ -92,7 +92,7 @@ describe('ProfileName', function () {
withAnalyticsMock(() => {
const { getByText, getByDisplayValue } = setup();
const button = getByText(defaultName);
button.click();
fireEvent.click(button);
const input = getByDisplayValue(defaultName);
fireEvent.change(input, { target: { value: 'Custom name' } });
fireEvent.blur(input);
Expand Down

0 comments on commit 6366efb

Please sign in to comment.