From cbcfc86164524c5bd60badaddd397324b8e6f17b Mon Sep 17 00:00:00 2001 From: Danilo Hoffmann Date: Tue, 10 Dec 2019 09:51:38 +0100 Subject: [PATCH] fix: always write apiToken cookie as it is a functional cookie (#36) --- .../services/cookies/cookies.service.spec.ts | 51 +++++-------------- .../core/services/cookies/cookies.service.ts | 11 ++-- .../store/restore/restore.effects.spec.ts | 41 ++------------- src/app/core/store/restore/restore.effects.ts | 13 ++--- 4 files changed, 23 insertions(+), 93 deletions(-) diff --git a/src/app/core/services/cookies/cookies.service.spec.ts b/src/app/core/services/cookies/cookies.service.spec.ts index c79bcf3b67..772c8a99eb 100644 --- a/src/app/core/services/cookies/cookies.service.spec.ts +++ b/src/app/core/services/cookies/cookies.service.spec.ts @@ -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'; @@ -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); }); }); diff --git a/src/app/core/services/cookies/cookies.service.ts b/src/app/core/services/cookies/cookies.service.ts index a5fb838fed..e72ea62568 100644 --- a/src/app/core/services/cookies/cookies.service.ts +++ b/src/app/core/services/cookies/cookies.service.ts @@ -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(1); constructor( @@ -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() ) ) ) @@ -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); } } diff --git a/src/app/core/store/restore/restore.effects.spec.ts b/src/app/core/store/restore/restore.effects.spec.ts index 98de777794..96624071b9 100644 --- a/src/app/core/store/restore/restore.effects.spec.ts +++ b/src/app/core/store/restore/restore.effects.spec.ts @@ -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'; @@ -28,12 +28,9 @@ describe('Restore Effects', () => { let actions$: Observable; let cookiesServiceMock: CookiesService; let store$: TestStore; - let cookieLawSubject$: ReplaySubject; beforeEach(() => { cookiesServiceMock = mock(CookiesService); - cookieLawSubject$ = new ReplaySubject(1); - when(cookiesServiceMock.cookieLawSeen$).thenReturn(cookieLawSubject$); TestBed.configureTestingModule({ imports: [ @@ -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' })); @@ -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( @@ -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({ @@ -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() })); diff --git a/src/app/core/store/restore/restore.effects.ts b/src/app/core/store/restore/restore.effects.ts index 74b1aaf997..fbf23293cf 100644 --- a/src/app/core/store/restore/restore.effects.ts +++ b/src/app/core/store/restore/restore.effects.ts @@ -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 }) @@ -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')), @@ -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()) @@ -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())