[ENG-9974] Investigate cause of navigation issues on feature/pbs-25-24 branch

src/app/features/registries/pages/my-registrations/my-registrations.component.html

@@ -10,7 +10,7 @@
   </div>
 
   <div class="flex-column flex flex-1 w-full">
-    <p-tabs [value]="selectedTab()" (valueChange)="selectedTab.set(+$event)" class="flex-1">
+    <p-tabs [value]="selectedTab()" (valueChange)="onTabChange(+$event)" class="flex-1">
       @if (!isMobile()) {
         <p-tablist class="px-4">
           @for (tab of tabOptions; track tab.value) {
@@ -25,7 +25,8 @@
             class="block mb-4"
             [options]="tabOptions"
             [fullWidth]="true"
-            [(selectedValue)]="selectedTab"
+            [selectedValue]="selectedTab()"
+            (selectedValueChange)="onTabChange($event)"
           ></osf-select>
         }
 

src/app/features/registries/pages/my-registrations/my-registrations.component.spec.ts

@@ -1,9 +1,12 @@
 import { MockComponents, MockProvider } from 'ng-mocks';
 
+import { of } from 'rxjs';
+
 import { ComponentFixture, TestBed } from '@angular/core/testing';
 import { ActivatedRoute, Router } from '@angular/router';
 
 import { UserSelectors } from '@core/store/user';
+import { RegistrationTab } from '@osf/features/registries/enums';
 import { RegistriesSelectors } from '@osf/features/registries/store';
 import { CustomPaginatorComponent } from '@osf/shared/components/custom-paginator/custom-paginator.component';
 import { RegistrationCardComponent } from '@osf/shared/components/registration-card/registration-card.component';
@@ -23,6 +26,8 @@ describe('MyRegistrationsComponent', () => {
   let fixture: ComponentFixture<MyRegistrationsComponent>;
   let mockRouter: ReturnType<RouterMockBuilder['build']>;
   let mockActivatedRoute: Partial<ActivatedRoute>;
+  let customConfirmationService: jest.Mocked<CustomConfirmationService>;
+  let toastService: jest.Mocked<ToastService>;
 
   beforeEach(async () => {
     mockRouter = RouterMockBuilder.create().withUrl('/registries/me').build();
@@ -55,32 +60,101 @@ describe('MyRegistrationsComponent', () => {
 
     fixture = TestBed.createComponent(MyRegistrationsComponent);
     component = fixture.componentInstance;
+    customConfirmationService = TestBed.inject(CustomConfirmationService) as jest.Mocked<CustomConfirmationService>;
+    toastService = TestBed.inject(ToastService) as jest.Mocked<ToastService>;
     fixture.detectChanges();
   });
 
   it('should create', () => {
     expect(component).toBeTruthy();
   });
 
-  it('should default to submitted tab and fetch submitted registrations', () => {
+  it('should default to submitted tab when no query param', () => {
+    expect(component.selectedTab()).toBe(RegistrationTab.Submitted);
+  });
+
+  it('should switch to drafts tab when query param is drafts', () => {
+    (mockActivatedRoute.snapshot as any).queryParams = { tab: 'drafts' };
+
+    fixture = TestBed.createComponent(MyRegistrationsComponent);
+    component = fixture.componentInstance;
+    fixture.detectChanges();
+
+    expect(component.selectedTab()).toBe(RegistrationTab.Drafts);
+  });
+
+  it('should switch to submitted tab when query param is submitted', () => {
+    (mockActivatedRoute.snapshot as any).queryParams = { tab: 'submitted' };
+
+    fixture = TestBed.createComponent(MyRegistrationsComponent);
+    component = fixture.componentInstance;
+    fixture.detectChanges();
+
+    expect(component.selectedTab()).toBe(RegistrationTab.Submitted);
+  });
+
+  it('should handle tab change and update query params', () => {
     const actionsMock = {
       getDraftRegistrations: jest.fn(),
       getSubmittedRegistrations: jest.fn(),
       deleteDraft: jest.fn(),
     } as any;
     Object.defineProperty(component, 'actions', { value: actionsMock });
+    const navigateSpy = jest.spyOn(mockRouter, 'navigate');
+
+    component.onTabChange(RegistrationTab.Drafts);
+
+    expect(component.selectedTab()).toBe(RegistrationTab.Drafts);
+    expect(component.draftFirst).toBe(0);
+    expect(actionsMock.getDraftRegistrations).toHaveBeenCalledWith();
+    expect(navigateSpy).toHaveBeenCalledWith([], {
+      relativeTo: mockActivatedRoute,
+      queryParams: { tab: 'drafts' },
+      queryParamsHandling: 'merge',
+    });
+  });
 
-    component.selectedTab.set(component.RegistrationTab.Drafts);
-    fixture.detectChanges();
-    component.selectedTab.set(component.RegistrationTab.Submitted);
-    fixture.detectChanges();
-    expect(actionsMock.getSubmittedRegistrations).toHaveBeenCalledWith('user-1');
+  it('should handle tab change to submitted and update query params', () => {
+    const actionsMock = {
+      getDraftRegistrations: jest.fn(),
+      getSubmittedRegistrations: jest.fn(),
+      deleteDraft: jest.fn(),
+    } as any;
+    Object.defineProperty(component, 'actions', { value: actionsMock });
+    const navigateSpy = jest.spyOn(mockRouter, 'navigate');
+
+    component.onTabChange(RegistrationTab.Submitted);
+
+    expect(component.selectedTab()).toBe(RegistrationTab.Submitted);
+    expect(component.submittedFirst).toBe(0);
+    expect(actionsMock.getSubmittedRegistrations).toHaveBeenCalledWith();
+    expect(navigateSpy).toHaveBeenCalledWith([], {
+      relativeTo: mockActivatedRoute,
+      queryParams: { tab: 'submitted' },
+      queryParamsHandling: 'merge',
+    });
+  });
+
+  it('should not process tab change if tab is not a number', () => {
+    const actionsMock = {
+      getDraftRegistrations: jest.fn(),
+      getSubmittedRegistrations: jest.fn(),
+      deleteDraft: jest.fn(),
+    } as any;
+    Object.defineProperty(component, 'actions', { value: actionsMock });
+    const initialTab = component.selectedTab();
+
+    component.onTabChange('invalid' as any);
+
+    expect(component.selectedTab()).toBe(initialTab);
+    expect(actionsMock.getDraftRegistrations).not.toHaveBeenCalled();
+    expect(actionsMock.getSubmittedRegistrations).not.toHaveBeenCalled();
   });
 
   it('should navigate to create registration page', () => {
-    const navSpy = jest.spyOn(TestBed.inject(Router), 'navigate');
+    const navSpy = jest.spyOn(mockRouter, 'navigate');
     component.goToCreateRegistration();
-    expect(navSpy).toHaveBeenCalledWith(['/registries/osf/new']);
+    expect(navSpy).toHaveBeenLastCalledWith(['/registries', 'osf', 'new']);
   });
 
   it('should handle drafts pagination', () => {
@@ -95,23 +169,75 @@ describe('MyRegistrationsComponent', () => {
     const actionsMock = { getSubmittedRegistrations: jest.fn() } as any;
     Object.defineProperty(component, 'actions', { value: actionsMock });
     component.onSubmittedPageChange({ page: 1, first: 10 } as any);
-    expect(actionsMock.getSubmittedRegistrations).toHaveBeenCalledWith('user-1', 2);
+    expect(actionsMock.getSubmittedRegistrations).toHaveBeenCalledWith(2);
     expect(component.submittedFirst).toBe(10);
   });
 
-  it('should switch to drafts tab based on query param and fetch drafts', async () => {
-    (mockActivatedRoute.snapshot as any).queryParams = { tab: 'drafts' };
-    const actionsMock = { getDraftRegistrations: jest.fn(), getSubmittedRegistrations: jest.fn() } as any;
-    fixture = TestBed.createComponent(MyRegistrationsComponent);
-    component = fixture.componentInstance;
+  it('should delete draft after confirmation', () => {
+    const actionsMock = {
+      getDraftRegistrations: jest.fn(),
+      getSubmittedRegistrations: jest.fn(),
+      deleteDraft: jest.fn(() => of({})),
+    } as any;
     Object.defineProperty(component, 'actions', { value: actionsMock });
-    fixture.detectChanges();
-
-    expect(component.selectedTab()).toBe(0);
-    component.selectedTab.set(component.RegistrationTab.Submitted);
-    fixture.detectChanges();
-    component.selectedTab.set(component.RegistrationTab.Drafts);
-    fixture.detectChanges();
+    customConfirmationService.confirmDelete.mockImplementation(({ onConfirm }) => {
+      onConfirm();
+    });
+
+    component.onDeleteDraft('draft-123');
+
+    expect(customConfirmationService.confirmDelete).toHaveBeenCalledWith({
+      headerKey: 'registries.deleteDraft',
+      messageKey: 'registries.confirmDeleteDraft',
+      onConfirm: expect.any(Function),
+    });
+    expect(actionsMock.deleteDraft).toHaveBeenCalledWith('draft-123');
     expect(actionsMock.getDraftRegistrations).toHaveBeenCalled();
+    expect(toastService.showSuccess).toHaveBeenCalledWith('registries.successDeleteDraft');
+  });
+
+  it('should not delete draft if confirmation is cancelled', () => {
+    const actionsMock = {
+      getDraftRegistrations: jest.fn(),
+      getSubmittedRegistrations: jest.fn(),
+      deleteDraft: jest.fn(),
+    } as any;
+    Object.defineProperty(component, 'actions', { value: actionsMock });
+    customConfirmationService.confirmDelete.mockImplementation(() => {});
+
+    component.onDeleteDraft('draft-123');
+
+    expect(customConfirmationService.confirmDelete).toHaveBeenCalled();
+    expect(actionsMock.deleteDraft).not.toHaveBeenCalled();
+    expect(actionsMock.getDraftRegistrations).not.toHaveBeenCalled();
+    expect(toastService.showSuccess).not.toHaveBeenCalled();
+  });
+
+  it('should reset draftFirst when switching to drafts tab', () => {
+    component.draftFirst = 20;
+    const actionsMock = {
+      getDraftRegistrations: jest.fn(),
+      getSubmittedRegistrations: jest.fn(),
+      deleteDraft: jest.fn(),
+    } as any;
+    Object.defineProperty(component, 'actions', { value: actionsMock });
+
+    component.onTabChange(RegistrationTab.Drafts);
+
+    expect(component.draftFirst).toBe(0);
+  });
+
+  it('should reset submittedFirst when switching to submitted tab', () => {
+    component.submittedFirst = 20;
+    const actionsMock = {
+      getDraftRegistrations: jest.fn(),
+      getSubmittedRegistrations: jest.fn(),
+      deleteDraft: jest.fn(),
+    } as any;
+    Object.defineProperty(component, 'actions', { value: actionsMock });
+
+    component.onTabChange(RegistrationTab.Submitted);
+
+    expect(component.submittedFirst).toBe(0);
   });
 });

src/app/features/registries/pages/my-registrations/my-registrations.component.ts

@@ -8,18 +8,18 @@ import { Skeleton } from 'primeng/skeleton';
 import { TabsModule } from 'primeng/tabs';
 
 import { NgTemplateOutlet } from '@angular/common';
-import { ChangeDetectionStrategy, Component, effect, inject, signal } from '@angular/core';
+import { ChangeDetectionStrategy, Component, inject, signal } from '@angular/core';
 import { toSignal } from '@angular/core/rxjs-interop';
 import { FormsModule } from '@angular/forms';
 import { ActivatedRoute, Router, RouterLink } from '@angular/router';
 
 import { ENVIRONMENT } from '@core/provider/environment.provider';
-import { UserSelectors } from '@core/store/user';
 import { CustomPaginatorComponent } from '@osf/shared/components/custom-paginator/custom-paginator.component';
 import { RegistrationCardComponent } from '@osf/shared/components/registration-card/registration-card.component';
 import { SelectComponent } from '@osf/shared/components/select/select.component';
 import { SubHeaderComponent } from '@osf/shared/components/sub-header/sub-header.component';
 import { IS_XSMALL } from '@osf/shared/helpers/breakpoints.tokens';
+import { Primitive } from '@osf/shared/helpers/types.helper';
 import { CustomConfirmationService } from '@osf/shared/services/custom-confirmation.service';
 import { ToastService } from '@osf/shared/services/toast.service';
 
@@ -57,7 +57,6 @@ export class MyRegistrationsComponent {
   readonly isMobile = toSignal(inject(IS_XSMALL));
   readonly tabOptions = REGISTRATIONS_TABS;
 
-  private currentUser = select(UserSelectors.getCurrentUser);
   draftRegistrations = select(RegistriesSelectors.getDraftRegistrations);
   draftRegistrationsTotalCount = select(RegistriesSelectors.getDraftRegistrationsTotalCount);
   isDraftRegistrationsLoading = select(RegistriesSelectors.isDraftRegistrationsLoading);
@@ -83,33 +82,37 @@ export class MyRegistrationsComponent {
 
   constructor() {
     const initialTab = this.route.snapshot.queryParams['tab'];
-    if (initialTab == 'drafts') {
-      this.selectedTab.set(RegistrationTab.Drafts);
-    } else {
-      this.selectedTab.set(RegistrationTab.Submitted);
+    const selectedTab = initialTab == 'drafts' ? RegistrationTab.Drafts : RegistrationTab.Submitted;
+    this.onTabChange(selectedTab);
+  }
+
+  onTabChange(tab: Primitive): void {
+    if (typeof tab !== 'number') {
+      return;
     }
 
-    effect(() => {
-      const tab = this.selectedTab();
-
-      if (tab === 0) {
-        this.draftFirst = 0;
-        this.actions.getDraftRegistrations();
-      } else {
-        this.submittedFirst = 0;
-        this.actions.getSubmittedRegistrations(this.currentUser()?.id);
-      }
-
-      this.router.navigate([], {
-        relativeTo: this.route,
-        queryParams: { tab: tab === RegistrationTab.Drafts ? 'drafts' : 'submitted' },
-        queryParamsHandling: 'merge',
-      });
+    this.selectedTab.set(tab);
+    this.loadTabData(tab);
+
+    this.router.navigate([], {
+      relativeTo: this.route,
+      queryParams: { tab: tab === RegistrationTab.Drafts ? 'drafts' : 'submitted' },
+      queryParamsHandling: 'merge',
     });
   }
 
+  private loadTabData(tab: number): void {
+    if (tab === RegistrationTab.Drafts) {
+      this.draftFirst = 0;
+      this.actions.getDraftRegistrations();
+    } else {
+      this.submittedFirst = 0;
+      this.actions.getSubmittedRegistrations();
+    }
+  }
+
   goToCreateRegistration(): void {
-    this.router.navigate([`/registries/${this.provider}/new`]);
+    this.router.navigate(['/registries', this.provider, 'new']);
   }
 
   onDeleteDraft(id: string): void {
@@ -133,7 +136,7 @@ export class MyRegistrationsComponent {
   }
 
   onSubmittedPageChange(event: PaginatorState): void {
-    this.actions.getSubmittedRegistrations(this.currentUser()?.id, event.page! + 1);
+    this.actions.getSubmittedRegistrations(event.page! + 1);
     this.submittedFirst = event.first!;
   }
 }

src/app/features/registries/store/registries.actions.ts

@@ -111,7 +111,6 @@ export class FetchSubmittedRegistrations {
   static readonly type = '[Registries] Fetch Submitted Registrations';
 
   constructor(
-    public userId: string | undefined,
     public page = 1,
     public pageSize = 10
   ) {}

src/app/features/registries/store/registries.state.ts

@@ -1,10 +1,11 @@
-import { Action, State, StateContext } from '@ngxs/store';
+import { Action, State, StateContext, Store } from '@ngxs/store';
 
 import { catchError, tap } from 'rxjs/operators';
 
 import { inject, Injectable } from '@angular/core';
 
 import { ENVIRONMENT } from '@core/provider/environment.provider';
+import { UserSelectors } from '@core/store/user';
 import { ResourceType } from '@osf/shared/enums/resource-type.enum';
 import { getResourceTypeStringFromEnum } from '@osf/shared/helpers/get-resource-types.helper';
 import { handleSectionError } from '@osf/shared/helpers/state-error.handler';
@@ -56,6 +57,7 @@ export class RegistriesState {
   searchService = inject(GlobalSearchService);
   registriesService = inject(RegistriesService);
   private readonly environment = inject(ENVIRONMENT);
+  private readonly store = inject(Store);
 
   providersHandler = inject(ProvidersHandlers);
   projectsHandler = inject(ProjectsHandlers);
@@ -311,18 +313,20 @@ export class RegistriesState {
   @Action(FetchSubmittedRegistrations)
   fetchSubmittedRegistrations(
     ctx: StateContext<RegistriesStateModel>,
-    { userId, page, pageSize }: FetchSubmittedRegistrations
+    { page, pageSize }: FetchSubmittedRegistrations
   ) {
     const state = ctx.getState();
+    const currentUser = this.store.selectSnapshot(UserSelectors.getCurrentUser);
+
     ctx.patchState({
       submittedRegistrations: { ...state.submittedRegistrations, isLoading: true, error: null },
     });
 
-    if (!userId) {
+    if (!currentUser) {
       return;
     }
 
-    return this.registriesService.getSubmittedRegistrations(userId, page, pageSize).pipe(
+    return this.registriesService.getSubmittedRegistrations(currentUser.id, page, pageSize).pipe(
       tap((submittedRegistrations) => {
         ctx.patchState({
           submittedRegistrations: {