Fix end-user agreement checkbox reset and navigation hang

src/app/core/end-user-agreement/end-user-agreement-current-user.guard.spec.ts

@@ -12,6 +12,7 @@ import {
   of,
 } from 'rxjs';
 
+import { AuthService } from '../auth/auth.service';
 import { EndUserAgreementService } from './end-user-agreement.service';
 import { endUserAgreementCurrentUserGuard } from './end-user-agreement-current-user.guard';
 
@@ -21,9 +22,20 @@ describe('endUserAgreementGuard', () => {
   let environment: AppConfig;
 
   beforeEach(() => {
-    endUserAgreementService = jasmine.createSpyObj('endUserAgreementService', {
-      hasCurrentUserAcceptedAgreement: of(true),
-    });
+    endUserAgreementService = jasmine.createSpyObj('EndUserAgreementService', [
+      'hasCurrentUserAcceptedAgreement',
+      'hasCurrentUserOrCookieAcceptedAgreement',
+      'removeCookieAccepted',
+      'setUserAcceptedAgreement',
+      'isCookieAccepted',
+      'setCookieAccepted',
+    ]);
+
+    (endUserAgreementService.hasCurrentUserAcceptedAgreement as jasmine.Spy).and.returnValue(of(true));
+    (endUserAgreementService.hasCurrentUserOrCookieAcceptedAgreement as jasmine.Spy).and.returnValue(of(true));
+    (endUserAgreementService.setUserAcceptedAgreement as jasmine.Spy).and.returnValue(of(true));
+    (endUserAgreementService.removeCookieAccepted as jasmine.Spy).and.callFake(() => {});
+    (endUserAgreementService.setCookieAccepted as jasmine.Spy).and.callFake(() => {});
 
     router = jasmine.createSpyObj('router', {
       navigateByUrl: {},
@@ -40,6 +52,7 @@ describe('endUserAgreementGuard', () => {
         { provide: Router, useValue: router },
         { provide: EndUserAgreementService, useValue: endUserAgreementService },
         { provide: APP_CONFIG, useValue: environment },
+        { provide: AuthService, useValue: jasmine.createSpyObj('AuthService', { isAuthenticated: of(true) }) },
       ],
     });
 
@@ -61,7 +74,7 @@ describe('endUserAgreementGuard', () => {
 
     describe('when the user hasn\'t accepted the agreement', () => {
       beforeEach(() => {
-        (endUserAgreementService.hasCurrentUserAcceptedAgreement as jasmine.Spy).and.returnValue(of(false));
+        (endUserAgreementService.hasCurrentUserOrCookieAcceptedAgreement as jasmine.Spy).and.returnValue(of(false));
       });
 
       it('should return a UrlTree', (done) => {

src/app/core/end-user-agreement/end-user-agreement-current-user.guard.ts

@@ -1,25 +1,36 @@
 import { inject } from '@angular/core';
-import { CanActivateFn } from '@angular/router';
+import {
+  CanActivateFn,
+  Router,
+  UrlTree,
+} from '@angular/router';
 import { APP_CONFIG } from '@dspace/config/app-config.interface';
 import { of } from 'rxjs';
+import {
+  map,
+  take,
+} from 'rxjs/operators';
 
-import { endUserAgreementGuard } from './end-user-agreement.guard';
 import { EndUserAgreementService } from './end-user-agreement.service';
 
+export const endUserAgreementCurrentUserGuard: CanActivateFn = (route, state) => {
+  const endUserAgreementService = inject(EndUserAgreementService);
+  const router = inject(Router);
+  const config = inject(APP_CONFIG);
 
-/**
- * Guard for preventing unauthorized access to certain pages
- * requiring the end user agreement to have been accepted by the current user
+  if (!config.info.enableEndUserAgreement) {
+    return of(true);
+  }
 
- */
-export const endUserAgreementCurrentUserGuard: CanActivateFn =
-  endUserAgreementGuard(
-    () => {
-      const endUserAgreementService = inject(EndUserAgreementService);
-      if (!inject(APP_CONFIG).info.enableEndUserAgreement) {
-        return of(true);
+  return endUserAgreementService.hasCurrentUserOrCookieAcceptedAgreement(false).pipe(
+    map(accepted => {
+      if (accepted) {
+        return true;
       }
-
-      return endUserAgreementService.hasCurrentUserAcceptedAgreement(true);
-    },
+      return router.createUrlTree(['/end-user-agreement'], {
+        queryParams: { redirect: state.url },
+      }) as UrlTree;
+    }),
+    take(1),
   );
+};

src/app/core/end-user-agreement/end-user-agreement.service.ts

@@ -35,11 +35,16 @@ export class EndUserAgreementService {
    *                              currently not authenticated (anonymous)
    */
   hasCurrentUserOrCookieAcceptedAgreement(acceptedWhenAnonymous: boolean): Observable<boolean> {
-    if (this.isCookieAccepted()) {
-      return of(true);
-    } else {
-      return this.hasCurrentUserAcceptedAgreement(acceptedWhenAnonymous);
-    }
+    return this.authService.isAuthenticated().pipe(
+      switchMap((authenticated) => {
+        if (authenticated) {
+          return this.hasCurrentUserAcceptedAgreement(acceptedWhenAnonymous);
+        } else {
+          return of(this.isCookieAccepted());
+        }
+      }),
+      take(1),
+    );
   }
 
   /**
@@ -52,12 +57,18 @@ export class EndUserAgreementService {
       switchMap((authenticated) => {
         if (authenticated) {
           return this.authService.getAuthenticatedUserFromStore().pipe(
-            map((user) => hasValue(user) && user.hasMetadata(END_USER_AGREEMENT_METADATA_FIELD) && user.firstMetadata(END_USER_AGREEMENT_METADATA_FIELD).value === 'true'),
+            take(1),
+            map((user) =>
+              hasValue(user) &&
+              user.hasMetadata(END_USER_AGREEMENT_METADATA_FIELD) &&
+              user.firstMetadata(END_USER_AGREEMENT_METADATA_FIELD).value === 'true',
+            ),
           );
         } else {
           return of(acceptedWhenAnonymous);
         }
       }),
+      take(1),
     );
   }
 
@@ -84,7 +95,13 @@ export class EndUserAgreementService {
               return this.ePersonService.patch(user, [operation]);
             }),
             getFirstCompletedRemoteData(),
-            map((response) => response.hasSucceeded),
+            map((response) => {
+              const success = response.hasSucceeded;
+              if (success) {
+                this.setCookieAccepted(true);
+              }
+              return success;
+            }),
           );
         } else {
           this.setCookieAccepted(accepted);

src/app/info/end-user-agreement/end-user-agreement.component.spec.ts

@@ -41,12 +41,14 @@ describe('EndUserAgreementComponent', () => {
     endUserAgreementService = jasmine.createSpyObj('endUserAgreementService', {
       hasCurrentUserOrCookieAcceptedAgreement: of(false),
       setUserAcceptedAgreement: of(true),
+      removeCookieAccepted: jasmine.createSpy('removeCookieAccepted'),
     });
     notificationsService = jasmine.createSpyObj('notificationsService', ['success', 'error']);
-    authService = jasmine.createSpyObj('authService', {
-      isAuthenticated: of(true),
-    });
-    store = jasmine.createSpyObj('store', ['dispatch']);
+    authService = jasmine.createSpyObj('AuthService', ['isAuthenticated']);
+    (authService.isAuthenticated as jasmine.Spy).and.returnValue(of(true));
+
+    store = jasmine.createSpyObj('Store', ['dispatch']);
+    (store.dispatch as jasmine.Spy).and.callFake(() => {});
     router = jasmine.createSpyObj('router', ['navigate', 'navigateByUrl']);
     route = Object.assign(new ActivatedRouteStub(), {
       queryParams: of({

src/app/info/end-user-agreement/end-user-agreement.component.ts

@@ -69,7 +69,9 @@ export class EndUserAgreementComponent implements OnInit {
    * Initialize the "accepted" property of this component by checking if the current user has accepted it before
    */
   initAccepted() {
-    this.endUserAgreementService.hasCurrentUserOrCookieAcceptedAgreement(false).subscribe((accepted) => {
+    this.endUserAgreementService.hasCurrentUserOrCookieAcceptedAgreement(false).pipe(
+      take(1),
+    ).subscribe((accepted) => {
       this.accepted = accepted;
     });
   }
@@ -93,6 +95,8 @@ export class EndUserAgreementComponent implements OnInit {
     ).subscribe((redirectUrl) => {
       if (isNotEmpty(redirectUrl)) {
         this.router.navigateByUrl(decodeURIComponent(redirectUrl));
+      } else {
+        this.router.navigate(['/home']);
       }
     });
   }
@@ -105,6 +109,7 @@ export class EndUserAgreementComponent implements OnInit {
   cancel() {
     this.authService.isAuthenticated().pipe(take(1)).subscribe((authenticated) => {
       if (authenticated) {
+        this.endUserAgreementService.removeCookieAccepted();
         this.store.dispatch(new LogOutAction());
       } else {
         this.router.navigate(['home']);