From 36fb45bd465914cba23791e719c50ed9cea9d18a Mon Sep 17 00:00:00 2001 From: mkovalua Date: Tue, 23 Sep 2025 20:43:18 +0300 Subject: [PATCH 1/3] fix(tos banner): if user is authenticated we check whether is accepted terms of service to hide banner or show if not otherwise user is not authenticated we hide banner always --- .../tos-consent-banner.component.spec.ts | 31 +++++++++++++++---- .../tos-consent-banner.component.ts | 30 ++++++++++++++---- src/assets/i18n/en.json | 3 +- 3 files changed, 51 insertions(+), 13 deletions(-) diff --git a/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.spec.ts b/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.spec.ts index c14796bcd..d86a913be 100644 --- a/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.spec.ts +++ b/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.spec.ts @@ -2,6 +2,8 @@ import { Store } from '@ngxs/store'; import { MockComponent } from 'ng-mocks'; +import { of } from 'rxjs'; + import { ComponentFixture, TestBed } from '@angular/core/testing'; import { By } from '@angular/platform-browser'; @@ -13,16 +15,19 @@ import { IconComponent } from '@shared/components'; import { TosConsentBannerComponent } from './tos-consent-banner.component'; import { TranslationServiceMock } from '@testing/mocks/translation.service.mock'; -import { OSFTestingStoreModule } from '@testing/osf.testing.module'; +import { OSFTestingModule, OSFTestingStoreModule } from '@testing/osf.testing.module'; import { provideMockStore } from '@testing/providers/store-provider.mock'; +import { ToastServiceMockBuilder } from '@testing/providers/toast-provider.mock'; -describe('Component: Tos Consent Banner', () => { +describe('TosConsentBannerComponent', () => { + let component: TosConsentBannerComponent; let fixture: ComponentFixture; - let store: Store; + let store: jest.Mocked; + let toastServiceMock: ReturnType; beforeEach(async () => { await TestBed.configureTestingModule({ - imports: [OSFTestingStoreModule, TosConsentBannerComponent, MockComponent(IconComponent)], + imports: [TosConsentBannerComponent, OSFTestingStoreModule, OSFTestingModule, MockComponent(IconComponent)], providers: [ provideMockStore({ signals: [{ selector: UserSelectors.getCurrentUser, value: MOCK_USER }], @@ -32,12 +37,16 @@ describe('Component: Tos Consent Banner', () => { }).compileComponents(); fixture = TestBed.createComponent(TosConsentBannerComponent); - store = TestBed.inject(Store); + store = TestBed.inject(Store) as jest.Mocked; + component = fixture.componentInstance; + store.dispatch = jest.fn().mockReturnValue(of(undefined)); + toastServiceMock = ToastServiceMockBuilder.create().build(); fixture.detectChanges(); }); it('should have the "Continue" button disabled by default', () => { const continueButton = fixture.debugElement.query(By.css('p-button button')).nativeElement; + console.log('continueButton ' + continueButton ) expect(continueButton.disabled).toBe(true); }); @@ -50,7 +59,6 @@ describe('Component: Tos Consent Banner', () => { }); it('should dispatch AcceptTermsOfServiceByUser action when "Continue" is clicked', () => { - jest.spyOn(store, 'dispatch'); const checkboxInput = fixture.debugElement.query(By.css('p-checkbox input')).nativeElement; checkboxInput.click(); fixture.detectChanges(); @@ -60,5 +68,16 @@ describe('Component: Tos Consent Banner', () => { fixture.detectChanges(); expect(store.dispatch).toHaveBeenCalledWith(new AcceptTermsOfServiceByUser()); + expect(toastServiceMock.showError).not.toHaveBeenCalled(); }); + + it('should show toast banner if acceptedTermsOfService is false and "Continue" is clicked', () => { + component.acceptedTermsOfService.set(false); + const continueButton = fixture.debugElement.query(By.css('p-button button')).nativeElement; + continueButton.disabled = false; + continueButton.click(); + fixture.detectChanges(); + expect(component.errorMessage).toEqual('toast.tos-consent.errorMessage'); + }); + }); diff --git a/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.ts b/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.ts index 196665dd4..b7c511099 100644 --- a/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.ts +++ b/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.ts @@ -1,17 +1,18 @@ import { createDispatchMap, select } from '@ngxs/store'; -import { TranslatePipe } from '@ngx-translate/core'; +import { TranslatePipe, TranslateService } from '@ngx-translate/core'; import { Button } from 'primeng/button'; import { Checkbox } from 'primeng/checkbox'; import { Message } from 'primeng/message'; -import { Component, computed, signal } from '@angular/core'; +import { Component, computed, inject, signal } from '@angular/core'; import { FormsModule } from '@angular/forms'; import { RouterLink } from '@angular/router'; import { AcceptTermsOfServiceByUser, UserSelectors } from '@core/store/user'; import { IconComponent } from '@osf/shared/components'; +import { ToastService } from '@osf/shared/services'; /** * TosConsentBannerComponent displays a Terms of Service (ToS) consent banner for users who haven't accepted yet. @@ -34,6 +35,9 @@ import { IconComponent } from '@osf/shared/components'; templateUrl: './tos-consent-banner.component.html', }) export class TosConsentBannerComponent { + private readonly toastService = inject(ToastService); + private readonly translateService = inject(TranslateService); + /** * NGXS dispatch map for the AcceptTermsOfServiceByUser action. */ @@ -48,14 +52,21 @@ export class TosConsentBannerComponent { * Local signal tracking whether the user has accepted the Terms of Service via checkbox. */ acceptedTermsOfService = signal(false); + errorMessage: string | null = null; /** * Computed signal indicating whether the user has already accepted the Terms of Service. */ - readonly acceptedTermsOfServiceChange = computed(() => { - const user = this.currentUser(); - return user?.acceptedTermsOfService ?? false; - }); + acceptedTermsOfServiceChange = computed(() => + { + const user = this.currentUser() + /** + * if user is authenticated we check whether is accepted terms of service to hide banner or show if not + * otherwise user is not authenticated we hide banner always + */ + return user ? user?.acceptedTermsOfService : true; + } + ); /** * Triggered when the user clicks the Continue button. @@ -63,6 +74,13 @@ export class TosConsentBannerComponent { * - Dispatches `AcceptTermsOfServiceByUser` action otherwise. */ onContinue() { + if (!this.acceptedTermsOfService()) { + this.errorMessage = this.translateService.instant('toast.tos-consent.errorMessage'); + this.toastService.showError(this.errorMessage as string); + return; + } + + this.errorMessage = null; this.actions.acceptTermsOfServiceByUser(); } } diff --git a/src/assets/i18n/en.json b/src/assets/i18n/en.json index dac44ba8b..774d026e2 100644 --- a/src/assets/i18n/en.json +++ b/src/assets/i18n/en.json @@ -221,7 +221,8 @@ "and": " and ", "privacyPolicy": " privacy policy.", "haveReadAndAgree": "I've read and agree to the terms and conditions", - "continue": "Continue" + "continue": "Continue", + "errorMessage": "We were unable to save your consent." }, "cookie-consent": { "message": "Notice: This website relies on cookies to help provide a better user experience. By clicking accept or continuing to use the site, you consent to our use of cookies. See our Privacy Policy and Cookie Use for more information.", From 0b85fd2d571a4bb080e1210818b4b88d00b23ee6 Mon Sep 17 00:00:00 2001 From: mkovalua Date: Wed, 24 Sep 2025 18:10:28 +0300 Subject: [PATCH 2/3] fix(tos banner): resolve CR comments --- .../tos-consent-banner.component.spec.ts | 37 +++++++++++-------- .../tos-consent-banner.component.ts | 12 ------ 2 files changed, 22 insertions(+), 27 deletions(-) diff --git a/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.spec.ts b/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.spec.ts index d86a913be..bf80c9aae 100644 --- a/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.spec.ts +++ b/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.spec.ts @@ -15,19 +15,18 @@ import { IconComponent } from '@shared/components'; import { TosConsentBannerComponent } from './tos-consent-banner.component'; import { TranslationServiceMock } from '@testing/mocks/translation.service.mock'; -import { OSFTestingModule, OSFTestingStoreModule } from '@testing/osf.testing.module'; +import { OSFTestingStoreModule } from '@testing/osf.testing.module'; import { provideMockStore } from '@testing/providers/store-provider.mock'; -import { ToastServiceMockBuilder } from '@testing/providers/toast-provider.mock'; + describe('TosConsentBannerComponent', () => { let component: TosConsentBannerComponent; let fixture: ComponentFixture; let store: jest.Mocked; - let toastServiceMock: ReturnType; beforeEach(async () => { await TestBed.configureTestingModule({ - imports: [TosConsentBannerComponent, OSFTestingStoreModule, OSFTestingModule, MockComponent(IconComponent)], + imports: [TosConsentBannerComponent, OSFTestingStoreModule, MockComponent(IconComponent)], providers: [ provideMockStore({ signals: [{ selector: UserSelectors.getCurrentUser, value: MOCK_USER }], @@ -40,13 +39,11 @@ describe('TosConsentBannerComponent', () => { store = TestBed.inject(Store) as jest.Mocked; component = fixture.componentInstance; store.dispatch = jest.fn().mockReturnValue(of(undefined)); - toastServiceMock = ToastServiceMockBuilder.create().build(); fixture.detectChanges(); }); it('should have the "Continue" button disabled by default', () => { const continueButton = fixture.debugElement.query(By.css('p-button button')).nativeElement; - console.log('continueButton ' + continueButton ) expect(continueButton.disabled).toBe(true); }); @@ -68,16 +65,26 @@ describe('TosConsentBannerComponent', () => { fixture.detectChanges(); expect(store.dispatch).toHaveBeenCalledWith(new AcceptTermsOfServiceByUser()); - expect(toastServiceMock.showError).not.toHaveBeenCalled(); }); - it('should show toast banner if acceptedTermsOfService is false and "Continue" is clicked', () => { - component.acceptedTermsOfService.set(false); - const continueButton = fixture.debugElement.query(By.css('p-button button')).nativeElement; - continueButton.disabled = false; - continueButton.click(); - fixture.detectChanges(); - expect(component.errorMessage).toEqual('toast.tos-consent.errorMessage'); - }); + it('should return true for "acceptedTermsOfServiceChange" when user is null to not show banner', async () => { + await TestBed.resetTestingModule() + .configureTestingModule({ + imports: [TosConsentBannerComponent, OSFTestingStoreModule, MockComponent(IconComponent)], + providers: [ + provideMockStore({ + signals: [{ selector: UserSelectors.getCurrentUser, value: null }], + }), + TranslationServiceMock, + ], + }) + .compileComponents(); + + const fixture = TestBed.createComponent(TosConsentBannerComponent); + const component = fixture.componentInstance; + + fixture.detectChanges(); + expect(component.acceptedTermsOfServiceChange()).toBe(true); + }); }); diff --git a/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.ts b/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.ts index b7c511099..ebbaff9dc 100644 --- a/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.ts +++ b/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.ts @@ -12,7 +12,6 @@ import { RouterLink } from '@angular/router'; import { AcceptTermsOfServiceByUser, UserSelectors } from '@core/store/user'; import { IconComponent } from '@osf/shared/components'; -import { ToastService } from '@osf/shared/services'; /** * TosConsentBannerComponent displays a Terms of Service (ToS) consent banner for users who haven't accepted yet. @@ -35,9 +34,6 @@ import { ToastService } from '@osf/shared/services'; templateUrl: './tos-consent-banner.component.html', }) export class TosConsentBannerComponent { - private readonly toastService = inject(ToastService); - private readonly translateService = inject(TranslateService); - /** * NGXS dispatch map for the AcceptTermsOfServiceByUser action. */ @@ -52,7 +48,6 @@ export class TosConsentBannerComponent { * Local signal tracking whether the user has accepted the Terms of Service via checkbox. */ acceptedTermsOfService = signal(false); - errorMessage: string | null = null; /** * Computed signal indicating whether the user has already accepted the Terms of Service. @@ -74,13 +69,6 @@ export class TosConsentBannerComponent { * - Dispatches `AcceptTermsOfServiceByUser` action otherwise. */ onContinue() { - if (!this.acceptedTermsOfService()) { - this.errorMessage = this.translateService.instant('toast.tos-consent.errorMessage'); - this.toastService.showError(this.errorMessage as string); - return; - } - - this.errorMessage = null; this.actions.acceptTermsOfServiceByUser(); } } From ab49ee8d6c94c9a358a55294ddf66e3ce4784a86 Mon Sep 17 00:00:00 2001 From: mkovalua Date: Wed, 24 Sep 2025 20:49:14 +0300 Subject: [PATCH 3/3] fix(tos banner): resolve CR comments --- .../tos-consent-banner/tos-consent-banner.component.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.ts b/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.ts index ebbaff9dc..235a97f4f 100644 --- a/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.ts +++ b/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.ts @@ -1,6 +1,6 @@ import { createDispatchMap, select } from '@ngxs/store'; -import { TranslatePipe, TranslateService } from '@ngx-translate/core'; +import { TranslatePipe } from '@ngx-translate/core'; import { Button } from 'primeng/button'; import { Checkbox } from 'primeng/checkbox'; @@ -59,7 +59,7 @@ export class TosConsentBannerComponent { * if user is authenticated we check whether is accepted terms of service to hide banner or show if not * otherwise user is not authenticated we hide banner always */ - return user ? user?.acceptedTermsOfService : true; + return user ? user.acceptedTermsOfService : true; } );