This is an automated email from the ASF dual-hosted git repository.

tbonelee pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git


The following commit(s) were added to refs/heads/master by this push:
     new 1d471c6c59 [ZEPPELIN-6423] Reload note when switching notebooks in the 
Angular UI
1d471c6c59 is described below

commit 1d471c6c59a3fe0fc362619db21d48a8c874064f
Author: ChanHo Lee <[email protected]>
AuthorDate: Wed Jun 3 01:02:16 2026 +0900

    [ZEPPELIN-6423] Reload note when switching notebooks in the Angular UI
    
    ### What is this PR for?
    Navigating between notes in the Angular UI changed the URL but left the 
page showing the previously opened note.
    
    The note fetch in `NotebookComponent` was bound only to the WebSocket 
`connectedStatus$` stream (introduced in ZEPPELIN-6387). Because Angular reuses 
`NotebookComponent` across `:noteId` route changes — so `ngOnInit` does not 
re-run — and the WebSocket stays connected, selecting another note from the 
header notebook list never re-fetched the note. The URL/route params updated, 
but `getNote()` was never called for the new `noteId`, so the page kept 
rendering the old note.
    
    This PR drives the fetch from a `combineLatest` of the connection status 
and the route params, so it fires on **both** a WebSocket (re)connect **and** a 
`noteId`/`revisionId` change. The reconnect-reload behavior from ZEPPELIN-6387 
is preserved; `distinctUntilChanged` on the connection stream avoids a 
redundant fetch on init.
    
    ### What type of PR is it?
    Bug Fix
    
    ### Todos
    * [x] Re-fetch the note on `noteId`/`revisionId` route changes
    * [x] Preserve WebSocket reconnect-reload behavior
    * [x] Add e2e regression test
    
    ### What is the Jira issue?
    * https://issues.apache.org/jira/browse/ZEPPELIN-6423
    
    ### How should this be tested?
    * **Automated:** 
`zeppelin-web-angular/e2e/tests/notebook/main/notebook-navigation.spec.ts` 
opens one note, then navigates to a second note via the header "Notebook" 
dropdown and asserts the displayed note title (not just the URL) updates. 
Verified failing before the fix and passing after, against a live backend.
    * **Manual:**
      1. Open a notebook.
      2. From the header **Notebook** dropdown, click a different note.
      3. The page content should switch to the newly selected note (previously 
it stayed on the old note while the URL changed).
    
    ### Screenshots (if appropriate)
    N/A
    
    ### Questions:
    * Does the license files need to update? No
    * Is there breaking changes for older versions? No
    * Does this needs documentation? No
    
    
    Closes #5259 from tbonelee/fix/notebook-reload-on-note-switch.
    
    Signed-off-by: ChanHo Lee <[email protected]>
---
 .../notebook/main/notebook-navigation.spec.ts      | 77 ++++++++++++++++++++++
 .../pages/workspace/notebook/notebook.component.ts | 46 +++++++------
 2 files changed, 103 insertions(+), 20 deletions(-)

