Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
2ff8416
SF-3633 Mark AddChapters obsolete so it can be removed at a later date
pmachapman Dec 3, 2025
c503515
SF-3633 Update backend to report draft import status correctly
pmachapman Jan 6, 2026
4aed29b
SF-3633 Frontend work-in-progress
pmachapman Dec 3, 2025
9860c4f
SF-3633 Frontend work-in-progress part 2
pmachapman Dec 3, 2025
11d0e14
SF-3633 Frontend work-in-progress part 3
pmachapman Dec 8, 2025
72fd59f
SF-3633 Frontend work-in-progress part 4
pmachapman Dec 15, 2025
8fcf700
SF-3633 Frontend work-in-progress part 5
pmachapman Dec 15, 2025
07034ab
SF-3633 Frontend work-in-progress part 6
pmachapman Jan 6, 2026
f174c6a
SF-3633 Hide the wizard steps at the top of the dialog
pmachapman Jan 7, 2026
52bedb0
SF-3633 Simplified download draft button, made it secondary
pmachapman Jan 11, 2026
94bed0b
SF-3633 Import dialog fixes
pmachapman Jan 11, 2026
dc0bffe
SF-3633 Allow clicking next before the project is selected
pmachapman Jan 13, 2026
fa12199
SF-3633 Work on the storybook, and fixes found when using it
pmachapman Jan 14, 2026
b2e2f72
SF-3633 Adding all of the steps to the storybook
pmachapman Jan 19, 2026
3e61607
SF-3633 Adding unit tests to cover the basic wizard workflows
pmachapman Jan 19, 2026
4674c0d
SF-3633 Fix unit tests and storybook stories
pmachapman Jan 26, 2026
c1e83da
SF-3633 Fix issues raised with the draft wizard
pmachapman Jan 26, 2026
35fb4ad
SF-3633 Fix issues raised in code review feedback
pmachapman Jan 27, 2026
36a4270
SF-3633 Pruning unused translation strings
pmachapman Jan 27, 2026
a8443db
SF-3633 Fixed chapter plurality bug
pmachapman Jan 27, 2026
f95e6aa
SF-3633 Removed the old draft apply dialog and progress
pmachapman Feb 1, 2026
56cde0b
SF-3633 Release SignalR notification handlers
pmachapman Jan 26, 2026
07a09c4
SF-3633 Removed dependency on custom validators
pmachapman Feb 1, 2026
26ef360
SF-3633 Updates based on code review feedback
pmachapman Feb 1, 2026
3315e6c
SF-3633 Fixed issues found in testing on 4 Feb
pmachapman Feb 10, 2026
c900881
SF-3633 Fixed issues found in testing on 5 Feb
pmachapman Feb 10, 2026
f01ffbb
SF-3633 Remove redundant code
pmachapman Feb 11, 2026
6eaf22d
SF-3633 Create draft notification service with stateful reconnection
pmachapman Feb 16, 2026
0f2e7cc
SF-3633 Record source when creating ShareDB documents
pmachapman Feb 16, 2026
e00c2f7
SF-3633 Update the history tab a draft is applied
pmachapman Feb 16, 2026
478a7a1
SF-3633 Fix two test warnings noticed in testing
pmachapman Feb 16, 2026
48da70f
SF-3633 Fix unhandled errors when the project has been deleted
pmachapman Feb 17, 2026
21929f4
SF-3633 Fix issues raised by Nathaniel
pmachapman Feb 18, 2026
c8b8597
SF-3633 Fixed issues found in testing on 11 Feb
pmachapman Feb 18, 2026
6772a2c
SF-3633 Fix issues raised in code review
pmachapman Feb 22, 2026
256f3f7
SF-3633 Fixed issues found in testing on 18 Feb
pmachapman Feb 22, 2026
e214df9
SF-3633 Fixed issues found in testing on 23 Feb
pmachapman Feb 23, 2026
a8c7d27
SF-3633 Fix issues raised in code review
pmachapman Feb 24, 2026
5538524
Fix e2e tests for new draft import dialog (#3708)
Nateowami Feb 24, 2026
cfcee87
SF-3633 Addressed issues raised in code review
pmachapman Feb 24, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions src/RealtimeServer/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,8 @@ export = {
collection: string,
id: string,
data: any,
typeName: OTType
typeName: OTType,
source: string | undefined
): void => {
if (server == null) {
callback(new Error('Server not started.'));
Expand All @@ -221,7 +222,17 @@ export = {
callback(new Error('Connection not found.'));
return;
}
doc.create(data, typeName, err => callback(err, createSnapshot(doc)));
const options: any = {};
doc.submitSource = source != null;
if (source != null) {
options.source = source;
}
doc.create(data, typeName, options, err => {
if (source != null) {
doc.submitSource = false;
}
callback(err, createSnapshot(doc));
});
},

fetchDoc: (callback: InteropCallback, handle: number, collection: string, id: string): void => {
Expand Down
10 changes: 8 additions & 2 deletions src/RealtimeServer/common/utils/sharedb-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,15 @@ export function docFetch(doc: Doc): Promise<void> {
});
}

export function docCreate(doc: Doc, data: any, type?: OTType): Promise<void> {
export function docCreate(
doc: Doc,
data: any,
type?: OTType,
source: boolean | any | undefined = undefined
): Promise<void> {
const options: ShareDBSourceOptions = source != null ? { source } : {};
return new Promise<void>((resolve, reject) => {
doc.create(data, type, err => {
doc.create(data, type, options, err => {
if (err != null) {
reject(err);
} else {
Expand Down
15 changes: 13 additions & 2 deletions src/RealtimeServer/common/utils/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,19 @@ export async function hasDoc(conn: Connection, collection: string, id: string):
return doc.data != null;
}

export function createDoc<T>(conn: Connection, collection: string, id: string, data: T, type?: OTType): Promise<void> {
return docCreate(conn.get(collection, id), data, type);
export function createDoc<T>(
conn: Connection,
collection: string,
id: string,
data: T,
type?: OTType,
source: boolean | any | undefined = undefined
): Promise<void> {
const doc = conn.get(collection, id);
if (source != null) {
doc.submitSource = true;
}
return docCreate(doc, data, type, source);
}

export async function submitOp(
Expand Down
16 changes: 16 additions & 0 deletions src/RealtimeServer/scriptureforge/services/text-service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,22 @@ describe('TextService', () => {
});
});
});

it('writes the op source to the database on create', async () => {
const env = new TestEnvironment();
await env.createData();

const conn = clientConnect(env.server, 'administrator');
const id: string = getTextDocId('project01', 40, 2);
const source: string = 'history';
await createDoc<TextData>(conn, TEXTS_COLLECTION, id, new Delta(), 'rich-text', source);
await new Promise<void>(resolve => {
env.db.getOps(TEXTS_COLLECTION, id, 0, null, { metadata: true }, (_, ops) => {
expect(ops[0].m.source).toBe(source);
resolve();
});
});
});
});

class TestEnvironment {
Expand Down
24 changes: 15 additions & 9 deletions src/SIL.XForge.Scripture/ClientApp/e2e/workflows/generate-draft.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { expect } from 'npm:@playwright/test';
import { Locator, Page } from 'npm:playwright';
import { preset, ScreenshotContext } from '../e2e-globals.ts';
import { E2E_SYNC_DEFAULT_TIMEOUT, preset, ScreenshotContext } from '../e2e-globals.ts';
import {
enableDeveloperMode,
enableDraftingOnProjectAsServalAdmin,
Expand Down Expand Up @@ -203,21 +203,27 @@ export async function generateDraft(
await user.click(page.getByRole('button', { name: 'Save' }));

// Preview and apply chapter 1
await user.click(page.getByRole('radio', { name: bookToDraft }));
await user.click(page.getByRole('button', { name: bookToDraft, exact: true }));
await user.click(page.getByRole('button', { name: 'Add to project' }));
await user.click(page.getByRole('button', { name: 'Overwrite chapter' }));
await user.click(page.locator('app-tab-header').filter({ hasText: DRAFT_PROJECT_SHORT_NAME }));

// Go back to generate draft page and apply all chapters
await user.click(page.getByRole('link', { name: 'Generate draft' }));
await user.click(page.locator('app-draft-preview-books mat-button-toggle:last-child button'));
await user.click(page.getByRole('menuitem', { name: 'Add to project' }));
await user.check(page.getByRole('checkbox', { name: /I understand the draft will overwrite .* in .* project/ }));
await user.click(page.getByRole('button', { name: 'Add to project' }));
await expect(
page.getByRole('heading', { name: `Successfully applied all chapters to ${bookToDraft}` })
).toBeVisible();
await user.click(page.getByRole('button', { name: 'Close' }));
await user.click(page.getByRole('combobox', { name: 'Choose a project' }));
await user.type(DRAFT_PROJECT_SHORT_NAME);
await user.click(page.getByRole('option', { name: `${DRAFT_PROJECT_SHORT_NAME} -` }));
await user.click(page.getByRole('button', { name: 'Next' }));
await user.check(page.getByRole('checkbox', { name: /I understand that existing content will be overwritten/ }));
await user.click(page.getByRole('button', { name: 'Import' }));
await expect(page.getByText('Import complete', { exact: true })).toBeVisible();
await user.click(page.getByRole('button', { name: 'Next' }));
await user.click(page.locator('[data-test-id="step-7-sync"]'));
await expect(page.getByText(`The draft has been imported into ${DRAFT_PROJECT_SHORT_NAME}`)).toBeVisible({
timeout: E2E_SYNC_DEFAULT_TIMEOUT
});
await user.click(page.getByRole('button', { name: 'Done' }));

await screenshot(page, { pageName: 'generate_draft_add_to_project', ...context });

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -432,11 +432,11 @@ export async function localizedScreenshots(
await user.click(page.getByRole('button', { name: 'Save' }));

await forEachLocale(async locale => {
await user.hover(page.getByRole('radio').first(), defaultArrowLocation);
await user.hover(page.getByRole('button', { name: 'Ruth', exact: true }), defaultArrowLocation);
await screenshot(page, { ...context, pageName: 'draft_complete', locale });
});

await page.getByRole('radio', { name: 'Ruth' }).first().click();
await user.click(page.getByRole('button', { name: 'Ruth', exact: true }));

await expect(page.getByRole('button', { name: 'Add to project' })).toBeVisible({ timeout: 15_000 });

Expand All @@ -461,19 +461,16 @@ export async function localizedScreenshots(
await expect(page.getByText('The draft is ready')).toBeVisible();

await forEachLocale(async locale => {
await page.getByRole('radio').nth(1).click();
await user.hover(page.getByRole('menuitem').last(), defaultArrowLocation);
await user.hover(page.getByRole('button', { name: 'Add to project' }), defaultArrowLocation);
await screenshot(page, { ...context, pageName: 'import_book', locale });
await page.keyboard.press('Escape');
});

await forEachLocale(async locale => {
await page.getByRole('radio').nth(1).click();
await page.getByRole('menuitem').last().click();
await user.click(page.getByRole('button', { name: 'Add to project' }));

await page.getByRole('combobox').fill('seedsp2');
await page.getByRole('option', { name: 'seedsp2 - ' }).click();
await page.getByRole('checkbox').check();
await user.hover(page.getByRole('button').last(), defaultArrowLocation);
await user.hover(page.getByRole('button', { name: 'next' }), defaultArrowLocation);
await screenshotElements(
page,
[page.locator('mat-dialog-container')],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import { Injectable } from '@angular/core';
import { AbortError, HubConnection, HubConnectionBuilder, IHttpConnectionOptions } from '@microsoft/signalr';
import {
AbortError,
HubConnection,
HubConnectionBuilder,
HubConnectionState,
IHttpConnectionOptions
} from '@microsoft/signalr';
import { AuthService } from 'xforge-common/auth.service';
import { OnlineStatusService } from 'xforge-common/online-status.service';

Expand All @@ -11,6 +17,7 @@ export class ProjectNotificationService {
private options: IHttpConnectionOptions = {
accessTokenFactory: async () => (await this.authService.getAccessToken()) ?? ''
};
private openConnections: number = 0;

constructor(
private authService: AuthService,
Expand All @@ -30,6 +37,10 @@ export class ProjectNotificationService {
this.connection.off('notifyBuildProgress', handler);
}

removeNotifyDraftApplyProgressHandler(handler: any): void {
this.connection.off('notifyDraftApplyProgress', handler);
}

removeNotifySyncProgressHandler(handler: any): void {
this.connection.off('notifySyncProgress', handler);
}
Expand All @@ -38,25 +49,39 @@ export class ProjectNotificationService {
this.connection.on('notifyBuildProgress', handler);
}

setNotifyDraftApplyProgressHandler(handler: any): void {
this.connection.on('notifyDraftApplyProgress', handler);
}

setNotifySyncProgressHandler(handler: any): void {
this.connection.on('notifySyncProgress', handler);
}

async start(): Promise<void> {
await this.connection.start().catch(err => {
// Suppress AbortErrors, as they are not caused by server error, but the SignalR connection state
// These will be thrown if a user navigates away quickly after
// starting the sync or the app loses internet connection
if (err instanceof AbortError || !this.appOnline) {
return;
} else {
throw err;
}
});
this.openConnections++;
if (
this.connection.state !== HubConnectionState.Connected &&
this.connection.state !== HubConnectionState.Connecting &&
this.connection.state !== HubConnectionState.Reconnecting
) {
await this.connection.start().catch(err => {
// Suppress AbortErrors, as they are not caused by server error, but the SignalR connection state
// These will be thrown if a user navigates away quickly after
// starting the sync or the app loses internet connection
if (err instanceof AbortError || !this.appOnline) {
return;
} else {
throw err;
}
});
}
}

async stop(): Promise<void> {
await this.connection.stop();
// Only stop the connection if this is the last open connection
if (--this.openConnections <= 0) {
await this.connection.stop();
}
Comment on lines 80 to 84

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 openConnections counter goes negative in ProjectNotificationService when stop() called without start()

Same issue as in DraftNotificationService: the history-chooser.component.ts:150-153 calls projectNotificationService.stop() unconditionally in onDestroy, but start() is only called inside a subscription (history-chooser.component.ts:141-148) that fires when projectId$ emits. If the component is destroyed before projectId$ emits, stop() decrements openConnections to -1.

Root Cause and Impact

This causes the same resource leak as BUG-0001: the SignalR connection to /project-notifications will never be stopped because the counter never reaches 0 again. This affects the HistoryChooserComponent which is used in the editor history view.

Suggested change
async stop(): Promise<void> {
await this.connection.stop();
// Only stop the connection if this is the last open connection
if (--this.openConnections === 0) {
await this.connection.stop();
}
async stop(): Promise<void> {
// Only stop the connection if this is the last open connection
if (this.openConnections > 0 && --this.openConnections === 0) {
await this.connection.stop();
}
}
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

}

async subscribeToProject(projectId: string): Promise<void> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,15 +119,6 @@ describe('SFProjectService', () => {
}));
});

describe('onlineAddChapters', () => {
it('should invoke the command service', fakeAsync(async () => {
const env = new TestEnvironment();
await env.service.onlineAddChapters('project01', 1, [2, 3]);
verify(mockedCommandService.onlineInvoke(anything(), 'addChapters', anything())).once();
expect().nothing();
}));
});

describe('onlineSetDraftApplied', () => {
it('should invoke the command service', fakeAsync(async () => {
const env = new TestEnvironment();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,6 @@ export class SFProjectService extends ProjectService<SFProject, SFProjectDoc> {
});
}

onlineAddChapters(projectId: string, book: number, chapters: number[]): Promise<void> {
return this.onlineInvoke<void>('addChapters', { projectId, book, chapters });
}

onlineUpdateSettings(id: string, settings: SFProjectSettings): Promise<void> {
return this.onlineInvoke('updateSettings', { projectId: id, settings });
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,25 +183,6 @@ describe('TextDocService', () => {
});
});

describe('createTextDoc', () => {
it('should throw error if text doc already exists', fakeAsync(() => {
const env = new TestEnvironment();
expect(() => {
env.textDocService.createTextDoc(env.textDocId, getTextDoc(env.textDocId));
tick();
}).toThrowError();
}));

it('creates the text doc if it does not already exist', fakeAsync(async () => {
const env = new TestEnvironment();
const textDocId = new TextDocId('project01', 40, 2);
const textDoc = await env.textDocService.createTextDoc(textDocId, getTextDoc(textDocId));
tick();

expect(textDoc.data).toBeDefined();
}));
});

describe('isDataInSync', () => {
it('should return true if the project is undefined', () => {
const env = new TestEnvironment();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ import { SF_PROJECT_RIGHTS, SFProjectDomain } from 'realtime-server/lib/esm/scri
import { TextData } from 'realtime-server/lib/esm/scriptureforge/models/text-data';
import { Chapter, TextInfo } from 'realtime-server/lib/esm/scriptureforge/models/text-info';
import { TextInfoPermission } from 'realtime-server/lib/esm/scriptureforge/models/text-info-permission';
import { type } from 'rich-text';
import { Observable, Subject } from 'rxjs';
import { RealtimeService } from 'xforge-common/realtime.service';
import { UserService } from 'xforge-common/user.service';
import { TextDoc, TextDocId, TextDocSource } from './models/text-doc';
import { SFProjectService } from './sf-project.service';
Expand All @@ -21,8 +19,7 @@ export class TextDocService {

constructor(
private readonly projectService: SFProjectService,
private readonly userService: UserService,
private readonly realtimeService: RealtimeService
private readonly userService: UserService
) {}

/**
Expand Down Expand Up @@ -81,18 +78,6 @@ export class TextDocService {
);
}

async createTextDoc(textDocId: TextDocId, data?: TextData): Promise<TextDoc> {
let textDoc: TextDoc = await this.projectService.getText(textDocId);

if (textDoc?.data != null) {
throw new Error(`Text Doc already exists for ${textDocId}`);
}

data ??= { ops: [] };
textDoc = await this.realtimeService.create(TextDoc.COLLECTION, textDocId.toString(), data, type.uri);
return textDoc;
}

/**
* Determines if the data is in sync for the project.
*
Expand Down
Loading
Loading