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

fix: basket promotion code assignment gets lost after registration #497

Merged
merged 3 commits into from
Jan 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -54,32 +54,50 @@ describe('Basket Handling', () => {
at(ProductDetailPage, page => {
page.addProductToCart().its('status').should('equal', 201);
page.header.miniCart.total.should('contain', _.product.price);
page.header.miniCart.goToCart();
});
});

it('user adds a promotion code that can be applied yet', () => {
at(CartPage, page => {
page.lineItem(0).quantity.set(2);
cy.wait(1000);
page.collapsePromotionForm();
page.submitPromotionCode('INTERSHOP');
page.successMessage.message.should('contain', 'applied');
page.promotion.should('exist');
});
});

it('user logs in again and baskets should be merged', () => {
at(ProductDetailPage, page => page.header.gotoLoginPage());
at(CartPage, page => page.header.gotoLoginPage());
at(LoginPage, page => {
page.fillForm(_.user.login, _.user.password);
page.submit().its('status').should('equal', 200);
waitLoadingEnd(5000);
});
at(MyAccountPage, page => {
page.header.miniCart.total.should('contain', _.product.price * 2);
page.header.miniCart.total.should('contain', _.product.price * 3);
page.header.miniCart.goToCart();
});
at(CartPage, page => {
page.header.miniCart.total.should('contain', _.product.price * 3);
page.header.miniCart.goToCart();
page.promotion.should('exist');
});
});