diff --git 
a/zeppelin-web-angular/e2e/tests/notebook/main/notebook-navigation.spec.ts 
b/zeppelin-web-angular/e2e/tests/notebook/main/notebook-navigation.spec.ts
new file mode 100644
index 0000000000..db259de834
--- /dev/null
+++ b/zeppelin-web-angular/e2e/tests/notebook/main/notebook-navigation.spec.ts
@@ -0,0 +1,77 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { expect, Page, test } from '@playwright/test';
+import { HeaderPage } from '../../../models/header-page';
+import { HomePage } from '../../../models/home-page';
+import { addPageAnnotationBeforeEach, PAGES, performLoginIfRequired, 
waitForZeppelinReady } from '../../../utils';
+
+const noteIdFromUrl = (url: string): string => {
+  const match = url.match(/\/notebook\/([^/?]+)/);
+  if (!match) {
+    throw new Error(`Could not extract noteId from URL: ${url}`);
+  }
+  return match[1];
+};
+
+const createRootNote = async (page: Page, homePage: HomePage, name: string): 
Promise<string> => {
+  await page.goto('/#/');
+  await waitForZeppelinReady(page);
+  await homePage.createNote(name);
+  await page.waitForURL(/\/notebook\//, { timeout: 45000 });
+  return noteIdFromUrl(page.url());
+};
+
+test.describe('Notebook Navigation', () => {
+  addPageAnnotationBeforeEach(PAGES.WORKSPACE.NOTEBOOK);
+
+  test.beforeEach(async ({ page }) => {
+    await page.goto('/#/');
+    await waitForZeppelinReady(page);
+    await performLoginIfRequired(page);
+  });
+
+  // Regression: ZEPPELIN-6387 moved the note fetch onto the WebSocket 
connectedStatus$
+  // stream only. Because NotebookComponent is reused across :noteId param 
changes
+  // (ngOnInit does not re-run) and the socket stays connected, navigating 
between notes
+  // via the header list changed the URL but never re-fetched the note — the 
page kept
+  // showing the previous note. The fetch must also fire on route param 
changes.
+  test('Given the user is viewing a note, When they pick another note from the 
header list, Then the note content updates', async ({
+    page
+  }) => {
+    const homePage = new HomePage(page);
+    const headerPage = new HeaderPage(page);
+    const stamp = Date.now();
+    const nameA = `_e2e_nav_A_${stamp}`;
+    const nameB = `_e2e_nav_B_${stamp}`;
+
+    // Create two distinct root-level notes. createNote lands on each new 
note's page.
+    const noteIdA = await createRootNote(page, homePage, nameA);
+    const noteIdB = await createRootNote(page, homePage, nameB);
+
+    // We are now on note B (freshly mounted) — the URL and title must both 
reflect note B.
+    await expect(page).toHaveURL(new RegExp(`/notebook/${noteIdB}`));
+    const title = page.locator('[data-testid="notebook-title"]');
+    await expect(title).toContainText(nameB, { timeout: 15000 });
+
+    // In-app navigation to note A via the header notebook list — this reuses 
the
+    // already-mounted NotebookComponent, which is exactly what the regression 
broke.
+    await headerPage.clickNotebookMenu();
+    const noteALink = page.locator(`a[href*="/notebook/${noteIdA}"]`).first();
+    await noteALink.waitFor({ state: 'visible', timeout: 10000 });
+    await noteALink.click();
+
+    // The URL changing alone never caught the bug — the content must change 
too.
+    await expect(page).toHaveURL(new RegExp(`/notebook/${noteIdA}`));
+    await expect(title).toContainText(nameA, { timeout: 15000 });
+  });
+});
diff --git 
a/zeppelin-web-angular/src/app/pages/workspace/notebook/notebook.component.ts 
b/zeppelin-web-angular/src/app/pages/workspace/notebook/notebook.component.ts
index 7d86a01932..6905a5fc4e 100644
--- 
a/zeppelin-web-angular/src/app/pages/workspace/notebook/notebook.component.ts
+++ 
b/zeppelin-web-angular/src/app/pages/workspace/notebook/notebook.component.ts
@@ -22,8 +22,8 @@ import {
 import { Title } from '@angular/platform-browser';
 import { ActivatedRoute, Router } from '@angular/router';
 import { isNil } from 'lodash';
-import { Subject } from 'rxjs';
-import { distinctUntilKeyChanged, startWith, takeUntil } from 'rxjs/operators';
+import { combineLatest, Subject } from 'rxjs';
+import { distinctUntilChanged, distinctUntilKeyChanged, startWith, takeUntil } 
from 'rxjs/operators';
 
 import { NzResizeEvent } from 'ng-zorro-antd/resizable';
 
@@ -439,25 +439,31 @@ export class NotebookComponent extends 
MessageListenersManager implements OnInit
     });
     this.revisionView = !!this.activatedRoute.snapshot.params.revisionId;
 
-    // Fetch note when WebSocket connects or reconnects
-    this.messageService.connectedStatus$
-      .pipe(startWith(this.messageService.connectedStatus), 
takeUntil(this.destroy$))
-      .subscribe(connected => {
-        console.log('connectedStatus$ changed to ', connected ? 'connected' : 
'disconnected');
-        if (connected) {
-          const { noteId, revisionId } = this.activatedRoute.snapshot.params;
-          if (!noteId) {
-            throw new Error('Route parameter `noteId` is required.');
-          }
-          if (revisionId) {
-            this.messageService.noteRevision(noteId, revisionId);
-          } else {
-            this.messageService.getNote(noteId);
-          }
-          this.cdr.markForCheck();
-          this.messageService.listRevisionHistory(noteId);
-          // TODO(hsuanxyz) scroll to current paragraph
+    // Fetch the note whenever the WebSocket (re)connects OR the route's 
noteId/revisionId changes.
+    // Navigating between notes reuses this component (ngOnInit does not 
re-run) and keeps the socket
+    // connected, so the fetch must be driven by route params too — connection 
status alone would
+    // leave the page showing the previously loaded note after navigation.
+    combineLatest([
+      
this.messageService.connectedStatus$.pipe(startWith(this.messageService.connectedStatus),
 distinctUntilChanged()),
+      this.activatedRoute.params
+    ])
+      .pipe(takeUntil(this.destroy$))
+      .subscribe(([connected, params]) => {
+        if (!connected) {
+          return;
+        }
+        const { noteId, revisionId } = params;
+        if (!noteId) {
+          throw new Error('Route parameter `noteId` is required.');
         }
+        if (revisionId) {
+          this.messageService.noteRevision(noteId, revisionId);
+        } else {
+          this.messageService.getNote(noteId);
+        }
+        this.cdr.markForCheck();
+        this.messageService.listRevisionHistory(noteId);
+        // TODO(hsuanxyz) scroll to current paragraph
       });
   }
 

Reply via email to