Skip to content

Commit

Permalink
🔥 Remove unused Shopify connect page component to clean up project st…
Browse files Browse the repository at this point in the history
…ructure

♻️ Refactor deleteContentSpaceFile to return revalidateTagKey for better testability

♻️ Refactor deleteContentSpaceFiles to return revalidateTagKey for consistency and testability

♻️ Refactor deleteEventPassFile to return revalidateTagKey, aligning with new testing strategy

♻️ Refactor deleteEventPassFiles to return revalidateTagKey, improving code consistency and testability

✅ Update tests to expect return values from delete actions, enhancing test coverage and reliability

✨ (useConnectWallet.spec.tsx, useConnectWallet.tsx): add integration with iframe for dapp connection
✅ (useConnectWallet.spec.tsx): update tests to cover new functionality and error handling
♻️ (useConnectWallet.tsx): refactor to use useEffect for automatic dapp connection on wallet and customer id availability
💡 (useConnectWallet.tsx): add comments to clarify mutation usage and effects

♻️ (Connect.stories.tsx, Connect.tsx, examples.tsx): refactor Shopify connect components to simplify and centralize UI logic

✨ (Connect.tsx): enhance button loading states and error handling for a smoother user experience

🔧 (examples.tsx): update mock data to align with new simplified connect logic

💡 (Connect.tsx): add detailed comments and clean up unused code to improve maintainability
  • Loading branch information
sebpalluel committed Jul 15, 2024
1 parent c03afdb commit 180900d
Show file tree
Hide file tree
Showing 14 changed files with 247 additions and 278 deletions.
13 changes: 0 additions & 13 deletions apps/unlock/app/shopify/v1/connect/[address]/page.tsx

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import {
} from './deleteContentSpaceFile';

jest.mock('@file-upload/admin');
jest.mock('next/cache');
jest.mock('next/cache', () => ({
revalidateTag: jest.fn(),
}));