it('user adds one more product to basket when logged in', () => {
at(MyAccountPage, page => page.header.gotoCategoryPage(_.catalog));
at(CartPage, page => page.header.gotoCategoryPage(_.catalog));
at(CategoryPage, page => page.gotoSubCategory(_.category.id));
at(FamilyPage, page => page.productList.gotoProductDetailPageBySku(_.product.sku));
at(ProductDetailPage, page => {
page.addProductToCart().its('status').should('equal', 200);
waitLoadingEnd(1000);
page.header.miniCart.total.should('contain', _.product.price * 3);
page.header.miniCart.total.should('contain', _.product.price * 4);
});
at(CartPage, page => {
page.lineItem(0).quantity.get().should('equal', '3');
page.lineItem(0).quantity.get().should('equal', '4');
});
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
import { at } from '../../framework';
import { MyAccountPage } from '../../pages/account/my-account.page';
import { Registration, RegistrationPage, sensibleDefaults } from '../../pages/account/registration.page';
import { CartPage } from '../../pages/checkout/cart.page';
import { ProductDetailPage } from '../../pages/shopping/product-detail.page';

const _ = {
productSku: '201807171',
user: {
login: `testuser${new Date().getTime()}@test.intershop.de`,
...sensibleDefaults,
} as Registration,
};

describe('Promotion Handling in Cart', () => {
Expand Down Expand Up @@ -35,6 +41,23 @@ describe('Promotion Handling in Cart', () => {
});
});

it('user registers and the basket is merged with the promotion code', () => {
at(CartPage, page => {
page.header.gotoRegistrationPage();
});
at(RegistrationPage, page => {
page.fillForm(_.user);
page.acceptTAC();
page.submitAndObserve().its('statusMessage').should('equal', '201 (Created)');
});
at(MyAccountPage, page => {
page.header.miniCart.goToCart();
});
at(CartPage, page => {
page.promotion.should('exist');
});
});

it('user removes a promotion code', () => {
at(CartPage, page => {
page.removePromotionCode();
Expand Down
18 changes: 12 additions & 6 deletions src/app/core/services/user/user.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,12 @@ describe('User Service', () => {
describe('SignIn a user', () => {
it('should login a user when correct credentials are entered', done => {
const loginDetail = { login: 'patricia@test.intershop.de', password: '!InterShop00!' };
when(apiServiceMock.get(anything(), anything())).thenReturn(of({ customerNo: 'PC' } as Customer));
when(apiServiceMock.get('customers/-', anything())).thenReturn(of({ customerNo: 'PC' } as Customer));
when(apiServiceMock.get('privatecustomers/-')).thenReturn(of({ customerNo: 'PC' } as Customer));

userService.signinUser(loginDetail).subscribe(data => {
const [, options] = capture<{}, { headers: HttpHeaders }>(apiServiceMock.get).last();
const headers = options.headers;
const [, options] = capture<{}, { headers: HttpHeaders }>(apiServiceMock.get).beforeLast();
const headers = options?.headers;
expect(headers).toBeTruthy();
expect(headers.get('Authorization')).toEqual('BASIC cGF0cmljaWFAdGVzdC5pbnRlcnNob3AuZGU6IUludGVyU2hvcDAwIQ==');

Expand All @@ -51,11 +52,12 @@ describe('User Service', () => {

it('should login a private user when correct credentials are entered', done => {
const loginDetail = { login: 'patricia@test.intershop.de', password: '!InterShop00!' };
when(apiServiceMock.get(anything(), anything())).thenReturn(of({ customerNo: 'PC' } as Customer));
when(apiServiceMock.get('customers/-', anything())).thenReturn(of({ customerNo: 'PC' } as Customer));
when(apiServiceMock.get('privatecustomers/-')).thenReturn(of({ customerNo: 'PC' } as Customer));

userService.signinUser(loginDetail).subscribe(() => {
verify(apiServiceMock.get(`customers/-`, anything())).once();
verify(apiServiceMock.get(`privatecustomers/-`, anything())).once();
verify(apiServiceMock.get(`privatecustomers/-`)).once();
done();
});
});
Expand Down Expand Up @@ -119,16 +121,20 @@ describe('User Service', () => {

it("should create a new individual user when 'createUser' is called", done => {
when(apiServiceMock.post(anyString(), anything(), anything())).thenReturn(of({}));
when(apiServiceMock.get(anything(), anything())).thenReturn(of({ customerNo: 'PC' } as Customer));
when(apiServiceMock.get(anything())).thenReturn(of({ customerNo: 'PC' } as Customer));

const payload = {
customer: { customerNo: '4711', isBusinessCustomer: false } as Customer,
address: {} as Address,
credentials: {} as Credentials,
credentials: { login: 'patricia@test.intershop.de', password: 'xyz' } as Credentials,
user: {} as User,
} as CustomerRegistrationType;

userService.createUser(payload).subscribe(() => {
verify(apiServiceMock.post('privatecustomers', anything(), anything())).once();
verify(apiServiceMock.get('customers/-', anything())).once();
verify(apiServiceMock.get('privatecustomers/-')).once();
done();
});
});
Expand Down
48 changes: 18 additions & 30 deletions src/app/core/services/user/user.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,39 +54,29 @@ export class UserService {
'BASIC ' + b64u.toBase64(b64u.encode(`${loginCredentials.login}:${loginCredentials.password}`))
);

return this.apiService
.get<CustomerData>('customers/-', { headers })
.pipe(
withLatestFrom(this.appFacade.isAppTypeREST$),
concatMap(([data, isAppTypeRest]) =>
// ToDo: #IS-30018 use the customer type for this decision
isAppTypeRest && !data.companyName
? this.apiService.get<CustomerData>('privatecustomers/-', { headers })
: of(data)
),
map(CustomerMapper.mapLoginData)
);
return this.fetchCustomer({ headers });
}

signinUserByToken(): Observable<CustomerUserType> {
return this.apiService
.get<CustomerData>('customers/-', { skipApiErrorHandling: true, runExclusively: true })
.pipe(
withLatestFrom(this.appFacade.isAppTypeREST$),
concatMap(([data, isAppTypeRest]) =>
// ToDo: #IS-30018 use the customer type for this decision
isAppTypeRest && !data.companyName ? this.apiService.get<CustomerData>('privatecustomers/-') : of(data)
),
map(CustomerMapper.mapLoginData),
catchError(() => EMPTY)
);
return this.fetchCustomer({ skipApiErrorHandling: true, runExclusively: true }).pipe(catchError(() => EMPTY));
}

private fetchCustomer(options?: AvailableOptions): Observable<CustomerUserType> {
return this.apiService.get<CustomerData>('customers/-', options).pipe(
withLatestFrom(this.appFacade.isAppTypeREST$),
concatMap(([data, isAppTypeRest]) =>
// ToDo: #IS-30018 use the customer type for this decision
isAppTypeRest && !data.companyName ? this.apiService.get<CustomerData>('privatecustomers/-') : of(data)
),
map(CustomerMapper.mapLoginData)
);
}

/**
* Create a new user for the given data.
* @param body The user data (customer, user, credentials, address) to create a new user.
*/
createUser(body: CustomerRegistrationType): Observable<void> {
createUser(body: CustomerRegistrationType): Observable<CustomerUserType> {
if (!body || !body.customer || !body.user || !body.credentials || !body.address) {
return throwError('createUser() called without required body data');
}
Expand Down Expand Up @@ -118,13 +108,11 @@ export class UserService {
return this.appFacade.isAppTypeREST$.pipe(
first(),
concatMap(isAppTypeRest =>
this.apiService.post<void>(
AppFacade.getCustomerRestResource(body.customer.isBusinessCustomer, isAppTypeRest),
newCustomer,
{
this.apiService
.post<void>(AppFacade.getCustomerRestResource(body.customer.isBusinessCustomer, isAppTypeRest), newCustomer, {
captcha: pick(body, ['captcha', 'captchaAction']),
}
)
})
.pipe(concatMap(() => this.fetchCustomer()))
)
);
}
Expand Down
4 changes: 2 additions & 2 deletions src/app/core/store/customer/basket/basket.effects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
import { Basket } from 'ish-core/models/basket/basket.model';
import { BasketService } from 'ish-core/services/basket/basket.service';
import { RouterState } from 'ish-core/store/core/router/router.reducer';
import { loadUserByAPIToken, loginUser, loginUserSuccess } from 'ish-core/store/customer/user';
import { createUser, loadUserByAPIToken, loginUser, loginUserSuccess } from 'ish-core/store/customer/user';
import { ApiTokenService } from 'ish-core/utils/api-token/api-token.service';
import { mapErrorToAction, mapToPayloadProperty } from 'ish-core/utils/operators';

Expand Down Expand Up @@ -174,7 +174,7 @@ export class BasketEffects {
private anonymousBasket$ = createEffect(
() =>
combineLatest([this.store.pipe(select(getCurrentBasketId)), this.apiTokenService.apiToken$]).pipe(
sample(this.actions$.pipe(ofType(loginUser, loadUserByAPIToken))),
sample(this.actions$.pipe(ofType(loginUser, createUser, loadUserByAPIToken))),
startWith([undefined, undefined])
),
{ dispatch: false }
Expand Down
4 changes: 2 additions & 2 deletions src/app/core/store/customer/user/user.effects.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,11 +243,11 @@ describe('User Effects', () => {
});
});

it('should dispatch a CreateUserLogin action on successful user creation', () => {
it('should dispatch a LoginUserSuccess action on successful user creation', () => {
const credentials: Credentials = { login: '1234', password: 'xxx' };

const action = createUser({ credentials } as CustomerRegistrationType);
const completion = loginUser({ credentials });
const completion = loginUserSuccess({} as CustomerUserType);

actions$ = hot('-a', { a: action });
const expected$ = cold('-b', { b: completion });
Expand Down
6 changes: 1 addition & 5 deletions src/app/core/store/customer/user/user.effects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,7 @@ export class UserEffects {
ofType(createUser),
mapToPayload(),
mergeMap((data: CustomerRegistrationType) =>
this.userService.createUser(data).pipe(
// TODO:see #IS-22750 - user should actually be logged in after registration
map(() => loginUser({ credentials: data.credentials })),
mapErrorToAction(createUserFail)
)
this.userService.createUser(data).pipe(map(loginUserSuccess), mapErrorToAction(createUserFail))
)
)
);
Expand Down