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.

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

Show Barcode Scan results (basic) #570

Merged
merged 4 commits into from
Mar 15, 2019
Merged

Show Barcode Scan results (basic) #570

merged 4 commits into from
Mar 15, 2019

Conversation

alexbridge
Copy link
Contributor

Description

Show Barcode Scan results (basic)

@alexbridge alexbridge added the enhancement New feature or request label Mar 14, 2019
@alexbridge alexbridge self-assigned this Mar 14, 2019
@alexbridge alexbridge changed the base branch from PWA-1609 to v6.X March 14, 2019 13:47
@alexbridge alexbridge changed the base branch from v6.X to PWA-1609 March 14, 2019 13:47
@alexbridge alexbridge changed the base branch from PWA-1609 to PWA-1739-scanner-ui March 14, 2019 15:16
});

it('should show modal when products not found', async () => {
fetchProductsByQuery.mockReturnValue(Promise.resolve({
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can simplify this by replacing .mockReturnValue(Promise.resolve({})) with .mockResolvedValue({}) - same for the other occurrences.

] = subscribe.mock.calls;
});

it('should initialize subscriptions', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please split the tests by adding two additional describe blocks - one for each subscription.

fetchProductsByQuery.mockReturnValue(Promise.resolve({
totalProductCount: 0,
}));
showModal.mockImplementation(options => Promise.resolve(options));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why didn't you add the mock implementation to the mock creation - like you do it for the other mocks.

import { SCANNER_SUCCESS } from '../constants';

/** @type {Observable} */
export const scanSuccess$ = main$
Copy link
Collaborator

Choose a reason for hiding this comment

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

As discussed please rename scanSuccess$ to scanFinished$.

@alexbridge alexbridge changed the base branch from PWA-1739-scanner-ui to PWA-1609 March 15, 2019 09:12
@fkloes fkloes merged commit 14ba1f6 into PWA-1609 Mar 15, 2019
@fkloes fkloes deleted the PWA-1613 branch March 15, 2019 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants