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.

Sign up for GitHub

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

V6 fix initial state upi intent #2921

Merged
merged 2 commits into from
Oct 28, 2024
Merged

V6 fix initial state upi intent #2921

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d0b018e
fix(upi): initial state
longyulongyu Oct 25, 2024
7272a8c
refactor: fix tests
longyulongyu Oct 25, 2024
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
5 changes: 5 additions & 0 deletions .changeset/lovely-dodos-visit.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@adyen/adyen-web": patch
---

Fix `UPIComponent` initial value for `isValid`. It should only be default to `true` for UPI QR.
84 changes: 82 additions & 2 deletions packages/lib/src/components/UPI/UPI.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { render, screen } from '@testing-library/preact';
import { render, screen, waitFor } from '@testing-library/preact';
import userEvent from '@testing-library/user-event';
import UPI from './UPI';
import isMobile from '../../utils/isMobile';
import { SRPanel } from '../../core/Errors/SRPanel';
Expand Down Expand Up @@ -102,11 +103,90 @@ describe('UPI', () => {
});
});

describe('isValid', () => {
describe('select the QR code mode', () => {
test('should be valid', async () => {
isMobileMock.mockReturnValue(false);
const upi = new UPI(global.core, { ...props, defaultMode: 'qrCode' });
render(upi.render());
await waitFor(() => {
expect(upi.isValid).toBe(true);
});
});
});

describe('select the vpa mode', () => {
test('should not be valid on init', async () => {
isMobileMock.mockReturnValue(false);
const upi = new UPI(global.core, { ...props, defaultMode: 'vpa' });
render(upi.render());
await waitFor(() => {
expect(upi.isValid).toBe(false);
});
});

test('should be valid when filling in the vpa', async () => {
isMobileMock.mockReturnValue(false);
const upi = new UPI(global.core, { ...props, defaultMode: 'vpa' });
render(upi.render());
const user = userEvent.setup();
const vpaInput = await screen.findByLabelText(/Enter UPI ID \/ VPA/i);
await user.type(vpaInput, 'test@test');
expect(upi.isValid).toBe(true);
});
});

describe('select the intent mode', () => {
beforeEach(() => {
isMobileMock.mockReturnValue(true);
});

test('should not be valid on init', async () => {
const upi = new UPI(global.core, { ...props, apps: [{ id: 'gpay', name: 'Google Pay' }] });
render(upi.render());
await waitFor(() => {
expect(upi.isValid).toBe(false);
});
});

test('should be valid when selecting other apps', async () => {
const upi = new UPI(global.core, { ...props, apps: [{ id: 'gpay', name: 'Google Pay' }] });
render(upi.render());
const user = userEvent.setup();
const radioButton = await screen.findByRole('radio', { name: /google pay/i });
await user.click(radioButton);
expect(upi.isValid).toBe(true);
});

test('should not be valid when selecting upi collect', async () => {
const upi = new UPI(global.core, { ...props, apps: [{ id: 'gpay', name: 'Google Pay' }] });
render(upi.render());
const user = userEvent.setup();
const radioButton = await screen.findByRole('radio', { name: /Enter UPI ID/i });
await user.click(radioButton);
expect(upi.isValid).toBe(false);
});

test('should be valid when filling the vpa', async () => {
const upi = new UPI(global.core, { ...props, apps: [{ id: 'gpay', name: 'Google Pay' }] });
render(upi.render());
const user = userEvent.setup();
const radioButton = await screen.findByRole('radio', { name: /Enter UPI ID/i });
await user.click(radioButton);
expect(upi.isValid).toBe(false);

const vpaInput = await screen.findByLabelText(/Enter UPI ID \/ VPA/i);
await user.type(vpaInput, 'test@test');
expect(upi.isValid).toBe(true);
});
});
});

describe('render', () => {
test('should render the UPI component by default', async () => {
const upi = new UPI(global.core, props);
render(upi.render());
expect(await screen.findByRole('group')).toBeInTheDocument;
expect(await screen.findByRole('group')).toBeInTheDocument();
});
});
});
2 changes: 1 addition & 1 deletion packages/lib/src/components/UPI/components/UPIComponent/UPIComponent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export default function UPIComponent({ defaultMode, onChange, onUpdateMode, payB
const { i18n } = useCoreContext();
const getImage = useImage();
const [status, setStatus] = useState<UIElementStatus>('ready');
const [isValid, setIsValid] = useState<boolean>(true);
const [isValid, setIsValid] = useState<boolean>(defaultMode === 'qrCode');
Copy link
Contributor

Choose a reason for hiding this comment

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

How come the v5 version has a constant (defaultMode === UpiMode.QrCode) but here we're using a string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In v6, we use string literals for the UpiMode, but in v5 we define it as enum, that's why we need to code differently. The reason to change the type definition is because string literals are easier for JS users.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just an observation: we use the string qrCode in quite a few places - it just feels like something that could be defined as a constant somewhere.
This is also true in a lot of other places in our codebase

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just an observation: we use the string qrCode in quite a few places - it just feels like something that could be defined as a constant somewhere. This is also true in a lot of other places in our codebase

It feels a bit too much for this field, as it only allows 3 values and there is already a type guard for it. Typing any other strings throws a Typescript error.

Otherwise we will need to define the UpiMode like this, so that we can use VPA, QR_CODE or INTENT in the code.

export const VPA = 'vpa';
export const QR_CODE = 'qrCode';
export const INTENT = 'intent';

export type UpiMode = typeof VPA | typeof QR_CODE | typeof INTENT;

const [mode, setMode] = useState<UpiMode>(defaultMode);
const [vpa, setVpa] = useState<string>('');
const [vpaInputHandlers, setVpaInputHandlers] = useState<VpaInputHandlers>(null);
Expand Down
Loading