Skip to content

Commit

Permalink
fix: always write apiToken cookie as it is a functional cookie (#36)
Browse files Browse the repository at this point in the history
  • Loading branch information
dhhyi authored and shauke committed Dec 10, 2019
1 parent 70102d9 commit cbcfc86
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 93 deletions.
51 changes: 13 additions & 38 deletions src/app/core/services/cookies/cookies.service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { PLATFORM_ID } from '@angular/core';
import { TestBed } from '@angular/core/testing';
import { CookiesService as ForeignCookiesService } from '@ngx-utils/cookies';
import { anything, instance, mock, verify, when } from 'ts-mockito';
import { anything, instance, mock, verify } from 'ts-mockito';

import { CookiesService } from './cookies.service';

Expand Down Expand Up @@ -29,44 +29,19 @@ describe('Cookies Service', () => {
verify(foreignCookiesServiceMock.get('dummy')).once();
});

describe('when cookieLaw was not yet accepted', () => {
it('should call remove of underlying implementation', done => {
setTimeout(() => {
cookiesService.remove('dummy');
verify(foreignCookiesServiceMock.remove('dummy')).once();
done();
}, 2000);
});

it('should not call put of underlying implementation', done => {
setTimeout(() => {
cookiesService.put('dummy', 'value');
verify(foreignCookiesServiceMock.put(anything(), anything())).never();
verify(foreignCookiesServiceMock.put(anything(), anything(), anything())).never();
done();
}, 2000);
});
it('should call remove of underlying implementation', done => {
setTimeout(() => {
cookiesService.remove('dummy');
verify(foreignCookiesServiceMock.remove('dummy')).once();
done();
}, 2000);
});

describe('when cookieLaw was accepted', () => {
beforeEach(() => {
when(foreignCookiesServiceMock.get('cookieLawSeen')).thenReturn('true');
});

it('should call remove of underlying implementation', done => {
setTimeout(() => {
cookiesService.remove('dummy');
verify(foreignCookiesServiceMock.remove('dummy')).once();
done();
}, 2000);
});

it('should call put of underlying implementation', done => {
setTimeout(() => {
cookiesService.put('dummy', 'value');
verify(foreignCookiesServiceMock.put('dummy', anything(), anything())).once();
done();
}, 2000);
});
it('should call put of underlying implementation', done => {
setTimeout(() => {
cookiesService.put('dummy', 'value');
verify(foreignCookiesServiceMock.put('dummy', anything(), anything())).once();
done();
}, 2000);
});
});
11 changes: 3 additions & 8 deletions src/app/core/services/cookies/cookies.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,12 @@ import { isPlatformBrowser } from '@angular/common';
import { ApplicationRef, Inject, Injectable, PLATFORM_ID } from '@angular/core';
import { CookiesOptions, CookiesService as ForeignCookiesService } from '@ngx-utils/cookies';
import { ReplaySubject, timer } from 'rxjs';
import { distinct, map, switchMap, take, tap } from 'rxjs/operators';
import { distinct, map, switchMap, take } from 'rxjs/operators';

import { whenTruthy } from 'ish-core/utils/operators';

@Injectable({ providedIn: 'root' })
export class CookiesService {
private cookieLawSeen: boolean;

cookieLawSeen$ = new ReplaySubject<boolean>(1);

constructor(
Expand All @@ -25,8 +23,7 @@ export class CookiesService {
switchMap(() =>
timer(0, 1000).pipe(
map(() => this.cookiesService.get('cookieLawSeen') === 'true'),
distinct(),
tap(cookieLawSeen => (this.cookieLawSeen = cookieLawSeen))
distinct()
)
)
)
Expand All @@ -45,8 +42,6 @@ export class CookiesService {
}

put(key: string, value: string, options?: CookiesOptions) {
if (this.cookieLawSeen) {
this.cookiesService.put(key, value, options);
}
this.cookiesService.put(key, value, options);
}
}
41 changes: 3 additions & 38 deletions src/app/core/store/restore/restore.effects.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { RouterTestingModule } from '@angular/router/testing';
import { provideMockActions } from '@ngrx/effects/testing';
import { Action, combineReducers } from '@ngrx/store';
import { cold } from 'jest-marbles';
import { Observable, ReplaySubject, of } from 'rxjs';
import { Observable, of } from 'rxjs';
import { anyString, anything, capture, instance, mock, verify, when } from 'ts-mockito';

import { Order } from 'ish-core/models/order/order.model';
Expand All @@ -28,12 +28,9 @@ describe('Restore Effects', () => {
let actions$: Observable<Action>;
let cookiesServiceMock: CookiesService;
let store$: TestStore;
let cookieLawSubject$: ReplaySubject<boolean>;

beforeEach(() => {
cookiesServiceMock = mock(CookiesService);
cookieLawSubject$ = new ReplaySubject(1);
when(cookiesServiceMock.cookieLawSeen$).thenReturn(cookieLawSubject$);

TestBed.configureTestingModule({
imports: [
Expand Down Expand Up @@ -118,10 +115,6 @@ describe('Restore Effects', () => {
});

describe('saveAPITokenToCookie$', () => {
beforeEach(() => {
cookieLawSubject$.next(true);
});

it('should not save token when neither basket nor user nor order is available', () => {
store$.dispatch(new SetAPIToken({ apiToken: 'dummy' }));

Expand Down Expand Up @@ -197,9 +190,7 @@ describe('Restore Effects', () => {
});

describe('logOutUserIfTokenVanishes$', () => {
it('should log out user when token is not available and cookie law was accepted', done => {
cookieLawSubject$.next(true);

it('should log out user when token is not available', done => {
store$.dispatch(new LoginUserSuccess({ user: { email: 'test@intershop.de' } as User, customer: undefined }));

restoreEffects.logOutUserIfTokenVanishes$.subscribe(
Expand All @@ -211,23 +202,10 @@ describe('Restore Effects', () => {
fail
);
});

it('should do nothing when cookie law was not yet accepted', done => {
cookieLawSubject$.next(false);

store$.dispatch(new LoginUserSuccess({ user: { email: 'test@intershop.de' } as User, customer: undefined }));

restoreEffects.logOutUserIfTokenVanishes$.subscribe(fail, fail, fail);

// terminate checking
setTimeout(done, 3000);
});
});

describe('removeAnonymousBasketIfTokenVanishes$', () => {
it('should remove basket when token is not available and cookie law was accepted', done => {
cookieLawSubject$.next(true);

it('should remove basket when token is not available', done => {
store$.dispatch(new LoadBasketSuccess({ basket: BasketMockData.getBasket() }));

restoreEffects.removeAnonymousBasketIfTokenVanishes$.subscribe({
Expand All @@ -239,20 +217,7 @@ describe('Restore Effects', () => {
});
});

it('should do nothing when cookie law was not yet accepted', done => {
cookieLawSubject$.next(false);

store$.dispatch(new LoadBasketSuccess({ basket: BasketMockData.getBasket() }));

restoreEffects.removeAnonymousBasketIfTokenVanishes$.subscribe(fail, fail, fail);

// terminate checking
setTimeout(done, 3000);
});

it('should do nothing when user is available', done => {
cookieLawSubject$.next(true);

store$.dispatch(new LoginUserSuccess({ user: { email: 'test@intershop.de' } as User, customer: undefined }));
store$.dispatch(new LoadBasketSuccess({ basket: BasketMockData.getBasket() }));

Expand Down
13 changes: 4 additions & 9 deletions src/app/core/store/restore/restore.effects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ export class RestoreEffects {
this.store$.pipe(select(getCurrentBasket)),
this.store$.pipe(select(getSelectedOrderId)),
this.store$.pipe(select(getAPIToken)),
this.cookieService.cookieLawSeen$,
]).pipe(
filter(() => isPlatformBrowser(this.platformId)),
filter(([user, basket, orderId]) => !!user || !!basket || !!orderId),
map(([user, basket, orderId, apiToken]) =>
this.makeCookie({ apiToken, type: user ? 'user' : basket ? 'basket' : 'order', orderId })
Expand All @@ -82,6 +82,7 @@ export class RestoreEffects {
*/
@Effect()
restoreUserOrBasketOrOrderByToken$ = this.router.events.pipe(
filter(() => isPlatformBrowser(this.platformId)),
filter(event => event instanceof NavigationStart),
first(),
map(() => this.cookieService.get('apiToken')),
Expand Down Expand Up @@ -109,8 +110,7 @@ export class RestoreEffects {
concatMapTo(
interval(1000).pipe(
takeWhile(() => isPlatformBrowser(this.platformId)),
withLatestFrom(this.store$.pipe(select(getLoggedInUser)), this.cookieService.cookieLawSeen$),
filter(([, , cookieLawAccepted]) => cookieLawAccepted),
withLatestFrom(this.store$.pipe(select(getLoggedInUser))),
map(([, user]) => ({ user, apiToken: this.cookieService.get('apiToken') })),
filter(({ user, apiToken }) => user && !apiToken),
mapTo(new LogoutUser())
Expand All @@ -125,12 +125,7 @@ export class RestoreEffects {
concatMapTo(
interval(1000).pipe(
takeWhile(() => isPlatformBrowser(this.platformId)),
withLatestFrom(
this.store$.pipe(select(getLoggedInUser)),
this.store$.pipe(select(getCurrentBasket)),
this.cookieService.cookieLawSeen$
),
filter(([, , , cookieLawAccepted]) => cookieLawAccepted),
withLatestFrom(this.store$.pipe(select(getLoggedInUser)), this.store$.pipe(select(getCurrentBasket))),
map(([, user, basket]) => ({ user, basket, apiToken: this.cookieService.get('apiToken') })),
filter(({ user, basket, apiToken }) => !user && basket && !apiToken),
mapTo(new ResetBasket())
Expand Down

0 comments on commit cbcfc86

Please sign in to comment.