From eff8d6b19dc7b8fd1d60cd191eef633a400986f1 Mon Sep 17 00:00:00 2001 From: ChanHo Lee Date: Sat, 30 May 2026 17:42:53 +0900 Subject: [PATCH] [ZEPPELIN-6423] Reload note when switching notebooks in the Angular UI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The note fetch was bound only to the WebSocket connectedStatus$ stream. Navigating between notes reuses NotebookComponent (ngOnInit does not re-run) and keeps the socket connected, so selecting another note from the header list changed the URL but never re-fetched the note - the page kept showing the previously loaded note. Drive the fetch from a combineLatest of the connection status and the route params so it fires on both WebSocket (re)connect and noteId/revisionId changes, preserving the reconnect-reload behaviour from ZEPPELIN-6387. Add an e2e regression test that opens one note and navigates to another via the header notebook list, asserting the displayed note content updates and not just the URL. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Assisted-By: Claude --- .../notebook/main/notebook-navigation.spec.ts | 77 +++++++++++++++++++ .../workspace/notebook/notebook.component.ts | 46 ++++++----- 2 files changed, 103 insertions(+), 20 deletions(-) create mode 100644 zeppelin-web-angular/e2e/tests/notebook/main/notebook-navigation.spec.ts 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 00000000000..db259de8340 --- /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 => { + 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 6656945188a..a0ac3d626a0 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'; @@ -438,25 +438,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 }); }