Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ const verseCounts: Record<string, number> = {
SST: 64,
DNT: 424,
BLT: 42,
'3ES': 944,
'3ES': 434,
EZA: 715,
'5EZ': 88,
'6EZ': 141,
Expand All @@ -115,6 +115,118 @@ const verseCounts: Record<string, number> = {
LAO: 20
};

/** The expected number of chapters per book, based primarily on the eng.vrs versification files. */
const chapterCounts: Record<string, number> = {
GEN: 50,
EXO: 40,
LEV: 27,
NUM: 36,
DEU: 34,
JOS: 24,
JDG: 21,
RUT: 4,
'1SA': 31,
'2SA': 24,
'1KI': 22,
'2KI': 25,
'1CH': 29,
'2CH': 36,
EZR: 10,
NEH: 13,
EST: 10,
JOB: 42,
PSA: 150,
PRO: 31,
ECC: 12,
SNG: 8,
ISA: 66,
JER: 52,
LAM: 5,
EZK: 48,
DAN: 12,
HOS: 14,
JOL: 3,
AMO: 9,
OBA: 1,
JON: 4,
MIC: 7,
NAM: 3,
HAB: 3,
ZEP: 3,
HAG: 2,
ZEC: 14,
MAL: 4,
MAT: 28,
MRK: 16,
LUK: 24,
JHN: 21,
ACT: 28,
ROM: 16,
'1CO': 16,
'2CO': 13,
GAL: 6,
EPH: 6,
PHP: 4,
COL: 4,
'1TH': 5,
'2TH': 3,
'1TI': 6,
'2TI': 4,
TIT: 3,
PHM: 1,
HEB: 13,
JAS: 5,
'1PE': 5,
'2PE': 3,
'1JN': 5,
'2JN': 1,
'3JN': 1,
JUD: 1,
REV: 22,
TOB: 14,
JDT: 16,
ESG: 10,
WIS: 19,
SIR: 51,
BAR: 6,
LJE: 1,
S3Y: 1,
SUS: 1,
BEL: 1,
'1MA': 16,
'2MA': 15,
'3MA': 7,
'4MA': 18,
'1ES': 9,
'2ES': 16,
MAN: 1,
PS2: 1,
ODA: 14,
PSS: 18,
JSA: 24,
JDB: 21,
TBS: 14,
SST: 1,
DNT: 12,
BLT: 1,
'3ES': 9,
EZA: 12,
'5ES': 2,
'6ES': 2,
DAG: 14,
PS3: 4,
'2BA': 77,
LBA: 9,
JUB: 34,
ENO: 42,
'1MQ': 36,
'2MQ': 20,
'3MQ': 10,
REP: 6,
'4BA': 5,
LAO: 1
};
Comment on lines +119 to +228
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration bot Mar 12, 2026

Choose a reason for hiding this comment

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

🟡 Incorrect keys '5ES' and '6ES' in chapterCounts should be '5EZ' and '6EZ'

The new chapterCounts table uses keys '5ES' and '6ES' for 5 Ezra and 6 Ezra, but the canonical book IDs used throughout the codebase (in verseCounts at progress.service.ts:102-103, and in all localization files like checking_en.json:155-156) are '5EZ' and '6EZ'. When expectedBookChapters is called with Canon.bookNumberToId(bookNum) for these books, it will look up '5EZ'/'6EZ' in chapterCounts, find no match, and fall back to 1 instead of the correct value 2.

Open in Devin Review

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


export interface BookProgress {
/** The book identifier (e.g. "GEN", "MAT"). */
bookId: string;
Expand Down Expand Up @@ -159,6 +271,11 @@ export function estimatedActualBookProgress(bookProgress: BookProgress): number
}
}

/** Returns the expected number of chapters for a given bookId. */
export function expectedBookChapters(bookId: string): number {
return chapterCounts[bookId] ?? 1;
}

