feat: Enhance download flow handling by refactoring callback parsing and routing to AppComponent

Author: ErykKul

src/app/app.component.ts

@@ -3,6 +3,7 @@
 import { Component, OnInit, inject } from '@angular/core';
 import { PrimeNG } from 'primeng/config';
 import { DataService } from './data.service';
+import { DatasetService } from './dataset.service';
 import { ActivatedRoute, Router, RouterOutlet } from '@angular/router';
 import { PluginService } from './plugin.service';
 
@@ -15,6 +16,7 @@ import { PluginService } from './plugin.service';
 export class AppComponent implements OnInit {
   private primengConfig = inject(PrimeNG);
   dataService = inject(DataService);
+  private datasetService = inject(DatasetService);
   private router = inject(Router);
   private route = inject(ActivatedRoute);
   private pluginService = inject(PluginService);
@@ -54,10 +56,12 @@ export class AppComponent implements OnInit {
   /**
    * Checks if the query params indicate a download flow that should skip login redirect.
    * Download flows allow guest access with Globus OAuth only.
+   * Also handles navigation to /download when URL contains /download but route doesn't match.
    */
   private isDownloadFlow(params: Record<string, string | undefined>): boolean {
     // For testing: allow overriding window.location.href
     const locationHref =
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
       (this as any)._testWindowLocationHref ?? window.location.href;
 
     // eslint-disable-next-line no-console
@@ -67,13 +71,26 @@ export class AppComponent implements OnInit {
       windowLocation: locationHref,
     });
 
-    // Check if we're on the download page (via router or window.location)
+    // Check if we're on the download page via router
+    if (this.router.url.includes('/download')) {
+      // eslint-disable-next-line no-console
+      console.debug(
+        '[AppComponent] isDownloadFlow: true (router matches /download)',
+      );
+      return true;
+    }
+
+    // Check if URL contains /download but Angular routing failed (e.g., /connect/download)
+    // In this case, parse the callback and navigate to /download with the correct params
     if (
-      this.router.url.includes('/download') ||
-      locationHref.includes('/download')
+      locationHref.includes('/download') &&
+      !this.router.url.includes('/download')
     ) {
       // eslint-disable-next-line no-console
-      console.debug('[AppComponent] isDownloadFlow: true (download page)');
+      console.debug(
+        '[AppComponent] URL contains /download but route does not match, redirecting to /download',
+      );
+      this.redirectToDownload(locationHref);
       return true;
     }
 
@@ -139,6 +156,90 @@ export class AppComponent implements OnInit {
     return false;
   }
 
+  /**
+   * Parses the Globus callback from the URL and redirects to /download with the correct params.
+   * The callback is a base64-encoded URL like:
+   * https://example.com/api/v1/datasets/{datasetDbId}/globusDownloadParameters?locale=en&downloadId={uuid}
+   */
+  private redirectToDownload(locationHref: string): void {
+    try {
+      const url = new URL(locationHref);
+      const callback = url.searchParams.get('callback');
+
+      if (!callback) {
+        // No callback, just navigate to /download
+        this.router.navigate(['/download']);
+        return;
+      }
+
+      const callbackUrl = atob(callback);
+      const parts = callbackUrl.split('/');
+      if (parts.length <= 6) {
+        // eslint-disable-next-line no-console
+        console.warn(
+          '[AppComponent] Invalid callback URL format:',
+          callbackUrl,
+        );
+        this.router.navigate(['/download']);
+        return;
+      }
+
+      // Extract datasetDbId from URL (position 6 in the path)
+      const datasetDbId = parts[6];
+
+      // Extract downloadId from query params
+      let downloadId: string | undefined;
+      const queryString = callbackUrl.split('?')[1];
+      if (queryString) {
+        const globusParams = queryString.split('&');
+        for (const p of globusParams) {
+          if (p.startsWith('downloadId=')) {
+            downloadId = p.substring('downloadId='.length);
+            break;
+          }
+        }
+      }
+
+      // Get the persistent ID (DOI) from the dataset database ID
+      const dvToken = localStorage.getItem('dataverseToken');
+      this.datasetService
+        .getDatasetVersion(datasetDbId, dvToken ?? undefined)
+        .subscribe({
+          next: (x) => {
+            // eslint-disable-next-line no-console
+            console.debug('[AppComponent] Redirecting to /download with:', {
+              downloadId,
+              datasetPid: x.persistentId,
+            });
+            this.router.navigate(['/download'], {
+              queryParams: {
+                downloadId: downloadId,
+                datasetPid: x.persistentId,
+                apiToken: dvToken,
+              },
+            });
+          },
+          error: (err) => {
+            // eslint-disable-next-line no-console
+            console.error('[AppComponent] Failed to get dataset version:', err);
+            // Navigate anyway with what we have
+            this.router.navigate(['/download'], {
+              queryParams: {
+                downloadId: downloadId,
+              },
+            });
+          },
+        });
+    } catch (e) {
+      // eslint-disable-next-line no-console
+      console.error(
+        '[AppComponent] Failed to parse URL for download redirect:',
+        e,
+      );
+      this.router.navigate(['/download']);
+    }
+  }
+
   private static readonly REDIRECT_STORAGE_KEY = 'loginRedirectAttempt';
   private static readonly MAX_REDIRECTS = 2;
   private static readonly REDIRECT_WINDOW_MS = 30000; // 30 seconds

src/app/connect/connect.component.advanced.spec.ts

@@ -5,7 +5,7 @@ import {
 import { provideHttpClientTesting } from '@angular/common/http/testing';
 import { fakeAsync, TestBed, tick } from '@angular/core/testing';
 import { provideNoopAnimations } from '@angular/platform-browser/animations';
-import { ActivatedRoute, provideRouter, Router } from '@angular/router';
+import { ActivatedRoute, provideRouter } from '@angular/router';
 import { SelectItem, TreeNode } from 'primeng/api';
 import { Observable, of } from 'rxjs';
 import { DataStateService } from '../data.state.service';
@@ -370,10 +370,8 @@ describe('ConnectComponent advanced behaviors', () => {
     expect(err2).toBe('Malformed source url');
   });
 
-  it('handleGlobusCallback restores dataset and navigates to download', fakeAsync(() => {
+  it('handleGlobusCallback restores dataset from callback URL', fakeAsync(() => {
     const { comp } = createComponent();
-    const router = TestBed.inject(Router);
-    const navSpy = spyOn(router, 'navigate');
     const callback = btoa(
       'https://globus.example/api/files/visit/12345/details?downloadId=DLID&x=1',
     );
@@ -384,13 +382,7 @@ describe('ConnectComponent advanced behaviors', () => {
       apiToken: 'apiTok',
     });
     expect(comp.datasetId).toBe('doi:10.123/RESTORED');
-    expect(navSpy).toHaveBeenCalledWith(['/download'], {
-      queryParams: {
-        downloadId: 'DLID',
-        datasetPid: 'doi:10.123/RESTORED',
-        apiToken: 'apiTok',
-      },
-    });
+    // Note: Download redirect is now handled by AppComponent, not connect component
   }));
 
   it('restoreFromOauthState populates selections and fetches token', fakeAsync(() => {

src/app/connect/connect.component.ts

@@ -292,22 +292,16 @@ export class ConnectComponent
   ): void {
     const p = this.pluginService.getGlobusPlugin();
     if (!p) return;
-    // Don't preselect plugin - only preselect the dataset
-    // this.plugin = 'globus';
-    // this.pluginId = 'globus';
+    // If datasetPid is explicitly provided, no need to parse callback
     if (datasetPid) return;
+
+    // Parse callback to extract dataset info for the connect page (upload flow)
+    // Download flows are now handled by AppComponent routing to /download directly
     const callbackUrl = atob(callback);
     const parts = callbackUrl.split('/');
     if (parts.length <= 6) return;
     const datasetDbId = parts[6];
-    const g = callbackUrl.split('?');
-    const globusParams = g[g.length - 1].split('&');
-    let downloadId: string | undefined = undefined;
-    globusParams.forEach((p) => {
-      if (p.startsWith('downloadId=')) {
-        downloadId = p.substring('downloadId='.length);
-      }
-    });
+
     const versionSubscription = this.datasetService
       .getDatasetVersion(datasetDbId, apiToken)
       .subscribe((x) => {
@@ -325,15 +319,6 @@ export class ConnectComponent
         }
         this.subscriptions.delete(versionSubscription);
         versionSubscription.unsubscribe();
-        if (downloadId) {
-          this.router.navigate(['/download'], {
-            queryParams: {
-              downloadId: downloadId,
-              datasetPid: x.persistentId,
-              apiToken: apiToken,
-            },
-          });
-        }
       });
     this.subscriptions.add(versionSubscription);
   }
@@ -349,8 +334,11 @@ export class ConnectComponent
     } catch {
       return; // malformed state
     }
+    // Download flows with state.download are normally caught by AppComponent
+    // and routed to /download. This is a fallback for edge cases.
     if (loginState.download) {
       this.router.navigate(['/download'], { queryParams: params });
+      return;
     }
     this.sourceUrl = loginState.sourceUrl;
     this.url = loginState.url;