describe('deleteContentSpaceFile', () => {
const mockDeleteFile = jest.fn();
Expand All @@ -21,25 +23,25 @@ describe('deleteContentSpaceFile', () => {
beforeEach(() => {
jest.clearAllMocks();
});
it('should call deleteFile and revalidateTag with correct arguments', async () => {
it('should call deleteFile and return correct revalidateTagKey', async () => {
const props: DeleteContentSpaceFileProps = {
organizerId: 'testOrganizerId',
contentSpaceId: 'testContentSpaceId',
filePath:
'/local/organizers/testOrganizerId/content-spaces/testContentSpaceId/testFile',
};

await deleteContentSpaceFile(props);
const result = await deleteContentSpaceFile(props);

expect(mockDeleteFile).toHaveBeenCalledWith({
accountId: expect.any(String), // UPLOAD_ACCOUNT_ID
filePath:
'/local/organizers/testOrganizerId/content-spaces/testContentSpaceId/testFile',
});

expect(mockRevalidateTag).toHaveBeenCalledWith(
`${props.organizerId}-${props.contentSpaceId}-getContentSpaceFiles`,
);
expect(result).toEqual({
revalidateTagKey: `${props.organizerId}-${props.contentSpaceId}-getContentSpaceFiles`,
});
});

it('should handle error from deleteFile', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,9 @@ export async function deleteContentSpaceFile({
}: DeleteContentSpaceFileProps) {
const fileApi = new FileWrapper();
await fileApi.deleteFile({ accountId: env.UPLOAD_ACCOUNT_ID, filePath });
revalidateTag(`${organizerId}-${contentSpaceId}-getContentSpaceFiles`);

const revalidateTagKey = `${organizerId}-${contentSpaceId}-getContentSpaceFiles`;
revalidateTag(revalidateTagKey);

return { revalidateTagKey };
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,18 @@ describe('deleteContentSpaceFiles', () => {
filesSelected: { file1: true, file2: false },
};

await deleteContentSpaceFiles(props);
const result = await deleteContentSpaceFiles(props);

expect(result).toEqual({
revalidateTagKey: `${props.organizerId}-${props.contentSpaceId}-getContentSpaceFiles`,
});

expect(mockDeleteFilesBatchWithRetry).toHaveBeenCalledWith(
expect.any(String), // UPLOAD_ACCOUNT_ID
[
`/${env.UPLOAD_PATH_PREFIX}/organizers/testOrganizerId/content-spaces/testContentSpaceId/file1`,
], // filesToDelete
);

expect(mockRevalidateTag).toHaveBeenCalledWith(
`${props.organizerId}-${props.contentSpaceId}-getContentSpaceFiles`,
);
});
it('should call deleteFilesBatchWithRetry with all files when all are selected', async () => {
const props: DeleteContentSpaceFilesProps = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,7 @@ export async function deleteContentSpaceFiles({
.map(([fileName, _selected]) => `${folderPath}/${fileName}`);
if (!filesToDelete.length) throw new Error('No files to delete selected');
await fileApi.deleteFilesBatchWithRetry(env.UPLOAD_ACCOUNT_ID, filesToDelete);
revalidateTag(`${organizerId}-${contentSpaceId}-getContentSpaceFiles`);
const revalidateTagKey = `${organizerId}-${contentSpaceId}-getContentSpaceFiles`;
revalidateTag(revalidateTagKey);
return { revalidateTagKey };
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { FileWrapper } from '@file-upload/admin';
import { revalidateTag } from 'next/cache';
import {
deleteEventPassFile,
DeleteEventPassFileProps,
} from './deleteEventPassFile';
import { revalidateTag } from 'next/cache';

jest.mock('@file-upload/admin');
jest.mock('next/cache');
Expand All @@ -30,17 +30,17 @@ describe('deleteEventPassFile', () => {
'/local/organizers/testOrganizerId/events/testEventId/testEventPassId/testFile',
};

await deleteEventPassFile(props);
const result = await deleteEventPassFile(props);

expect(result).toEqual({
revalidateTagKey: `${props.organizerId}-${props.eventId}-${props.eventPassId}-getEventPassNftFile`,
});

expect(mockDeleteFile).toHaveBeenCalledWith({
accountId: expect.any(String), // UPLOAD_ACCOUNT_ID
filePath:
'/local/organizers/testOrganizerId/events/testEventId/testEventPassId/testFile',
});

expect(mockRevalidateTag).toHaveBeenCalledWith(
`${props.organizerId}-${props.eventId}-${props.eventPassId}-getEventPassNftFile`,
);
});

it('should handle error from deleteFile', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,7 @@ export async function deleteEventPassFile({
}: DeleteEventPassFileProps) {
const fileApi = new FileWrapper();
await fileApi.deleteFile({ accountId: env.UPLOAD_ACCOUNT_ID, filePath });
revalidateTag(`${organizerId}-${eventId}-${eventPassId}-getEventPassNftFile`);
const revalidateTagKey = `${organizerId}-${eventId}-${eventPassId}-getEventPassNftFile`;
revalidateTag(revalidateTagKey);
return { revalidateTagKey };
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import env from '@env/server';
import { FileWrapper } from '@file-upload/admin';
import { revalidateTag } from 'next/cache';
import {
deleteEventPassFiles,
DeleteEventPassFilesProps,
Expand All @@ -16,9 +15,6 @@ describe('deleteEventPassFiles', () => {
// Mock the FileWrapper instance method
FileWrapper.prototype.deleteFilesBatchWithRetry =
mockDeleteFilesBatchWithRetry;

// Mock the revalidateTag function
(revalidateTag as jest.Mock) = mockRevalidateTag;
});
beforeEach(() => {
jest.clearAllMocks();
Expand All @@ -31,18 +27,18 @@ describe('deleteEventPassFiles', () => {
filesSelected: { file1: true, file2: false },
};

await deleteEventPassFiles(props);
const result = await deleteEventPassFiles(props);

expect(result).toEqual({
revalidateTagKey: `${props.organizerId}-${props.eventId}-${props.eventPassId}-getEventPassNftFiles`,
});

expect(mockDeleteFilesBatchWithRetry).toHaveBeenCalledWith(
expect.any(String), // UPLOAD_ACCOUNT_ID
[
`/${env.UPLOAD_PATH_PREFIX}/organizers/testOrganizerId/events/testEventId/testEventPassId/file1`,
], // filesToDelete
);

expect(mockRevalidateTag).toHaveBeenCalledWith(
`${props.organizerId}-${props.eventId}-${props.eventPassId}-getEventPassNftFiles`,
);
});
it('should call deleteFilesBatchWithRetry with all files when all are selected', async () => {
const props: DeleteEventPassFilesProps = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export async function deleteEventPassFiles({
.map(([fileName, _selected]) => `${folderPath}/${fileName}`);
if (!filesToDelete.length) throw new Error('No files to delete selected');
await fileApi.deleteFilesBatchWithRetry(env.UPLOAD_ACCOUNT_ID, filesToDelete);
revalidateTag(
`${organizerId}-${eventId}-${eventPassId}-getEventPassNftFiles`,
);
const revalidateTagKey = `${organizerId}-${eventId}-${eventPassId}-getEventPassNftFiles`;
revalidateTag(revalidateTagKey);
return { revalidateTagKey };
}
135 changes: 115 additions & 20 deletions libs/features/unlock/shopify/src/lib/hooks/useConnectWallet.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { useIframeConnect } from '@next/iframe';
import { useWalletAuth } from '@next/wallet';
import { act, renderHook } from '@testing-library/react';
import { usePathname, useRouter, useSearchParams } from 'next/navigation';
import { useConnectWallet } from './useConnectWallet';
import { useShopifyCustomer } from './useShopifyCustomer';

jest.mock('next/navigation');
jest.mock('@next/wallet');
Expand All @@ -18,26 +19,37 @@ jest.mock('@tanstack/react-query', () => ({
...options,
})),
}));
jest.mock('@next/iframe', () => ({
useIframeConnect: jest.fn(),
}));
jest.mock('./useShopifyCustomer', () => ({
useShopifyCustomer: jest.fn(),
}));

describe('useConnectWallet', () => {
const mockRouter = { replace: jest.fn() };
const mockPathname = '/test';
const mockSearchParams = new URLSearchParams('param1=value1');
const mockConnect = jest.fn();
const mockSignWithEthereum = jest.fn();

beforeEach(() => {
jest.clearAllMocks();
(useRouter as jest.Mock).mockReturnValue(mockRouter);
(usePathname as jest.Mock).mockReturnValue(mockPathname);
(useSearchParams as jest.Mock).mockReturnValue(mockSearchParams);
(useWalletAuth as jest.Mock).mockReturnValue({ connect: mockConnect });
(useIframeConnect as jest.Mock).mockReturnValue({
signWithEthereum: mockSignWithEthereum,
});
(useShopifyCustomer as jest.Mock).mockReturnValue({
customer: { id: 'testCustomerId' },
});
});

it('should initialize with correct default values', () => {
const { result } = renderHook(() => useConnectWallet());

expect(result.current.accountNotMatching).toBe(false);
expect(result.current.connectWalletMutation).toBeDefined();
expect(result.current.connectToDappMutation).toBeDefined();
});

it('should call connect function with correct parameters', async () => {
Expand All @@ -54,8 +66,8 @@ describe('useConnectWallet', () => {
expect(mockConnect).toHaveBeenCalledWith('0x123', true, '0x456');
});

it('should update URL on successful connection', async () => {
mockConnect.mockResolvedValue('0x789');
it('should set accountNotMatching to true on specific error', async () => {
mockConnect.mockRejectedValue(new Error('Wallet address does not match'));
const { result } = renderHook(() => useConnectWallet());

await act(async () => {
Expand All @@ -66,13 +78,11 @@ describe('useConnectWallet', () => {
});
});

expect(mockRouter.replace).toHaveBeenCalledWith(
'/test/0x789?param1=value1',
);
expect(result.current.accountNotMatching).toBe(true);
});

it('should set accountNotMatching to true on specific error', async () => {
mockConnect.mockRejectedValue(new Error('Wallet address does not match'));
it('should not set accountNotMatching to true on other errors', async () => {
mockConnect.mockRejectedValue(new Error('Some other error'));
const { result } = renderHook(() => useConnectWallet());

await act(async () => {
Expand All @@ -83,21 +93,106 @@ describe('useConnectWallet', () => {
});
});

expect(result.current.accountNotMatching).toBe(true);
expect(result.current.accountNotMatching).toBe(false);
});

it('should not set accountNotMatching to true on other errors', async () => {
mockConnect.mockRejectedValue(new Error('Some other error'));
it('should provide a retryConnectToDapp function', () => {
const { result } = renderHook(() => useConnectWallet());
expect(result.current.retryConnectToDapp).toBeInstanceOf(Function);
});

it('should call signWithEthereum when wallet and customer id are available', async () => {
(useWalletAuth as jest.Mock).mockReturnValue({
wallet: '0x789',
connect: mockConnect,
});
(useShopifyCustomer as jest.Mock).mockReturnValue({
customer: { id: 'testCustomerId' },
});

const { result } = renderHook(() => useConnectWallet());

await act(async () => {
await result.current.connectWalletMutation.mutate({
walletAddress: '0x123',
isCreatingAccount: false,
walletToConnect: '0x456',
});
// Trigger the useEffect
result.current.connectToDappMutation.mutate('testCustomerId');
});

expect(result.current.accountNotMatching).toBe(false);
expect(mockSignWithEthereum).toHaveBeenCalledWith('testCustomerId');
});

it('should not call signWithEthereum when wallet is not available', () => {
(useWalletAuth as jest.Mock).mockReturnValue({
wallet: null,
connect: mockConnect,
});
(useShopifyCustomer as jest.Mock).mockReturnValue({
customer: { id: 'testCustomerId' },
});

renderHook(() => useConnectWallet());

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

it('should not call signWithEthereum when customer id is not available', () => {
(useWalletAuth as jest.Mock).mockReturnValue({
wallet: '0x789',
connect: mockConnect,
});
(useShopifyCustomer as jest.Mock).mockReturnValue({ customer: null });

renderHook(() => useConnectWallet());

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

it('should call signWithEthereum when retryConnectToDapp is called', async () => {
(useShopifyCustomer as jest.Mock).mockReturnValue({
customer: { id: 'testCustomerId' },
});

const { result } = renderHook(() => useConnectWallet());

await act(async () => {
result.current.retryConnectToDapp();
});

expect(mockSignWithEthereum).toHaveBeenCalledWith('testCustomerId');
});

it('should log success message when connectToDappMutation succeeds', async () => {
const consoleSpy = jest.spyOn(console, 'log');
(useShopifyCustomer as jest.Mock).mockReturnValue({
customer: { id: 'testCustomerId' },
});

const { result } = renderHook(() => useConnectWallet());

await act(async () => {
await result.current.connectToDappMutation.mutate('testCustomerId');
});

expect(consoleSpy).toHaveBeenCalledWith('Connected to dapp');
consoleSpy.mockRestore();
});

it('should log error message when connectToDappMutation fails', async () => {
const consoleErrorSpy = jest.spyOn(console, 'error');
mockSignWithEthereum.mockRejectedValue(new Error('Connection failed'));
(useShopifyCustomer as jest.Mock).mockReturnValue({
customer: { id: 'testCustomerId' },
});

const { result } = renderHook(() => useConnectWallet());

await act(async () => {
await result.current.connectToDappMutation.mutate('testCustomerId');
});

expect(consoleErrorSpy).toHaveBeenCalledWith(
'Error connecting to dapp:',
expect.any(Error),
);
consoleErrorSpy.mockRestore();
});
});
Loading

0 comments on commit 180900d

Please sign in to comment.