@Injectable({ providedIn: 'root' })
export class ProgressService {
constructor(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,11 @@ describe('TextComponent', () => {
env.component.onEditorCreated(mockedQuill);
env.waitForEditor();
// Placeholder should be no-content, as there is nothing to show.
expect(env.component.placeholder).toEqual('text.book_does_not_exist');
expect(env.component.placeholder).toEqual('text.error');
// The user goes offline, but 'no-content' is still the placeholder.
env.onlineStatus = false;
env.waitForEditor();
expect(env.component.placeholder).toEqual('text.book_does_not_exist');
expect(env.component.placeholder).toEqual('text.error');
env.onlineStatus = true;
env.waitForEditor();

Expand Down Expand Up @@ -118,19 +118,17 @@ describe('TextComponent', () => {
env.runWithDelayedGetText(env.notPresentTextDocId, () => {
env.id = env.notPresentTextDocId;
env.waitForEditor();
expect(env.component.placeholder).toEqual('text.book_does_not_exist');
expect(env.component.placeholder).toEqual('text.this_book_does_not_exist');
});

// The user is offline and goes to a location that the project does not have. The placeholder should indicate
// 'no-content'.
env.id = env.matTextDocId;
env.waitForEditor();
env.onlineStatus = false;
env.waitForEditor();
env.runWithDelayedGetText(env.notPresentTextDocId, () => {
env.id = env.notPresentTextDocId;
env.waitForEditor();
expect(env.component.placeholder).toEqual('text.book_does_not_exist');
expect(env.component.placeholder).toEqual('text.this_book_does_not_exist');
});
env.onlineStatus = true;
env.waitForEditor();
Expand All @@ -139,18 +137,16 @@ describe('TextComponent', () => {
// is not present in the source text. Suppose this is so. Then the placeholder should indicate 'no-content'.
env.id = undefined;
env.waitForEditor();
expect(env.component.placeholder).toEqual('text.book_does_not_exist');
expect(env.component.placeholder).toEqual('text.error');

// Suppose we are offline and id was set to undefined by editor.component because the current book is not present in
// the source text. The problem is that the book is not present, not that we are offline. So the placeholder should
// indicate 'no-content', not 'offline'.
env.onlineStatus = true;
env.waitForEditor();
env.id = env.matTextDocId;
env.waitForEditor();
env.id = undefined;
env.id = env.notPresentTextDocId;
env.waitForEditor();
expect(env.component.placeholder).toEqual('text.book_does_not_exist');
expect(env.component.placeholder).toEqual('text.this_book_does_not_exist');

const callback: (env: TestEnvironment) => void = (env: TestEnvironment) => {
env.realtimeService.addSnapshot<User>(UserDoc.COLLECTION, {
Expand Down Expand Up @@ -260,7 +256,7 @@ describe('TextComponent', () => {
const env = new TestEnvironment();
env.hostComponent.isTextRightToLeft = true;
env.waitForEditor();
expect(env.component.placeholder).toEqual('text.book_does_not_exist');
expect(env.component.placeholder).toEqual('text.error');
expect(env.component.isRtl).toBe(false);
expect(env.fixture.nativeElement.querySelector('quill-editor[dir="auto"]')).not.toBeNull();
}));
Expand Down Expand Up @@ -1775,6 +1771,9 @@ class TestEnvironment {
when(mockedTranslocoService.translate<string>(anything())).thenCall(
(translationStringKey: string) => translationStringKey
);
when(mockedTranslocoService.translate<string>(anything(), anything())).thenCall(
(translationStringKey: string, _: any) => translationStringKey
);

this.realtimeService.addSnapshot<SFProjectProfile>(SFProjectProfileDoc.COLLECTION, {
id: 'project01',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { QuillEditorComponent } from 'ngx-quill';
import Quill, { Delta, EmitterSource, Range } from 'quill';
import QuillCursors from 'quill-cursors';
import { AuthType, getAuthType } from 'realtime-server/lib/esm/common/models/user';
import { SFProjectProfile } from 'realtime-server/lib/esm/scriptureforge/models/sf-project';
import { SFProjectRole } from 'realtime-server/lib/esm/scriptureforge/models/sf-project-role';
import { TextAnchor } from 'realtime-server/lib/esm/scriptureforge/models/text-anchor';
import { StringMap } from 'rich-text';
Expand Down Expand Up @@ -285,6 +286,7 @@ export class TextComponent implements AfterViewInit, OnDestroy {
private _selectionBoundsTop: number = 0;
private highlightMarkerHeight: number = 0;
private _placeholder?: string;
private project?: SFProjectProfile;
private currentUserDoc?: UserDoc;
private readonly cursorColorStorageKey = 'cursor_color';
private isDestroyed: boolean = false;
Expand Down Expand Up @@ -347,7 +349,14 @@ export class TextComponent implements AfterViewInit, OnDestroy {
case 'no-content':
// There isn't any content to show, even if we were online. Setting the placeholder Input customizes this
// no-content message.
return this._placeholder ?? this.transloco.translate('text.book_does_not_exist');
if (this._placeholder != null) return this._placeholder;
if (this.id == null || this.project == null) return this.transloco.translate('text.error');

const bookId = Canon.bookNumberToId(this.id.bookNum);
const bookName = this.transloco.translate(`canon.book_names.${bookId}`);
return this.project.texts.some(t => t.bookNum === this.id!.bookNum)
? this.transloco.translate('text.chapter_does_not_exist', { projectName: this.project.name })
: this.transloco.translate('text.this_book_does_not_exist', { bookName, projectName: this.project.name });
case 'offline-or-loading':
if (this.onlineStatusService.isOnline) {
return this.transloco.translate('text.loading');
Expand Down Expand Up @@ -1075,18 +1084,18 @@ export class TextComponent implements AfterViewInit, OnDestroy {
async projectHasText(): Promise<boolean> {
if (this.id == null) throw new Error('Invalid state. id is null.');
const id: TextDocId = this.id;

this.project = undefined;
if (!this.userProjects?.includes(this.projectId)) {
this.loadingState = 'permission-denied';
return false;
}
const profile: SFProjectProfileDoc = await this.projectService.getProfile(this.projectId);
if (profile.data == null) throw new Error('Failed to fetch project profile.');
if (
profile.data.texts.some(
text => text.bookNum === id.bookNum && text.chapters.some(chapter => chapter.number === id.chapterNum)
)
) {
const profileDoc: SFProjectProfileDoc = await this.projectService.getProfile(this.projectId);
if (profileDoc.data == null) throw new Error('Failed to fetch project profile.');
this.project = profileDoc.data;
const chapterExists = this.project.texts.some(
t => t.bookNum === id.bookNum && t.chapters.some(c => c.number === id.chapterNum)
);
if (chapterExists) {
return true;
}
this.loadingState = 'no-content';
Expand Down Expand Up @@ -1158,6 +1167,7 @@ export class TextComponent implements AfterViewInit, OnDestroy {
}

if (!(await this.projectHasText())) {
this.loaded.emit();
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -426,14 +426,16 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges {

private getTargetOps(): Observable<DeltaOperation[]> {
return from(this.projectService.getText(this.textDocId!)).pipe(
switchMap(textDoc =>
textDoc.changes$.pipe(
startWith(undefined),
throttleTime(2000, asyncScheduler, { leading: true, trailing: true }),
map(() => textDoc.data?.ops),
filterNullish()
)
)
switchMap(textDoc => {
if (textDoc.isLoaded)
return textDoc.changes$.pipe(
startWith(undefined),
throttleTime(2000, asyncScheduler, { leading: true, trailing: true }),
map(() => textDoc.data?.ops),
filterNullish()
);
else return of([] as DeltaOperation[]);
Comment on lines +430 to +437
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚩 editor-draft returns empty ops array for unloaded text docs

At editor-draft.component.ts:437, when textDoc.isLoaded is false, the observable returns of([] as DeltaOperation[]). This empty array flows into the targetOps$ stream. Downstream code at editor-draft.component.ts:356 uses this.hasContent(this.targetDelta?.ops) to determine if there's existing content to warn about overwriting. An empty array means hasContent returns false, so the overwrite warning won't be shown for unloaded docs. This is probably the desired behavior — if the text doc doesn't exist (e.g., a chapter only available as a draft), there's nothing to overwrite. However, if the doc exists but is temporarily unloaded (e.g., offline), this could silently allow overwriting without warning.

Open in Devin Review

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

})
);
}
}
Loading
Loading