94072: Group page list issue fixes

src/app/access-control/group-registry/group-form/group-form.component.ts

@@ -377,9 +377,7 @@ export class GroupFormComponent implements OnInit, OnDestroy {
    */
   setActiveGroup(groupId: string) {
     this.groupDataService.cancelEditGroup();
-    const nextGroup$ = this.groupDataService.findById(groupId);
-    this.subs.push(nextGroup$.subscribe());
-    nextGroup$
+    this.groupDataService.findById(groupId)
       .pipe(
         getFirstSucceededRemoteData(),
         getRemoteDataPayload())

src/app/access-control/group-registry/group-form/members-list/members-list.component.ts

@@ -10,7 +10,7 @@ import {
   combineLatest as observableCombineLatest,
   ObservedValueOf,
 } from 'rxjs';
-import { map, mergeMap, switchMap, take } from 'rxjs/operators';
+import { defaultIfEmpty, map, mergeMap, switchMap, take } from 'rxjs/operators';
 import {buildPaginatedList, PaginatedList} from '../../../../core/data/paginated-list.model';
 import { RemoteData } from '../../../../core/data/remote-data';
 import { EPersonDataService } from '../../../../core/eperson/eperson-data.service';
@@ -25,6 +25,7 @@ import { NotificationsService } from '../../../../shared/notifications/notificat
 import { PaginationComponentOptions } from '../../../../shared/pagination/pagination-component-options.model';
 import {EpersonDtoModel} from '../../../../core/eperson/models/eperson-dto.model';
 import { PaginationService } from '../../../../core/pagination/pagination.service';
+import { isNotEmpty } from '../../../../shared/empty.util';
 
 /**
  * Keys to keep track of specific subscriptions
@@ -144,7 +145,7 @@ export class MembersListComponent implements OnInit, OnDestroy {
         }
       }),
       switchMap((epersonListRD: RemoteData<PaginatedList<EPerson>>) => {
-        const dtos$ = observableCombineLatest(...epersonListRD.payload.page.map((member: EPerson) => {
+        const dtos$ = observableCombineLatest([...epersonListRD.payload.page.map((member: EPerson) => {
           const dto$: Observable<EpersonDtoModel> = observableCombineLatest(
             this.isMemberOfGroup(member), (isMember: ObservedValueOf<Observable<boolean>>) => {
               const epersonDtoModel: EpersonDtoModel = new EpersonDtoModel();
@@ -153,8 +154,8 @@ export class MembersListComponent implements OnInit, OnDestroy {
               return epersonDtoModel;
             });
           return dto$;
-        }));
-        return dtos$.pipe(map((dtos: EpersonDtoModel[]) => {
+        })]);
+        return dtos$.pipe(defaultIfEmpty([]), map((dtos: EpersonDtoModel[]) => {
           return buildPaginatedList(epersonListRD.payload.pageInfo, dtos);
         }));
       }))
@@ -274,7 +275,7 @@ export class MembersListComponent implements OnInit, OnDestroy {
           }
         }),
         switchMap((epersonListRD: RemoteData<PaginatedList<EPerson>>) => {
-          const dtos$ = observableCombineLatest(...epersonListRD.payload.page.map((member: EPerson) => {
+          const dtos$ = observableCombineLatest([...epersonListRD.payload.page.map((member: EPerson) => {
             const dto$: Observable<EpersonDtoModel> = observableCombineLatest(
               this.isMemberOfGroup(member), (isMember: ObservedValueOf<Observable<boolean>>) => {
                 const epersonDtoModel: EpersonDtoModel = new EpersonDtoModel();
@@ -283,8 +284,8 @@ export class MembersListComponent implements OnInit, OnDestroy {
                 return epersonDtoModel;
               });
             return dto$;
-          }));
-          return dtos$.pipe(map((dtos: EpersonDtoModel[]) => {
+          })]);
+          return dtos$.pipe(defaultIfEmpty([]), map((dtos: EpersonDtoModel[]) => {
             return buildPaginatedList(epersonListRD.payload.pageInfo, dtos);
           }));
         }))

src/app/core/cache/builders/remote-data-build.service.spec.ts

@@ -18,6 +18,7 @@ import { take } from 'rxjs/operators';
 import { HALLink } from '../../shared/hal-link.model';
 import { RequestEntryState } from '../../data/request-entry-state.model';
 import { RequestEntry } from '../../data/request-entry.model';
+import { cold } from 'jasmine-marbles';
 
 describe('RemoteDataBuildService', () => {
   let service: RemoteDataBuildService;
@@ -646,4 +647,21 @@ describe('RemoteDataBuildService', () => {
       });
     });
   });
+
+  describe('buildFromHref', () => {
+    beforeEach(() => {
+      (objectCache.getRequestUUIDBySelfLink as jasmine.Spy).and.returnValue(cold('a', { a: 'request/uuid' }));
+    });
+
+    describe('when both getRequestFromRequestHref and getRequestFromRequestUUID emit nothing', () => {
+      beforeEach(() => {
+        (requestService.getByHref as jasmine.Spy).and.returnValue(cold('a', { a: undefined }));
+        (requestService.getByUUID as jasmine.Spy).and.returnValue(cold('a', { a: undefined }));
+      });
+
+      it('should not emit anything', () => {
+        expect(service.buildFromHref(cold('a', { a: 'rest/api/endpoint' }))).toBeObservable(cold(''));
+      });
+    });
+  });
 });

src/app/core/cache/builders/remote-data-build.service.ts

@@ -3,9 +3,8 @@ import {
   combineLatest as observableCombineLatest,
   Observable,
   of as observableOf,
-  race as observableRace
 } from 'rxjs';
-import { map, switchMap, filter, distinctUntilKeyChanged } from 'rxjs/operators';
+import { map, switchMap, filter, distinctUntilKeyChanged, startWith } from 'rxjs/operators';
 import { hasValue, isEmpty, isNotEmpty, hasNoValue, isUndefined } from '../../../shared/empty.util';
 import { createSuccessfulRemoteDataObject$ } from '../../../shared/remote-data.utils';
 import { FollowLinkConfig, followLink } from '../../../shared/utils/follow-link-config.model';
@@ -21,7 +20,7 @@ import { HALResource } from '../../shared/hal-resource.model';
 import { PAGINATED_LIST } from '../../data/paginated-list.resource-type';
 import { getUrlWithoutEmbedParams } from '../../index/index.selectors';
 import { getResourceTypeValueFor } from '../object-cache.reducer';
-import { hasSucceeded, RequestEntryState } from '../../data/request-entry-state.model';
+import { hasSucceeded, isStale, RequestEntryState } from '../../data/request-entry-state.model';
 import { getRequestFromRequestHref, getRequestFromRequestUUID } from '../../shared/request.operators';
 import { RequestEntry } from '../../data/request-entry.model';
 import { ResponseState } from '../../data/response-state.model';
@@ -207,10 +206,27 @@ export class RemoteDataBuildService {
         this.objectCache.getRequestUUIDBySelfLink(href)),
     );
 
-    const requestEntry$ = observableRace(
-      href$.pipe(getRequestFromRequestHref(this.requestService)),
-      requestUUID$.pipe(getRequestFromRequestUUID(this.requestService)),
-    ).pipe(
+    const requestEntry$ = observableCombineLatest([
+      href$.pipe(getRequestFromRequestHref(this.requestService), startWith(undefined)),
+      requestUUID$.pipe(getRequestFromRequestUUID(this.requestService), startWith(undefined)),
+    ]).pipe(
+      filter(([r1, r2]) => hasValue(r1) || hasValue(r2)),
+      map(([r1, r2]) => {
+        // If one of the two requests has no value, return the other (both is impossible due to the filter above)
+        if (hasNoValue(r2)) {
+          return r1;
+        } else if (hasNoValue(r1)) {
+          return r2;
+        }
+
+        if ((isStale(r1.state) && isStale(r2.state)) || (!isStale(r1.state) && !isStale(r2.state))) {
+          // Neither or both are stale, pick the most recent request
+          return r1.lastUpdated >= r2.lastUpdated ? r1 : r2;
+        } else {
+          // One of the two is stale, return the not stale request
+          return isStale(r2.state) ? r1 : r2;
+        }
+      }),
       distinctUntilKeyChanged('lastUpdated')
     );
 

src/app/shared/mocks/object-cache.service.mock.ts

@@ -11,7 +11,8 @@ export function getMockObjectCacheService(): ObjectCacheService {
     'getRequestHrefByUUID',
     'getList',
     'hasByUUID',
-    'hasByHref'
+    'hasByHref',
+    'getRequestUUIDBySelfLink',
   ]);
 
 }