Skip to content

Commit

Permalink
Fix showing success bunner on app install twice (#5298)
Browse files Browse the repository at this point in the history
* Add loading and test

* Update test

* Add changeset
  • Loading branch information
poulch committed Dec 5, 2024
1 parent 8d08677 commit 315676c
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 4 deletions.
5 changes: 5 additions & 0 deletions .changeset/flat-waves-sip.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"saleor-dashboard": patch
---

The double success banner no longer appears during app installation
109 changes: 109 additions & 0 deletions src/apps/hooks/useActiveAppsInstallations.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import { AppsInstallationsQuery } from "@dashboard/graphql";
import useLocalStorage from "@dashboard/hooks/useLocalStorage";
import { renderHook } from "@testing-library/react-hooks";

import useActiveAppsInstallations from "./useActiveAppsInstallations";

jest.mock("react-intl", () => ({
useIntl: jest.fn(() => ({
formatMessage: jest.fn(x => x.defaultMessage),
})),
defineMessages: jest.fn(x => x),
}));

jest.mock("@apollo/client", () => ({
gql: jest.fn(),
useApolloClient: jest.fn(),
}));

jest.mock("@dashboard/hooks/useLocalStorage");
jest.mock("@dashboard/graphql", () => ({
useAppRetryInstallMutation: jest.fn(() => [jest.fn(), {}]),
useAppDeleteFailedInstallationMutation: jest.fn(() => [jest.fn(), {}]),
}));

jest.useFakeTimers();

describe("useActiveAppsInstallations", () => {
it("install app notify should not be called when apps in progress data loading", () => {
// Arrange
const mockedActiveInstallations = [
{
id: "1",
name: "app1",
},
];
const appInProgressLoading = true;
const appsInProgressData = {
appInstallations: [],
} as unknown as AppsInstallationsQuery;
const mockNotify = jest.fn();

(useLocalStorage as jest.Mock).mockReturnValue([mockedActiveInstallations, jest.fn()]);

renderHook(() =>
useActiveAppsInstallations({
appsInProgressData,
appsInProgressRefetch: jest.fn(),
appInProgressLoading,
appsRefetch: jest.fn(),
installedAppNotify: mockNotify,
removeInProgressAppNotify: jest.fn(),
onInstallSuccess: jest.fn(),
onInstallError: jest.fn(),
onRemoveInProgressAppSuccess: jest.fn(),
}),
);

expect(mockNotify).not.toHaveBeenCalled();
});

it("should call install app notify when apps in progress data loaded and no item found", () => {
// Arrange
const mockedActiveInstallations = [
{
id: "1",
name: "app1",
},
];
const appsInProgressData = {
appsInstallations: [],
} as unknown as AppsInstallationsQuery;
const mockNotify = jest.fn();

jest.useFakeTimers();

(useLocalStorage as jest.Mock).mockReturnValue([mockedActiveInstallations, jest.fn()]);

const { rerender } = renderHook(
({ appsInProgressData, appInProgressLoading }) =>
useActiveAppsInstallations({
appsInProgressData,
appsInProgressRefetch: jest.fn(),
appInProgressLoading,
appsRefetch: jest.fn(),
installedAppNotify: mockNotify,
removeInProgressAppNotify: jest.fn(),
onInstallSuccess: jest.fn(),
onInstallError: jest.fn(),
onRemoveInProgressAppSuccess: jest.fn(),
}),
{
initialProps: {
appsInProgressData: undefined,
appInProgressLoading: true,
} as {
appsInProgressData: AppsInstallationsQuery | undefined;
appInProgressLoading: boolean;
},
},
);

rerender({
appsInProgressData,
appInProgressLoading: false,
});

expect(mockNotify).toHaveBeenCalled();
});
});
14 changes: 11 additions & 3 deletions src/apps/hooks/useActiveAppsInstallations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { useEffect, useRef } from "react";

interface UseActiveAppsInstallations {
appsInProgressData: AppsInstallationsQuery | undefined;
appInProgressLoading: boolean;
appsInProgressRefetch: () => void;
appsRefetch: () => void;
removeInProgressAppNotify: () => void;
Expand All @@ -24,6 +25,7 @@ interface UseActiveAppsInstallations {
function useActiveAppsInstallations({
appsInProgressData,
appsInProgressRefetch,
appInProgressLoading,
appsRefetch,
installedAppNotify,
removeInProgressAppNotify,
Expand Down Expand Up @@ -130,7 +132,7 @@ function useActiveAppsInstallations({
useEffect(() => {
const appsInProgress = appsInProgressData?.appsInstallations || [];

if (activeInstallations.length && !!appsInProgressData) {
if (activeInstallations.length && !appInProgressLoading) {
let newAppInstalled = false;

activeInstallations.forEach(installation => {
Expand Down Expand Up @@ -162,8 +164,14 @@ function useActiveAppsInstallations({
return {
handleAppInstallRetry,
handleRemoveInProgress,
retryInstallAppOpts,
deleteInProgressAppOpts,
retryInstallAppOpts: {
status: retryInstallAppOpts.status,
loading: retryInstallAppOpts.loading,
},
deleteInProgressAppOpts: {
status: deleteInProgressAppOpts.status,
loading: deleteInProgressAppOpts.loading,
},
};
}
export default useActiveAppsInstallations;
7 changes: 6 additions & 1 deletion src/apps/views/AppListView/AppListView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,11 @@ export const AppListView: React.FC<Props> = ({ params }) => {
installedAppsData?.apps?.pageInfo,
paginationState,
);
const { data: appsInProgressData, refetch: appsInProgressRefetch } = useAppsInstallationsQuery({
const {
data: appsInProgressData,
refetch: appsInProgressRefetch,
loading: appInProgressLoading,
} = useAppsInstallationsQuery({
displayLoader: false,
skip: !hasManagedAppsPermission,
});
Expand Down Expand Up @@ -98,6 +102,7 @@ export const AppListView: React.FC<Props> = ({ params }) => {
const { handleAppInstallRetry, handleRemoveInProgress, deleteInProgressAppOpts } =
useActiveAppsInstallations({
appsInProgressData,
appInProgressLoading,
appsInProgressRefetch,
appsRefetch,
installedAppNotify,
Expand Down

0 comments on commit 315676c

Please sign in to comment.